[Solved] Why does this causes the application to hang [closed]

Question

The biggest problem here is that your constructor does not return until all of the tasks have completed. Until the constructor returns, the window will not be shown, because the window messages related to drawing the window aren’t going to be processed.

Note that you don’t really have “deadlock” here per se. Instead, if you waited long enough (i.e. until all the tasks have completed), the window would actually be shown.

When you add the call to MessageBox.Show(), you give the UI thread a chance to process the window message queue. That is, the normal modal dialog includes a thread message pump which winds up handling those messages in the queue, including those related to showing your window. Note that even if you add the MessageBox.Show(), that won’t result in the window being updated as your processing progresses. It just allows the window to be shown before you block the UI thread again.

One way to address this is to switch to the async/await pattern. For example:

public MainWindow()
{
    InitializeComponent();
    var _ = ProcessURLs();
}

public async Task ProcessURLs()
{
    List<Task<string>> tasks = URLsToProcess.Select(uri => DownloadStringAsTask(new Uri(uri))).ToList();

    while (tasks.Count > 0)
    {
        Task<string> task = await Task.WhenAny(tasks);
        string messageText;

        if (task.Status == TaskStatus.RanToCompletion)
        {
            messageText = string.Format("{0} has completed", task.AsyncState);
            // TODO: do something with task.Result, i.e. the actual downloaded text
        }
        else
        {
            messageText = string.Format("{0} has completed with failure: {1}", task.AsyncState, task.Status);
        }

        this.tbStatusBar.Text = messageText;
        tasks.Remove(task);
    }

    tbStatusBar.Text = "All tasks completed";
}

I’ve rewritten the ProcessURLs() method as an async method. This means that when the constructor calls it, it will run synchronously up to the first await statement, at which point it will yield and allow the current thread to continue normally.

When the call to Task.WhenAny() completes (i.e. any of the tasks complete), the runtime will resume execution of the ProcessURLs() method by invoking the continuation on the UI thread. This allows the method to access the UI objects (e.g. this.tbStatusBar.Text) normally, while occupying the UI thread only long enough to process the completion.

When the loop returns to the top and the Task.WhenAny() method is called again, the whole sequence is repeated (i.e. just the way a loop is supposed to work 🙂 ).

Some other notes:

The var _ = bit in the constructor is there to suppress the compiler warning that would otherwise occur when the Task return value is ignored.
IMHO, it would be better to not initialize these operations in the constructor. The constructor is just generally a bad place to be doing significant work like this. Instead, I would (for example) override the OnActivated() method, making it async so you can use the await statement with the call to ProcessURLs() (i.e. a more idiomatic way to call an async method). This ensures the window is completely initialized and shown before you start doing any other processing.

In this particular example, starting the processing in the constructor is probably not really going to hurt anything, as long as you’re using async/await, since the UI-related stuff isn’t going to be able to be executed in any case until at least the constructor has returned. I just try to avoid doing this sort of thing in the constructor as a general rule.

I also modified the general handling of your task collection, to something that I feel is somewhat more suitable. It gets rid of the repeated reinitialization of the tasks collection, as well as takes advantage of the semantics of the WhenAny() method. I also removed the AsParallel(); given that the long-running part of the processing is handled asynchronously already, there did not seem to be any advantage in the attempt to parallelize the Select() itself.

Accepted Answer