How to download multiple files using webclient and threading?

Question

I need to create multiple file downloaders using WebClient and threading, for learning purposes I created this code (below), the file is downloaded correctly but sometime the download stops and doesn't start running again. I also want to control the ProgressBar. Invoker of this code is a button.

for (int i = 0; i < 3; i++)
{
    Start(
        delegate
        {
            WebClient wc = new WebClient();
            if(wc.IsBusy != true)
            {
                wc.DownloadFileAsync(new Uri("http://ipv4.download.thinkbroadband.com/10MB.zip"), @"D:\File" + Convert.ToString(i) + ".txt");
            wc.DownloadProgressChanged += 
                delegate(object sender1, DownloadProgressChangedEventArgs e1) {
                    foreach (ToolStripProgressBar it in statusStrip1.Items)
                    {
                        if ((it.Name == "pg" + Convert.ToString(x)) == true)
                        {
                            it.Control.Invoke((MethodInvoker)delegate() { it.Value = e1.ProgressPercentage; });
                        }
                    }
            };
            }
        });
}

public static Thread Start(Action action)
{
    Thread thread = new Thread(() => { action(); });
    thread.Start();
    return thread;
}

This may not be EXACTLY what you are looking for [http://stackoverflow.com/a/6992743/507025][1] — MRebai, Jun 24 '14 at 16:47
There is no need to use threads here at all. Your method is asynchronous; it takes almost no time to run. You can just remove all of your code to push it to another thread and lose nothing, without blocking your UI. — Servy, Jun 24 '14 at 16:55
Your code does IO bound work, no need for extra threads. Look into the `Task Asynchronous Pattern` and the use of `async/await` — Yuval Itzchakov, Jun 24 '14 at 17:00

score 2 · Accepted Answer · answered Jun 24 '14 at 17:20

Your program doesn't work for several reasons: (I believe) Your are using several progress bars (one for every thread), but the WebClient doesn't provide any information which file caused the DownloadProgressChanged event to be fired, so you need one WebClient instance per file.

Second when you pass the loop variable i to the wc.DownloadFileAsync(new Uri("http://ipv4.download.thinkbroadband.com/10MB.zip"), @"D:\File" + Convert.ToString(i) + ".txt"); method, you pass a captured reference to the loop variable and not the value (because you are using a delegate referencing to a variable outside its scope, google for "Closure"), which means when the file starts downloading it may be copied to the wrong file. You avoid this by passing the value to the delegate.

An example, which is not exactly the same as your program but you can easily modify it:

var progressBars = new ProgressBar[]
{
    this.progressBar1,
    this.progressBar2,
    this.progressBar3
};

for (int i = 0; i < 3; i++)
{
    var webClient = new WebClient();
    webClient.DownloadFileAsync(new Uri("http://ipv4.download.thinkbroadband.com/10MB.zip"), @"E:\File" + Convert.ToString(i) + ".txt");
    var progressBar = progressBars[i];
    webClient.DownloadProgressChanged += (sender1, e1) =>
    {
        progressBar.Invoke((MethodInvoker)(() =>
        {
            progressBar.Value = e1.ProgressPercentage;
        }));
    };
}

thank you for your answer, before I thought to ask how to collect control in the array, and you answer it. and this code works fine http://i.imgur.com/ueX5yLj.png — Vico Ervanda, Jun 25 '14 at 02:32

Servy · Answer 2 · 2014-06-24T17:06:32.630

You're not holding onto a reference to your WebClient, so the Garbage Collector is technically able to reclaim it at pretty much any time after you finish starting the request. You need to hold onto a reference to it somewhere so that the GC can't do this.

Fortunately, this is a fairly solvable problem. We can just create a new class that will create a client for us while also storing it internally until it is disposed of:

public class ClientCreator
{
    private static HashSet<WebClient> clients = new HashSet<WebClient>();
    public static WebClient CreateClient()
    {
        WebClient client = new WebClient();
        lock (clients)
            clients.Add(client);
        client.Disposed += (s, args) =>
        {
            lock (clients) clients.Remove(client);
        };
        return client;
    }
}

Now all you need to do is make sure that you dispose of your web client when you're done with it (something you really should be doing anyway).

Just add wc.DownloadFileCompleted += (s, args) => wc.Dispose(); when you attach your other handlers, so that you dispose of it when the file download is completed.

It's also worth noting that there is no need to create additional threads here at all. Your method is already inherently asynchronous; it takes almost no (CPU) time to run. You can just remove all of your code to push it to another thread and lose nothing, without blocking your UI.

How to download multiple files using webclient and threading?

2 Answers2