none
Howto: Canceling while in Task.WaitAll, are disposing needed?

    Question

  • Hi.

    My question is about tasks composition and cancelation.
    Imagine some app which should track statuses of some devices. 
    We have class Device with method StartUpdate which returns a Task. That task is polling task. When it completes that means we finished getting info from device.
    And we have a endless loop with polling all devices (property "AllDevices"):
    while
    (true
    )
    {
      Task[] tasks = null
    ;
      try
    
      {
        tasks = AllDevices.AsParallel().Select(
          device =>
            {
              if
     (cancelToken.IsCancellationRequested)
    return null ;
    return
    device.StartUpdate();
    }).ToArray();
      Task.WaitAll(tasks, (int )TimeSpan.FromSeconds(30).TotalMilliseconds, cancelToken);
    }
    catch (Exception ex)
    {
    // swallow up the exception
    Trace.WriteLine("Error:" + ex);
    }
    if (cancelToken.IsCancellationRequested)
    return ;
    }
    As you can see we support cancelation through a cancelToken.
    If the root task is canceled through setting cancelToken when we are in Task.WaitAll then it throws OperationCanceledException .
    But at that time some child tasks (in tasks array) are still running and their results havn't been observed yet.
    So my question is should I add handling of OperationCanceledException loging like this:
      ...
      Task.WaitAll(tasks, (int
    )TimeSpan.FromSeconds(30).TotalMilliseconds, cancelToken);
    }
    catch (OperationCanceledException ex)
    {
    if (tasks != null )
    foreach (var task in tasks)
    {
    try
    {
    task.Dispose();
    }
    catch (Exception) {}
    }
    }
    ?
    I want to prevent unhandled exceptions from TaskExceptionHolder.Finilize()
    Sergei Dorogin, Lead Developer, CROC Inc. (www.croc.ru)
    Tuesday, February 16, 2010 11:07 PM

Answers

  • Hi Sergei,

    Yes, a canceled WaitAll operation may not observe all exceptions for the Tasks it is waiting on, so you should add handling for an OperationCanceledExeption.  However, calling Dispose is not what you want (you should not try to Dispose Tasks that may still be running).  Calling Wait is the way to mark Task exceptions as observed (and to prevent them from being thrown on the finalizer).  I suggest just scheduling a Task that Waits on each Task:


    try
    {
        Task.WaitAll(...);
    }
    catch (OperationCanceledException)
    {
        // If WaitAll was canceled, schedule a Task to
        // mark all Task exceptions as observed.
        Task.Factory.StartNew(() =>
        {
            foreach (Task t in tasks)
            {
                try { t.Wait(); }
                catch (AggregateException) { }
            }
        });
    }
    catch (AggregateException ae)
    {
        // If WaitAll was not canceled, handle/swallow Task exceptions
    }

    Hope this helps,
    Danny
    Wednesday, February 17, 2010 1:05 AM
  • An alternative to scheduling a task that waits on all of the tasks is to schedule a multi-task continuation for all of the tasks.   When that continuation completes, you know that all of the tasks are done, and you can grab/aggregate all of their exceptions, e.g.

    try
    {
        Task.WaitAll(tasks);
    }
    catch(OperationCanceledException)
    {
        Task.Factory.ContinueWhenAll(tasks, _ =>
        {
            Exceptions [] exceptions = tasks.Where(t => t.IsFaulted).Select(t => t.Exception).ToArray();
            // Log or do whatever you want with any exceptions here.
        });
    }
    Wednesday, February 17, 2010 3:48 AM
    Owner

All replies

  • Hi Sergei,

    Yes, a canceled WaitAll operation may not observe all exceptions for the Tasks it is waiting on, so you should add handling for an OperationCanceledExeption.  However, calling Dispose is not what you want (you should not try to Dispose Tasks that may still be running).  Calling Wait is the way to mark Task exceptions as observed (and to prevent them from being thrown on the finalizer).  I suggest just scheduling a Task that Waits on each Task:


    try
    {
        Task.WaitAll(...);
    }
    catch (OperationCanceledException)
    {
        // If WaitAll was canceled, schedule a Task to
        // mark all Task exceptions as observed.
        Task.Factory.StartNew(() =>
        {
            foreach (Task t in tasks)
            {
                try { t.Wait(); }
                catch (AggregateException) { }
            }
        });
    }
    catch (AggregateException ae)
    {
        // If WaitAll was not canceled, handle/swallow Task exceptions
    }

    Hope this helps,
    Danny
    Wednesday, February 17, 2010 1:05 AM
  • An alternative to scheduling a task that waits on all of the tasks is to schedule a multi-task continuation for all of the tasks.   When that continuation completes, you know that all of the tasks are done, and you can grab/aggregate all of their exceptions, e.g.

    try
    {
        Task.WaitAll(tasks);
    }
    catch(OperationCanceledException)
    {
        Task.Factory.ContinueWhenAll(tasks, _ =>
        {
            Exceptions [] exceptions = tasks.Where(t => t.IsFaulted).Select(t => t.Exception).ToArray();
            // Log or do whatever you want with any exceptions here.
        });
    }
    Wednesday, February 17, 2010 3:48 AM
    Owner
  • Thank! It helps.
    I got the idea. We need Wait in any way.

    BTW what Task.Dispose is needed for then ?
    Sergei Dorogin, Lead Developer, CROC Inc. (www.croc.ru)
    Wednesday, February 17, 2010 9:27 AM
  • Internally, Task *may* allocate a wait handle if it was ever waited on and that wait operation allocated a wait handle (not all wait operations will), or if its IAsyncResult.AsyncWaitHandle was ever requested.  Since that WaitHandle is disposable, per .NET design guidelines, Task was also made disposable so that you can force cleanup of the underlying wait handle if you desire to do so.  Unless you're creating and waiting on huge numbers of tasks, disposing of the Tasks isn't all that important.
    Wednesday, February 17, 2010 5:49 PM
    Owner