c# Task.WhenAll(tasks) and SemaphoreSlim - how to know when all tasks have fully completed

Question

I am having an issue with Task/Thread management in C# and I'm wondering if there's a simple solution to my problem.

In my Windows Phone application, I create a bunch of "upload" tasks. Each task has a progress/completion handler attached, in which I check the status of the task, and if completed successfully, I need to perform a number of thread-safe file write operations.

Pseudo-code:

This snippet is from the method which sets up my tasks and starts them running. I expect this method only to return to caller when all the tasks have fully completed:

var progressCallback = new Progress<UploadOperation>(UploadProgress);
for (var i = 0; i < uploads.Count; i++)
{
    uploadTasks[i] = uploads[i].StartAsync().AsTask(ct, progressCallback);
}
await Task.WhenAll(uploadTasks);
// all uploads complete!
return;

My progress/task complete handler checks the status, and if OK, I raise an event which triggers a call to a thread-safe method which needs to perform some file-writes:

private void UploadProgress(UploadOperation upload){
  ....
  if(upload == successful)
  {
      //raise an event which results in a call to a thread-safe method
      //to perform post-upload tasks          
  }
  ....
}

And this is my thread-safe method (triggered by the above event), in which I use a SemaphoreSlim object to ensure that exactly only one thread at a time can access it:

private static readonly SemaphoreSlim _SemaphoreSlim = new SemaphoreSlim(1);
private async void OnItemUploadOperationCompleted(...)
{
    await _SemaphoreSlim.WaitAsync();
    try
    {
       //perform some await-able file updates / writes
    }
    catch(..){}

    finally
    {
        //release lock
        _SemaphoreSlim.Release();
    }

The difficulty I am having is that the main "Task setup" method returns and exits before all of the Upload Tasks have completed their turn in the thread-safe method, i.e. We hit the return statement below while several of the Tasks still haven't taken their turn in the OnItemUploadOperationCompleted method.

await Task.WhenAll(uploadTasks);
// all uploads complete!
return;

I'm trying to work out if there's a better way to do this. Is there a way of knowing for sure that all Tasks have "fully" completed, that they aren't still hanging and waiting in a queue to enter a thread safe operation? Basically I need to know when all processing is complete, including all the thread-safe post processing of each Task.

It seems that "Task.WhenAll" returns too early, perhaps when the initial Task itself completes but not when it's sub-task / spawned task has completed?

EDIT:

I followed the suggestion made by Servy (first answer below), like as follows:

foreach (var upload in uploads)
{
    uploadTasks.Add(upload.StartAsync().AsTask(ct, progressCallback).ContinueWith(task => ProcessUploadResult(task.Result), ct));
}

await Task.WhenAll(uploadTasks);

// all uploads complete?
return;

And my ProcessUploadResult method is something like as follows:

private void ProcessUploadResult(UploadOperation uploadResult){

 ....
 //pseudo code
 if(uploadResult == success){

     //RAISE an Event!
     //The listener of this event processes the result - 
     // - performs some file writes / updates
     // - therefore the handler for this eventy MUST be thread safe.
     OnItemUploadOperationCompleted(this, ...});

 }
}

So, my difficulty was, even using this approach, was that the event Handler had still not completed processing ALL of the uploads by the time "Task.WhenAll(...)" returns. There are still threads waiting for access to that Event handler.

So, I think I have figured out a solution and I'm wondering if it is a good solution, using ManualResetEvent:

 ....
 //pseudo code
 if(uploadResult == success){

     //RAISE an Event!
     //The listener of this event processes the result - 
     // - performs some file writes / updates
     // - therefore the handler for this eventy MUST be thread safe.


     var wait = new ManualResetEvent(false);

     // pass the reference to ManualResetEvent in the event Args
     OnItemUploadOperationCompleted(this, new MyEventArgs {waiter = wait});

     wait.WaitOne();

 }
}

And now in my handler, I use that ManualResetEvent object to signal to the waiting thread that we are done processing the upload response:

private async void OnItemUploadOperationCompleted(object sender, UploadResultEventArgs e)
{
        await _SemaphoreSlim.WaitAsync();
        try
        {
             //perform async file writes
        }
        catch{
               ....
        }
        finally
        {
            //release lock
            _SemaphoreSlim.Release();

            //signal we are done here
            var waiter = e.Waiter as ManualResetEvent;
            if (waiter != null)
            {
                waiter.Set();
            }
        }
}

This finally seems to work for me. I'm wondering though is it an ideal solution?

Answer 1

The Progress class is there to update the UI with the current progress of an operation, and that operation shouldn't care about what those updates are or when they finish.

What you have here is a continuation; some work that needs to be done after a task completes to do additional work based on the results of the previous task. You should use the ContinueWith method for that. (Or an async method, as that will be transformated into continuations.)

Given all of what you have, this is actually quite simple:

uploadTasks[i] = uploads[i].StartAsync()
    .AsTask(ct, progressCallback)
    .ContinueWith(task => ProcessResult(task.Result));

Your ProcessResult method can then process these results as you had been doing when you fired the Progress instance.