Updating GUI using Tasks

Question

I have a weird problem that i can't solve, i have a form that i open within another form, as soon as i open that form and since there is no event to fire after page finish loading, at the form load event i set a Timer that will start after 5sec. The timer will trigger a Task that will download files, downloading files is updated in a progressbar. The problem is that anything i try to change while Tasks are running doesn't update the GUI and will only change GUI after all Tasks finishes, note that the progressbar gets updated fine. Here is my code:

private void frm_HosterDownloader_Load(object sender, EventArgs e)
{
    StartDownloadTimer = new Timer();
    StartDownloadTimer.Tick += StartDownloadTimer_Tick;
    StartDownloadTimer.Interval = 5000;
    StartDownloadTimer.Start();
}

void StartDownloadTimer_Tick(object sender, EventArgs e)
{
    StartDownload();
    StartDownloadTimer.Stop();
}

private void StartDownload()
{
    int counter = 0;
    Dictionary<string, string> hosters = Hosters.GetAllHostersUrls();

    progressBar_Download.Maximum = hosters.Count * 100;
    progressBar_Download.Minimum = 0;
    progressBar_Download.Value = 0;

    foreach (KeyValuePair<string, string> host in hosters)
    {
        //Updating these tow lables never works, only when  everything finishes
        lbl_FileName.Text = host.Key;
        lbl_NumberOfDownloads.Text = (++counter).ToString() + "/" + hosters.Count().ToString();

        Task downloadTask = new Task(() =>
        {
            Downloader downloader = new Downloader(host.Value, string.Format(HostersImagesFolder + @"\{0}.png", IllegalChars(host.Key)));
            downloader.HosterName = host.Key;
            downloader.DownloadFinished += downloader_DownloadFinished;
            downloader.Execute();

        });
        downloadTask.Start();
        downloadTask.Wait();
    }
}

void downloader_DownloadFinished(object sender, ProgressEventArgs e)
{
    progressBar_Download.Value = progressBar_Download.Value + (int)e.ProgressPercentage;
}

I tired putting the tow label statments within the Task and even tried to pass them as an argument to be updated in the DownloadFinish event but no luck.

Edit:

Here is the Downloader Class:

 public class Downloader : DownloaderBase
{
    public string HosterName { set; get; }

    /// <summary>
    /// Initializes a new instance of the <see cref="Downloader"/> class.
    /// </summary>
    /// <param name="hoster">The hoster to download.</param>
    /// <param name="savePath">The path to save the video.</param>
    /// <param name="bytesToDownload">An optional value to limit the number of bytes to download.</param>
    /// <exception cref="ArgumentNullException"><paramref name="video"/> or <paramref name="savePath"/> is <c>null</c>.</exception>
    public Downloader(string hosterUrl, string savePath, int? bytesToDownload = null)
        : base(hosterUrl, savePath, bytesToDownload)
    { }

    /// <summary>
    /// Occurs when the downlaod progress of the file file has changed.
    /// </summary>
    public event EventHandler<ProgressEventArgs> DownloadProgressChanged;

    /// <summary>
    /// Starts download.
    /// </summary>
    /// <exception cref="IOException">The video file could not be saved.</exception>
    /// <exception cref="WebException">An error occured while downloading the video.</exception>
    public override void Execute()
    {
        this.OnDownloadStarted(new ProgressEventArgs(0, HosterName));

        var request = (HttpWebRequest)WebRequest.Create(this.HosterUrl);

        if (this.BytesToDownload.HasValue)
        {
            request.AddRange(0, this.BytesToDownload.Value - 1);
        }

        try
        {
            // the following code is alternative, you may implement the function after your needs
            request.Timeout = 100000;
            request.ReadWriteTimeout = 100000;
            request.ContinueTimeout = 100000;
            using (WebResponse response = request.GetResponse())
            {
                using (Stream source = response.GetResponseStream())
                {
                    using (FileStream target = File.Open(this.SavePath, FileMode.Create, FileAccess.Write))
                    {
                        var buffer = new byte[1024];
                        bool cancel = false;
                        int bytes;
                        int copiedBytes = 0;

                        while (!cancel && (bytes = source.Read(buffer, 0, buffer.Length)) > 0)
                        {
                            target.Write(buffer, 0, bytes);

                            copiedBytes += bytes;

                            var eventArgs = new ProgressEventArgs((copiedBytes * 1.0 / response.ContentLength) * 100, HosterName);

                            if (this.DownloadProgressChanged != null)
                            {
                                this.DownloadProgressChanged(this, eventArgs);

                                if (eventArgs.Cancel)
                                {
                                    cancel = true;
                                }
                            }
                        }
                    }
                }
            }
        }
        catch (WebException ex)
        {
            if (ex.Status == WebExceptionStatus.Timeout)
                Execute();
        }

        this.OnDownloadFinished(new ProgressEventArgs(100, HosterName));
    }
}

 public abstract class DownloaderBase
{
    /// <summary>
    /// Initializes a new instance of the <see cref="DownloaderBase"/> class.
    /// </summary>
    /// <param name="hosterUrl">The video to download/convert.</param>
    /// <param name="savePath">The path to save the video/audio.</param>
    /// /// <param name="bytesToDownload">An optional value to limit the number of bytes to download.</param>
    /// <exception cref="ArgumentNullException"><paramref name="hosterUrl"/> or <paramref name="savePath"/> is <c>null</c>.</exception>
    protected DownloaderBase(string hosterUrl, string savePath, int? bytesToDownload = null)
    {
        if (hosterUrl == null)
            throw new ArgumentNullException("video");

        if (savePath == null)
            throw new ArgumentNullException("savePath");

        this.HosterUrl = hosterUrl;
        this.SavePath = savePath;
        this.BytesToDownload = bytesToDownload;
    }

    /// <summary>
    /// Occurs when the download finished.
    /// </summary>
    public event EventHandler<ProgressEventArgs> DownloadFinished;

    /// <summary>
    /// Occurs when the download is starts.
    /// </summary>
    public event EventHandler<ProgressEventArgs> DownloadStarted;

    /// <summary>
    /// Gets the number of bytes to download. <c>null</c>, if everything is downloaded.
    /// </summary>
    public string HosterUrl { get; set; }

    /// <summary>
    /// Gets the number of bytes to download. <c>null</c>, if everything is downloaded.
    /// </summary>
    public int? BytesToDownload { get; private set; }

    /// <summary>
    /// Gets the path to save the video/audio.
    /// </summary>
    public string SavePath { get; private set; }

    /// <summary>
    /// Starts the work of the <see cref="DownloaderBase"/>.
    /// </summary>
    public abstract void Execute();

    protected void OnDownloadFinished(ProgressEventArgs e)
    {
        if (this.DownloadFinished != null)
        {
            this.DownloadFinished(this, e);
        }
    }

    protected void OnDownloadStarted(ProgressEventArgs e)
    {
        if (this.DownloadStarted != null)
        {
            this.DownloadStarted(this, e);
        }
    }
}

Answer 1

It is not useful to use a Task this way:

downloadTask.Start();
downloadTask.Wait();

The Wait() will block the calling code and that is handling an event. Your downloads are effectively executing on the main GUI thread, blocking it.

The solution is

//downloadTask.Wait();

You don't seem to need it.

Answer 2

There is rarely a good reason to use threads (either new ones you create or threadpool) to do IO bound work. Here is an async alternative to your synchronous Execute method:

public async Task ExecuteAsync()
{
    this.OnDownloadStarted(new ProgressEventArgs(0, HosterName));

    var httpClient = new HttpClient();
    var request = (HttpWebRequest)WebRequest.Create(this.HosterUrl);

    if (this.BytesToDownload.HasValue)
    {
        request.AddRange(0, this.BytesToDownload.Value - 1);
    }

    try
    {
        request.Timeout = 100000;
        request.ReadWriteTimeout = 100000;
        request.ContinueTimeout = 100000;

        var response = await httpClient.SendAsync(request);
        var responseStream = await response.Content.ReadAsStreamAsync();

        using (FileStream target = File.Open(this.SavePath, FileMode.Create, FileAccess.Write))
        {
            var buffer = new byte[1024];
            bool cancel = false;
            int bytes;
            int copiedBytes = 0;

            while (!cancel && (bytes = await responseStream.ReadAsync(buffer, 0, buffer.Length)) > 0)
            {
                await target.WriteAsync(buffer, 0, bytes);

                copiedBytes += bytes;

                var eventArgs = new ProgressEventArgs((copiedBytes * 1.0 / response.ContentLength) * 100, HosterName);

                if (this.DownloadProgressChanged != null)
                {
                    this.DownloadProgressChanged(this, eventArgs);

                    if (eventArgs.Cancel)
                    {
                        cancel = true;
                    }
                }
            }
        }
    }

    catch (WebException ex)
    {
        if (ex.Status == WebExceptionStatus.Timeout)
    }

    this.OnDownloadFinished(new ProgressEventArgs(100, HosterName));
}

Now, there is no need to use Task.Wait or create a new Task. IO bound work is asynchronous by nature. In a combination with the new async-await keywords in C# 5, you can keep your UI responsive through the whole time, as each await yields control back to the calling method, and frees your winforms message pump to process more messasges in the meanwhile.

private async void frm_HosterDownloader_Load(object sender, EventArgs e)
{
    await Task.Delay(5000);
    await StartDownloadAsync();
}

private async Task StartDownloadAsync()
{
    int counter = 0;
    Dictionary<string, string> hosters = Hosters.GetAllHostersUrls();

    progressBar_Download.Maximum = hosters.Count * 100;
    progressBar_Download.Minimum = 0;
    progressBar_Download.Value = 0;

    var downloadTasks = hosters.Select(hoster => 
    {
        lbl_FileName.Text = hoster.Key;
        lbl_NumberOfDownloads.Text = (++counter).ToString() + "/" + hosters.Count().ToString();

        Downloader downloader = new Downloader(host.Value, string.Format(HostersImagesFolder + @"\{0}.png", IllegalChars(host.Key)));
        downloader.HosterName = host.Key;
        downloader.DownloadFinished += downloader_DownloadFinished;

        return downloader.ExecuteAsync(); 
    });

    return Task.WhenAll(downloadTasks);
}

Note i changed your timer to a Task.Delay, since it internally uses a timer and you only need it to execute once.

If you want more on the use of async-await, you can start here.