33

I have a problem with exception handling and parallel tasks.

The code shown below starts 2 tasks and waits for them to finish. My problem is, that in case a task throws an exception, the catch handler is never reached.

        List<Task> tasks = new List<Task>();
        try
        {                
            tasks.Add(Task.Factory.StartNew(TaskMethod1));
            tasks.Add(Task.Factory.StartNew(TaskMethod2));

            var arr = tasks.ToArray();                
            Task.WaitAll(arr);
        }
        catch (AggregateException e)
        {
            // do something
        }

However when I use the following code to wait for the tasks with a timeout, the exception is caught.

 while(!Task.WaitAll(arr,100));

I seem to be missing something, as the documentation for WaitAll describes my first attempt to be the correct one. Please help me in understanding why it is not working.

1
  • 1
    What do TaskMethod1 and TaskMethod2 do? What thread are you executing on? If you could turn this into a short but complete example (like my answer) that would really help.
    – Jon Skeet
    Commented Nov 18, 2010 at 17:26

3 Answers 3

29

Can't reproduce this - it works fine for me:

using System;
using System.Threading;
using System.Threading.Tasks;

class Test
{
    static void Main()
    {
        Task t1 = Task.Factory.StartNew(() => Thread.Sleep(1000));
        Task t2 = Task.Factory.StartNew(() => {
            Thread.Sleep(500);
            throw new Exception("Oops");
        });

        try
        {
            Task.WaitAll(t1, t2);
            Console.WriteLine("All done");
        }
        catch (AggregateException)
        {
            Console.WriteLine("Something went wrong");
        }
    }
}

That prints "Something went wrong" just as I'd expect.

Is it possible that one of your tasks isn't finished? WaitAll really does wait for all the tasks to complete, even if some have already failed.

5
  • 2
    thanks for your quick answer Jon! My problem was/is that the other task depends on the failed task, therefore it would wait forever on the failed task. My idea was, that the exception is caught immediately when a task fails, which is not the case. thanks for pointing that out. Commented Nov 18, 2010 at 17:59
  • 6
    @pivotnig - take a look at 'Creating Task Continuations' here to express the dependency explicitly - msdn.microsoft.com/en-us/library/dd537609.aspx Commented Nov 18, 2010 at 18:28
  • @thumbmunkeys didn't realize this posted this. It's exactly what I asked here: stackoverflow.com/q/47820918/695964
    – KFL
    Commented Dec 14, 2017 at 22:15
  • I'm a bit scared to comment on a Jon Skeet question, but I think nowadays is better to use Task.Delay instead of Thread.Sleep even for basic samples: I still see many developers using Thread.Sleep in fully async/await scenarios. BTW I understand that in this context Thread.Sleep is perfectly valid.
    – adospace
    Commented Jul 9 at 10:07
  • 1
    @adospace: I'd be reluctant to introduce more asynchrony into the question. Aside from anything else, either you'd need to wait on the returned task from Task.Delay directly (urgh), or change the argument to StartNew to be an asynchronous delegate. Additionally, it looks like Task.Delay didn't even exist in 2010 when this answer was written.
    – Jon Skeet
    Commented Jul 9 at 10:53
12

Here's how I solved the problem, as alluded to in the comments on my answer/question (above):

The caller catches any exceptions raised by the tasks being coordinated by the barrier, and signals the other tasks with a forced cancellation:

CancellationTokenSource cancelSignal = new CancellationTokenSource();
try
{
    // do work
    List<Task> workerTasks = new List<Task>();
    foreach (Worker w in someArray)
    {
        workerTasks.Add(w.DoAsyncWork(cancelSignal.Token);
    }
    while (!Task.WaitAll(workerTasks.ToArray(), 100, cancelSignal.Token)) ;

 }
 catch (Exception)
 {
     cancelSignal.Cancel();
     throw;
 }
1
  • 2
    This is nice. Never realised this use case for CancellationTokenSource.
    – trailmax
    Commented Jan 9, 2014 at 1:53
0

I was trying to create a call for each item in a collection, which turned out something like this:

var parent = Task.Factory.StartNew(() => {
  foreach (var acct in AccountList)
    {
      var currAcctNo = acct.Number;
      Task.Factory.StartNew(() =>
      {
        MyLocalList.AddRange(ProcessThisAccount(currAcctNo));
      }, TaskCreationOptions.AttachedToParent);
      Thread.Sleep(50);
    }
  });

I had to add the Thread.Sleep after each addition of a child task because if I didn't, the process would tend to overwrite the currAcctNo with the next iteration. I would have 3 or 4 distinct account numbers in my list, and when it processed each, the ProcessThisAccount call would show the last account number for all calls. Once I put the Sleep in, the process works great.

3

Your Answer

By clicking “Post Your Answer”, you agree to our terms of service and acknowledge you have read our privacy policy.

Not the answer you're looking for? Browse other questions tagged or ask your own question.