I spent a couple of days searching in Google and trying to understand why in my case Windows Forms UI is blocked when executing pings in Tasks. I saw a lot of similar cases, but none of them explains my specific case.
I have an application which sends pings asynchronously. Each ping is send inside of a Task. I use .ContinueWith to receive result of a ping and print it to textbox without blocking UI thread. It works OK if I launch all pings once. If I add a while {run} loop to make them run forever my UI becomes unresponsive and blocked, and none of the results are printed to the textbox.
Problematic Code:
Action action2 = () => {
for (int i = 0; i < ipquantity; i++)
{
int temp1 = i;
string ip = listView1.Items[temp1].SubItems[1].Text;
if (finished[temp1] == true) // Variable helps to check if ping reply was received and printed
continutask[temp1] = Task<string>.Run(() => PingStart(ip, temp1)).ContinueWith(antecedent => PrintResult(antecedent.Result, temp1));
}
};
while (run)
{
action2();
Thread.Sleep(1000);
}
while loop and why is it not blocked without it?private async void buttonStart_Click(object sender, EventArgs e)
{
run = true;
int count = listView1.Items.Count;
task = new Task<string>[count];
result1 = new string[count];
finished = new bool[count];
continutask = new Task[count];
for (int i = 0; i < count; i++)
{
finished[i] = true;
}
Action action2 = () =>
{
for (int i = 0; i < count; i++)
{
int temp1 = i;
string ip = listView1.Items[temp1].SubItems[1].Text;
if (finished[temp1] == true)
continutask[temp1] = Task<string>.Run(() => PingStart(ip, temp1)).ContinueWith(antecedent => PrintResult(antecedent.Result, temp1));
}
};
while (run)
{
action2();
//await Task.Delay;
//Thread.Sleep(1000);
}
}
public void PrintResult(string message, int seqnum)
{
Action action = () =>
{
textBox1.AppendText(message);
textBox1.AppendText(Environment.NewLine);
textBox1.AppendText("");
textBox1.AppendText(Environment.NewLine);
};
if (InvokeRequired)
Invoke(action);
else
action();
finished[seqnum] = true;
}
public string PingStart(string ip, int seqnum)
{
finished[seqnum] = false;
Ping isPing = new Ping();
PingReply reply;
const int timeout = 2000;
const string data = "aaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaa";
var buffer = Encoding.ASCII.GetBytes(data);
PingOptions options = new PingOptions();
// Use the default Ttl value which is 128,
options.DontFragment = false;
reply = isPing.Send(ip, timeout, buffer, options);
string rtt = (reply.RoundtripTime.ToString());
string success = "N/A";
if (reply.Status == IPStatus.Success)
{
success = $"{ip}" + " Success!" + $" rtt: [{rtt}]" + $"Thread: {Thread.CurrentThread.GetHashCode()} Is pool thread: {Thread.CurrentThread.IsThreadPoolThread}";
}
else if (reply.Status != IPStatus.Success)
{
success = $"{ip}" + $" Not Successful! Status: {reply.Status}" + $"Thread: {Thread.CurrentThread.GetHashCode()} Is pool thread: {Thread.CurrentThread.IsThreadPoolThread}";
}
return success;
}
Since you already create (and save) your tasks, the easiest fix would be to await them for each iteration of your while loop:
while (run)
{
action2();
foreach (Task t in continutask)
await t;
}
That way, when all pings completed (successful or not) you start the entire process again - without delay.
One more thing: You could add a textBox1.ScrollToEnd(); to PrintResult
Since there is a lot of room for improvement, below is a rewritten and simplified example. I've removed a lot of unused variables (e.g. seqnum) and made the PingStart method completely asynchronous. I also replaced your ListBox with a TextBox for easier testing, so you might want to revert that in your code.
This still isn't the cleanest of all possible implementations (mainly because of the global run) but it should show you how to do things "more async" :)
private async void buttonStart_Click(object sender, EventArgs e)
{
// If the ping loops are already running, don't start them again
if (run)
return;
run = true;
// Get all IPs (in my case from a TextBox instead of a ListBox
string[] ips = txtIPs.Text.Split(new[] {"\r\n"}, StringSplitOptions.RemoveEmptyEntries);
// Create an array to store all Tasks
Task[] pingTasks = new Task[ips.Length];
// Loop through all IPs
for(int i = 0; i < ips.Length; i++)
{
string ip = ips[i];
// Launch and store a task for each IP
pingTasks[i] = Task.Run(async () =>
{
// while run is true, ping over and over again
while (run)
{
// Ping IP and wait for result (instead of storing it an a global array)
var result = await PingStart(ip);
// Print the result (here I removed seqnum)
PrintResult(result.Item2);
// This line is optional.
// If you want to blast pings without delay,
// you can remove it
await Task.Delay(1000);
}
}
);
}
// Wait for all loops to end after setting run = false.
// You could add a mechanism to call isPing.SendAsyncCancel() instead of waiting after setting run = false
foreach (Task pingTask in pingTasks)
await pingTask;
}
// (very) simplified explanation of changes:
// async = this method is async (and therefore awaitable)
// Task<> = This async method returns a result of type ...
// Tuple<bool, string> = A generic combination of a bool and a string
// (-)int seqnum = wasn't used so I removed it
private async Task<Tuple<bool, string>> PingStart(string ip)
{
Ping isPing = new Ping();
const int timeout = 2000;
const string data = "aaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaa";
var buffer = Encoding.ASCII.GetBytes(data);
PingOptions options = new PingOptions {DontFragment = false};
// await SendPingAsync = Ping and wait without blocking
PingReply reply = await isPing.SendPingAsync(ip, timeout, buffer, options);
string rtt = reply.RoundtripTime.ToString();
bool success = reply.Status == IPStatus.Success;
string text;
if (success)
{
text = $"{ip}" + " Success!" + $" rtt: [{rtt}]" + $"Thread: {Thread.CurrentThread.GetHashCode()} Is pool thread: {Thread.CurrentThread.IsThreadPoolThread}";
}
else
{
text = $"{ip}" + $" Not Successful! Status: {reply.Status}" + $"Thread: {Thread.CurrentThread.GetHashCode()} Is pool thread: {Thread.CurrentThread.IsThreadPoolThread}";
}
// return if the ping was successful and the status message
return new Tuple<bool, string>(success, text);
}
This way you will have a loop for each IP that will continue independently of each other until run is set to false.