Async function with DownloadStringAsync

Question

What I am trying

In my WPF I crawl my internal network and see if an IP/URL can be accessed. So I call 255 IPs with DownloadStringAsync in a loop. Now, whenever I receive a response or a timeout I want to update my progressbar (which has a max of 255). My code seems to work for the first x (I believe something like 10-15) IPs as I see the progressbar moving

The Call-Loop

try
   {
     while (i < 255)
     {
         var client = new WebClient();
         client.DownloadStringCompleted += new DownloadStringCompletedEventHandler(GetInfo);
         client.DownloadStringAsync(new Uri("http://" + myIPNet + "." + i + ":1400/status/topology"));
         i++;
     }
   }
   catch (Exception ex)
   {
       Console.WriteLine(ex.Message);
   }

The GetInfo Function

private void GetInfo(object sender, DownloadStringCompletedEventArgs e)
{
    /* Each call, count up Global Ip Counter and move progressBar */
    countIps++;
    pbSearch.Value = countIps;

    /* Do Stuff with e.Result */
    ...

    /* Check if we have all IPs in net already */
    if (countIps == 255)
    {
        /* Reset progressBar */
        pbSearch.Value = 0;

        /* Enable Button  */
        btnGetAll.IsEnabled = true;
     }
 }

I found this Asynchronous File Download with Progress Bar and I think I understand this answer https://stackoverflow.com/a/9459441/1092632 but this has the progressbar updated for one download in progress, any ideas on how to "convert" this to my case?

I see multiple issues with your code. You create a `WebClient` in the loop but don't dispose it (it **is** an `IDisposable`). You want to update the `ProgressBar` directly from your model code, but that's not the way a WPF app should be designed - use a MVVM approach and data `Binding`s instead. — dymanoid, Nov 11 '16 at 10:07

score 1 · Answer 1 · answered Nov 11 '16 at 10:25

I think the main issue with your code here is that you're expecting the DownloadStringAsync to block until the DownloadStringCompleted event has fired. This does not happen as per the description of the function:

Downloads the resource specified as a Uri. This method does not block the calling thread.

In effect you are rapidly making 255 concurrent web calls and then dropping out the bottom of the loop before they're completed.

As blocking using the Event-based Asynchronous Pattern (EAP) would require significant re-architecture of your code (and significantly increase complexity) I would encourage you to look at using Task-based Asynchronous Pattern instead. Stephen Toub has an excellent guide to converting the WebClient from EAP to TAP patterns here.

Using this you could rewrite you code as follows and should meet significantly more success:

var client = new WebClient();

while (i < 255)
{
    try
    {
        byte[] result = await client.DownloadDataTask(new Uri("http://" + myIPNet + "." + i + ":1400/status/topology"));
    }
    catch (Exception ex)
    {
        Console.WriteLine(ex.Message);
    }

    pbSearch.Value = i++;
}

/* Now we have all IPs */
/* Reset progressBar */
pbSearch.Value = 0;

/* Enable Button  */
btnGetAll.IsEnabled = true;

Hope it helps.

I think I understand the concept and was able to implement a working version. 2 things though: 1) shouldn't it be `byte[] result = await DownloadDataTask(client, new Uri("http://" + myIPNet + "." + i + ":1400/status/topology"));` and 2) now it is REALLY slow, I think because it is waiting for the timeout before going to the next one? Any way to work around that? — PrimuS, Nov 11 '16 at 11:14
I think DownloadDataTask is specified as an extension method (the first parameter is qualified as 'this') so it can be used either way around. Yes, it will be slow as it's timing out on any ip addresses that don't respond. To work around this you may want to look at using some type of ICMP message (i.e. ping) rather than an HTTP download to determine if a client exists at the specific address. — ibebbs, Nov 11 '16 at 11:17

Async function with DownloadStringAsync

1 Answers1