Reputation: 107
Im creating a remote administration tool for my university degree. Im currently really stuck on a bug in my code and was wondering if anyone could shed some light on my issue.
I have Applications, a server and a client. The server runs fine. However the client is the application that freezes.
Before connecting to the server the client works perfectly. When connected to the server the client is constantly frozen on the screen.
I've narrowed the bug down to a specific piece of code, without this code running the application doesn't freeze. However the application also doesn't work.
Here is an example of that code:
private static void ReceiveResponse()
{
var buffer = new byte[2048]; //The receive buffer
try
{
int received = 0;
if (!IsLinuxServer) received = _clientSocket.Receive(buffer, SocketFlags.None); //Receive data from the server
else received = _sslClient.Read(buffer, 0, 2048);
if (received == 0) return; //If failed to received data return
var data = new byte[received]; //Create a new buffer with the exact data size
Array.Copy(buffer, data, received); //Copy from the receive to the exact size buffer
if (isFileDownload) //File download is in progress
{
Buffer.BlockCopy(data, 0, recvFile, writeSize, data.Length); //Copy the file data to memory
writeSize += data.Length; //Increment the received file size
if (writeSize == fup_size) //prev. recvFile.Length == fup_size
{
using (FileStream fs = File.Create(fup_location))
{
Byte[] info = recvFile;
// Add some information to the file.
fs.Write(info, 0, info.Length);
}
Array.Clear(recvFile, 0, recvFile.Length);
SendCommand("frecv");
writeSize = 0;
isFileDownload = false;
return;
}
}
if (!isFileDownload) //Not downloading files
{
string text = (!IsLinuxServer) ? Encoding.Unicode.GetString(data) : Encoding.UTF8.GetString(data); //Convert the data to unicode string
string[] commands = GetCommands(text); //Get command of the message
foreach (string cmd in commands) //Loop through the commands
{
HandleCommand(Decrypt(cmd)); //Decrypt and execute the command
}
}
}
catch (Exception ex) //Somethind went wrong
{
MessageBox.Show(ex.Message);
RDesktop.isShutdown = true; //Stop streaming remote desktop
// MessageBox.Show("Connection ended");
}
}
This code is how the user receives a request from the server. So it is in a timer to be run every 100ms.
Im wondering if the timer has anything to do with it. Before it was in a While(true) loop and I had the same issue which makes me think that it's the actual code making the UI freeze.
The code still works even if the application is frozen, everything on the app works, apart from the UI.
This is really frustrating and I can't really see anything wrong with the code that would cause the application to freeze.
Any help would be much appreciated.
Thankyou In advance.
Upvotes: 2
Views: 5110
Reputation: 107
So worked out a little work around method for it, didn't use async however I did just end up creating a different thread for this task.
The new thread let me used a while(true) loop to forever loop receive response.
private void btnConnect_Click(object sender, EventArgs e)
{
ConnectToServer();
Thread timerThread = new Thread(StartTimer);
timerThread.Start();
//StartTimer();
//timerResponse.Enabled = true;
}
private static void StartTimer()
{
while (true)
{
ReceiveResponse();
}
}
This worked good enough for what I needed to do.
Thanks all for the help!
Upvotes: 0
Reputation: 660503
Six answers so far and no helpful, correct or actionable advice. Ignore them.
I've narrowed the bug down to a specific piece of code, without this code running the application doesn't freeze
Good, that's the first step in diagnosing the problem. Go further. Exactly which methods in here are the ones that have high latency? My guess would be Receive
or Read
but there are several possible candidates, and it might be more than one of them.
Once you have identified each high latency method, determine whether it is slow because it is using CPU or waiting for IO.
If it is slow because it is using CPU, what you want to do is move that operation on to another CPU and wait for the result. You can do that as the other answers suggest: use Task.Run
to move the work onto a background thread, and then await
the result.
If it is slow because it is waiting for IO then do not move the operation to a background thread unless you have no other choice. The background worker APIs are designed to offload work to other CPUs, and IO is not CPU-bound work. What you want to do here is use the async IO operations. If the blocker is Receive
, say, then be using ReceiveAsync
instead and await
the resulting Task
.
You should also make your method async
and return a Task
which is then awaited by its caller, and so on up to the event handler that kicks the whole thing off.
Keep doing this process until you have identified every operation in your method that takes more than 30 milliseconds, and made it either async on a worker thread, if CPU bound, or used an async IO method if IO bound. Your UI should then never have a delay of more than 30 ms.
Speaking of event handlers...
This code is how the user receives a request from the server. So it is in a timer to be run every 100ms.
That sounds very, very wrong. First of all, "receives a request from the server"? Servers do requests on behalf of clients, not vice versa.
Second, this sounds like a wrong way to deal with the problem.
If you asyncify this code properly there should be no need of a timer to constantly poll the socket. You should simply asynchronously read, and either the read succeeds and calls you back, or it times out.
Also, there's no loop here. You say that you run this code every 100 ms, and you read a maximum of 2K here, so you're reading 20K per second maximum no matter how saturated your network connection is, right? Does that seems right to you? because it seems wrong to me.
Upvotes: 3
Reputation: 176956
you can make use of BackGroundWorker
or make use of async/await
both will resolve issue of freezing UI.
Code given below is for example you can google it , and use one of this can resolve your issue
As there is operation which is doing IO calls and in newer framework methods with **async
is available suggestion to make use of async/awiat
async/await way (New way - .NET 4.5 for 4.0 version you will find nuget )
private async void Button_Click(object sender, RoutedEventArgs
{
var task = Task.Factory.StartNew( () => longrunnincode());
var items = await task;
//code after task get completed
}
Back Ground worker (Old way but still supported and used in lot of old application as async/await was delivered in .NET 4.5 for 4.0 version you will find nuget)
private BackgroundWorker backgroundWorker1;
this.backgroundWorker1 = new BackgroundWorker();
this.backgroundWorker1.DoWork += new
DoWorkEventHandler(this.backgroundWorker1_DoWork);
private void backgroundWorker1_DoWork(object sender, DoWorkEventArgs e)
{
BackgroundProcessLogicMethod();
}
private void backgroundWorker1_RunWorkerCompleted(object sender,
RunWorkerCompletedEventArgs e)
{
if (e.Error != null) MessageBox.Show(e.Error.Message);
else MessageBox.Show(e.Result.ToString());
}
private void StartButton_Click(object sender, EventArgs e)
{
// Start BackgroundWorker
backgroundWorker1.RunWorkerAsync(2000);
}
Don't use background workers for IO unless there is no other way. Make calls to the asynchronous IO APIs directly.
Upvotes: 1
Reputation: 327
It sounds like it probably will be the timer. If you are using the Thread.Sleep()
method, this will definitely be the issue. The use of this function locks the current thread, and since your function is not running asynchronously to your UI thread, the UI will also freeze.
To get around this problem, you can simply add the code you provided to an asynchronous Task, and use the Task.Delay()
method to ensure the function will only execute every 100ms.
See this question for more information.
Upvotes: -1
Reputation: 9824
This is a common issue with GUI's. Events are supposed to finish ASAP. When one Event does not return, no other Event can run. Not even the code that Updates the UI with the last changes will be executed. And all Windows can tell the user is that the Programm is "not responding" (because it has not yet aknowledged the last Input Windows send it's way).
What you need to do is move the long running Operation into some form of Multitasking. There are many ways to do that, some involving multiple threads, some not. When starting with Multitasking in a GUI environment (wich is the ideal case), I would use the BackgroundWorker. He is not the thing you want to be seen using in productive code/later, but it is a good Beginners tool to learn the Idiosyncracies of Multitasking (Race Conditions, Exception Handling, Callbacks).
As Network operations are not usually known to be CPU bound, you may want to go for a Threadless way of Multitasking in the long run.
Upvotes: -1
Reputation: 108
It's more important in what context you are running this code. You are probably running it on the UI thread. You should defer heavy work to another worker thread or make the method async.
Upvotes: -1