Threading issues using Ping to map active IPs - C#

Question

I am trying to create a simple Network Tool to ping all possible IPs on your local subnet and provide a list of such IPs in a DataGridView. I am new to having to consider threading which is a good thing to come across as a budding programmer. Sorry, but you are probably going to have to do some explaining to me, but in my mind this should work. Before I tried putting it in a backgroundworker thread, the application would just hang and give me a "Not Responding".

thanks ahead of time.

        private void backgroundWorker1_DoWork(object sender, DoWorkEventArgs e)
    {
        count = 0;
        for (int j = 1; j < 255; j++)
            for (int i = 1; i < 255; i++)
            {
                Ping ping = new Ping();
                PingReply pingreply = ping.Send(IPAddress.Parse(locip[0] + "." + locip[1] + "." + j + "." + i));

                if (pingreply.Status == IPStatus.Success)
                {
                    status = "o";
                    repAddress = pingreply.Address.ToString(); ;
                    repRoundtrip = pingreply.RoundtripTime.ToString();
                    repTTL = pingreply.Options.Ttl.ToString();
                    repBuffer = pingreply.Buffer.Length.ToString();

                    string[] lineBuffer = { status, repAddress, repRoundtrip, repTTL, repBuffer };
                    ipList.Rows.Add(lineBuffer);
                    count += 1;
                    progressBar.Value += 1;
                }

            }


    }

Answer 1

Part of the problem is that you are directly accessing a UI element from your background thread. The field progressBar is presumably a UI progress bar control and can only be safely accessed from the UI thread. You must use a call to .Invoke to set this value from the UI thread.

progressBar.Invoke(new MethodInvoker(UpdateProgressBarbyOne));
...

private void UpdateProgressBarByOne() {
  progressBar.Value += 1;  
}

Answer 2

You cannot access directly the progressBar1 (or any other UI element) from the backgroundWorker1 "DoWork" event, you have to use the backgroundWorker1.ProgressChanged method and handle ProgressChanged event:

// instead of progressBar.Value += 1
// use the following

const int total = 254 * 254;
backgroundWorker1.ReportProgress(count / total);

WorkerReportsProgress should be assigned to true and the event of ProgressChanged to the following method

private void backgroundWorker1_ProgressChanged(object sender, ProgressChangedEventArgs e)
{
    // assuming the Minimum = 0 and Maximum = 100 on progressBar
    progressBar.Value = e.ProgressPercentage;
}

Answer 3

Ah I love threading. It makes programs so much more interesting...

So as I started off learning about how to make responsive applications I came across the function: Application.DoEvents()

(http://msdn.microsoft.com/en-us/library/system.windows.forms.application.doevents.aspx)

What this does is causes your form to process some of the window events it's receiving. I think that your code could change to include a call after each ping request...

i.e. within the on click event handler

count = 0;
        for (int j = 1; j < 255; j++)
            for (int i = 1; i < 255; i++)
            {
                Ping ping = new Ping();
                PingReply pingreply = ping.Send(IPAddress.Parse(locip[0] + "." + locip[1] + "." + j + "." + i));

                if (pingreply.Status == IPStatus.Success)
                {
                    status = "o";
                    repAddress = pingreply.Address.ToString(); ;
                    repRoundtrip = pingreply.RoundtripTime.ToString();
                    repTTL = pingreply.Options.Ttl.ToString();
                    repBuffer = pingreply.Buffer.Length.ToString();

                    string[] lineBuffer = { status, repAddress, repRoundtrip, repTTL, repBuffer };
                    ipList.Rows.Add(lineBuffer);
                    count += 1;
                    progressBar.Value += 1;
                }
                Application.DoEvents(); //but not too often.
            }

Now this was back in the pre dot net days and it's survived till now however, it's not something that you should take lightly. If you click another button on the form it will start off another thread that will attempt to execute and if you're not careful cause thread exceptions on your form. Some developers will tell you don't use this but since your starting off I'd say give it a shot :)

I might not use this method depending on the application. Instead what I would do it actually do is to create several processing "trains"; one for each cpu core that the system had. I'd add the ips to be scanned to a queue object and then I would start up 2 to 4 instances of threads (http://msdn.microsoft.com/en-us/library/system.threading.thread.aspx) each of which taking an item off the queue in turn, process the information (i.e. do the ping logic) and put the result on another queue; and output queue. Every time a train would finish an item for work it would raise an event at the other end of which there would be a handler in the form. Using the Invoke to make thread safe calls (http://msdn.microsoft.com/en-us/library/ms171728.aspx) on my form I would update the UI's information accordingly.

Threading is fun dude :) over time you can find that you can use MSMQ to make a system that uses the multicores of other computers to do jobs such as image processing (or something with pa....... ;)