3

I have a Dialog with a progressbar. A Backgroundworker should download two files (with the WebClient) and copy them to a specified location automatically, when the Dialog is shown. How can I wait for the files to be downloaded before copying the new files.

I have tried to do something with await, but I cant change the Backgroundworker to a async method. How can I wait in the worker for the download to complete?

Code to run the worker:

private void fmUpdateingDatabaseDialog_Shown(object sender, EventArgs e)
{
    device.Connect();
    lbInformation.Text = "uploading database to " + device.FriendlyName;
    device.Disconnect();

    worker.WorkerReportsProgress = true;
    worker.WorkerSupportsCancellation = true;
    worker.DoWork += new DoWorkEventHandler(worker_DoWork);
    worker.ProgressChanged += 
        new ProgressChangedEventHandler(worker_ProgressChanged);
    worker.RunWorkerCompleted += 
        new RunWorkerCompletedEventHandler(worker_RunWorkerCompleted);
    worker.RunWorkerAsync();
}

Code in DoWork handler (paths are not empty in actual code):

private void worker_DoWork(object sender, DoWorkEventArgs e)
{
    BackgroundWorker worker = sender as BackgroundWorker;

    //download files temporary
    WebClient client = new WebClient();
    client.DownloadProgressChanged += new DownloadProgressChangedEventHandler(client_DownloadProgressChanged);
    client.DownloadFileTaskAsync(new Uri(""), Path.Combine(tempPath + ""));

    WebClient client2 = new WebClient();
    client2.DownloadProgressChanged += new DownloadProgressChangedEventHandler(client2_DownloadProgressChanged);
    client2.DownloadFileTaskAsync(new Uri(""), Path.Combine(tempPath + ""));

    //upload files to phone
    device.Connect();
        device.TransferContentToDevice(Path.Combine(tempPath+""), folder.Id, folder, true);
        device.TransferContentToDevice(Path.Combine(tempPath+""), folder.Id, folder, true);
    device.Disconnect();
}
Uwe Keim
  • 39,551
  • 56
  • 175
  • 291
j0h4nn3s
  • 2,016
  • 2
  • 20
  • 35

2 Answers2

2

You can either use the synchronous WebClient methods (e.g., DownloadFile instead of DownloadFileTaskAsync), or you can just use async/await directly instead of BackgroundWorker. In this case, you're doing mainly I/O, so async is a better fit than BackgroundWorker.

An async solution would look something like this:

private async void fmUpdateingDatabaseDialog_Shown(object sender, EventArgs e)
{
  device.Connect();
  lbInformation.Text = "uploading database to " + device.FriendlyName;
  device.Disconnect();

  var progress = new Progress<T>(data =>
  {
    // TODO: move worker_ProgressChanged code into here.
  });
  await DownloadAsync(progress);
  // TODO: move worker_RunWorkerCompleted code here.
}

private async Task DownloadAsync(IProgress<T> progress)
{
  //download files temporary
  WebClient client = new WebClient();
  client.DownloadProgressChanged += new DownloadProgressChangedEventHandler(client_DownloadProgressChanged);
  await client.DownloadFileTaskAsync(new Uri(""), Path.Combine(tempPath + ""));

  WebClient client2 = new WebClient();
  client2.DownloadProgressChanged += new DownloadProgressChangedEventHandler(client2_DownloadProgressChanged);
  await client2.DownloadFileTaskAsync(new Uri(""), Path.Combine(tempPath + ""));

  //upload files to phone
  // TODO: Check for Async versions of these methods that you can await.
  //  If there aren't any, consider using Task.Run.
  device.Connect();
  device.TransferContentToDevice(Path.Combine(tempPath+""), folder.Id, folder, true);
  device.TransferContentToDevice(Path.Combine(tempPath+""), folder.Id, folder, true);
  device.Disconnect();
}
Stephen Cleary
  • 437,863
  • 77
  • 675
  • 810
  • 1
    Tell me if I'm wrong, but I thought I can use `await` only in async methods. Since `fmUpdateingDatabaseDialog_Shown` is not async, i dont know how to implement this. Edit: I don't synchronos WebClient because seeing the progress is important. – j0h4nn3s Jan 16 '15 at 14:36
  • May I ask what your view of `async void` is? [I read an article that it should be avoided](http://haacked.com/archive/2014/11/11/async-void-methods/) and I've seen you around here on SO answering a lot of Task questions :) I figured your view on the matter might be very interesting. – default Jan 16 '15 at 15:14
  • 1
    You should avoid `async void`. `async void` should only be used for event handlers (or for things that are logically event handlers, such as `ICommand.Execute`). In this case, `fmUpdateingDatabaseDialog_Shown` is an event handler, so `async void` is acceptable. `DownloadAsync` is not an event handler, so `async void` would be wrong for that method. I have [an article](http://msdn.microsoft.com/en-us/magazine/jj991977.aspx) that goes into more detail. – Stephen Cleary Jan 16 '15 at 15:17
1

You can use something like this, which uses a "wait and pulse" mechanism to delay the code until your download operation has completed:

var locker = new object(); 

Thread t = new Thread(new ThreadStart(() =>
{
    lock (locker)
    {
        //peform your downloading operation, and wait for it to finish.
        client.DownloadFileTaskAsync(new Uri(""), Path.Combine(tempPath + ""));
        while (/* not yet downloaded */) { }; 
        //inform the parent thread that the download has finished.
        Monitor.Pulse(locker);
    }
}));

t.Start();

lock(locker)
{
    Monitor.Wait(locker);
}

However, if you have the resources, I'd suggest refactoring your code to entirely use an async-await approach (thus obviating the background worker). The background worker is one of the legacy asynchronous approaches while the recommended approach is TAP.

See Stephen Cleary's answer for an example of how to do this.

rory.ap
  • 34,009
  • 10
  • 83
  • 174
  • 3
    -, **do never ever not suggest Thread.Sleep (of any kind, whether it be 0 or 1000) and a loop for such a sencario**, rather go for WaitHandles! (see my full why/how/... @ http://stackoverflow.com/questions/8815895/why-is-thread-sleep-so-harmful/8815944#8815944) –  Jan 16 '15 at 13:52
  • I thought of that, but it seemed a little bit dirty to me. Do you have resources for an async-await approach? I would not know how to start an async function from the Dialog_shown event – j0h4nn3s Jan 16 '15 at 13:52
  • 1
    @roryap your edit didn't make it better, rather even worse. "loop + `Thread.Sleep`" stays evil for soo many reasons. Also, your solution might not be release-build compatible as there might be a chance that the compiler will remove your comparison to a constant value ... Everytime one promotes "loop + `Thread.Sleep`" a kitten dies. –  Jan 16 '15 at 14:00
  • @AndreasNiedermair -- my provided solution is simply a stop-gap measure, but my overall recommendation is to go to TAP. – rory.ap Jan 16 '15 at 14:03
  • @roryap still, a kitten is caught in purgatory, unless you replace the first part with a WaitHandle-solution, albeit your preferred solution... –  Jan 16 '15 at 14:06
  • 1
    Thank you for your discussion. I used a WaitHandle to solve my problem. – j0h4nn3s Jan 16 '15 at 14:25
  • isn't there a possibility that the second `lock(locker)` is executed first and that it then just waits for the `Monitor.Wait(locker);` call and never enters the Thread block? That looks like a deadlock to me. However, I might be misinterpreting how Monitor handles this. – default Jan 16 '15 at 14:58
  • 1
    @Default nope, `Monitor.Wait` releases the wait, so that the `lock` in the task can aquire and pulse it. Please see http://www.codeproject.com/Articles/28785/Thread-synchronization-Wait-and-Pulse-demystified: *If thread A holds the lock on the key object, why does thread B not block when it tries to acquire the lock? This is, of course, handled properly. The call to Wait in thread A releases the lock before it waits. This allows thread B to acquire the lock and call Pulse. ...* –  Jan 16 '15 at 15:00
  • @Default -- No, as Andreas just said. From [MSDN](http://msdn.microsoft.com/en-us/library/system.threading.monitor.wait%28v=vs.110%29.aspx) "Releases the lock on an object and blocks the current thread until it reacquires the lock." – rory.ap Jan 16 '15 at 15:01