James Michael Hare

...hare-brained ideas from the realm of software development...
posts - 136 , comments - 1093 , trackbacks - 0

My Links

News

Welcome to my blog! I'm a Sr. Software Development Engineer in Seattle, WA. I've been doing C++/C#/Java development for over 18 years, but have definitely learned that there is always more to learn!

All thoughts and opinions expressed in my blog and my comments are my own and do not represent the thoughts of my employer.

Blogs I Read

MCC Logo MVP Logo

Follow BlkRabbitCoder on Twitter

Tag Cloud

Archives

Post Categories

C#/.NET Toolbox: Creating a "Safer" Task Dispose()

This post is another in a series that contains generic utility classes I’ve developed along the way to help make coding a bit easier.

If these already exist as part of the framework and I’ve overlooked them, feel free to let me know! And if you know of a better way to implement them, do the same! I’m never too old to learn something new (I hope!).

Update: modified the TryDispose() method to check for IsCompleted first and mark any Task exceptions as handled.

So recently, I’ve been moving some older utility classes logic over to use the excellent .NET Task Parallel Library (TPL).  This library contains, at it’s core, a class called Task which allows parallel programming without the need of working directly with threads.

During this process, I wrote a few extension methods I found to be useful in dealing with tasks in an easier manner, and thought I’d share them in a few posts.

Background: Tasks cannot be aborted

I was working on a consumer utility class, which would read from a collection of items and handle the items given an Action<T> to perform on each until stopped.  Now with the TPL it can take advantage of the BlockingCollection<T>, which makes concurrent producing and consuming easier and more efficient.

So I modified the old consumer to use this instead of a locked generic Queue<T>.  Then I moved the Thread to a Task, and I ran across the need for a pattern that would be a safe and easy way to halt and dispose of tasks on a process shutdown.

To illustrate, let’s say in my consumer, I have a Stop() method that attempts to halt the consumer and wait for it to complete.  This works well because one would assume that when calling a method such as Stop() some work is being done and that it may take a non-trivial amount of time.  I’d also want to be able to Dispose() of the consumer, which would stop the consumption if it were currently running, and then dispose of the resources in the task, collection, cancellation tokens, etc.

So given that we’re a responsible class using IDisposable items, we might implement IDisposable something like this:

   1: // Dispose the instance by stopping the consumption, etc.
   2: public void Dispose()
   3: {
   4:     // attempt to stop the process if its currently running
   5:     Stop(DefaultTimeSpanOnStop);
   6:  
   7:     // as we will see, this can throw!
   8:     _consumer.Dispose();
   9:  
  10:     // dispose other stuff...
  11: }

Seems pretty logical, right?  Well, yes and no.

One of the things the Thread class had (much to to your chagrin or delight, depending on your views) was the ability to Abort() a thread.  This was considered bad form because it could exit the thread in an unknown state.  Thus, the Task was implemented without a way to forcibly abort.

You can cancel a Task, using a CancellationToken, which will not start the Task if it’s not yet started, or allow the code executed by the Task to check the token and leave early.  The former is taken care of automatically by the TPL if you pass the cancellation token to the Task constructor.  The latter is the responsibility of the code being executed by the Task to check the token periodically and take whatever action is necessary to safely complete the task.

The reason why this is all important is because whether or not Dispose() succeeds depends completely on the current state of the Task.  What do I mean by succeed?  Well, Task’s Dispose() is one of those few disposal methods that may throw exceptions.

This makes sense, because a Task can’t dispose safely if the Task is not in a final state, yet we also don’t want to force Dispose() to wait.  So the designers of the TPL decided that Task would throw if called in an invalid situation.

Problem: Dispose() on task throws if task isn’t stopped

So, disposing a task that reached a final state (RunToCompletion, Cancelled, or Faulted) succeeds.  This means that task will throw on any running or pre-running states (Created, WaitingForActivation, WaitingToRun, Running, WaitingForChildrenToComplete).

The problem is, sometimes we want to halt a task and clean up its resources, but we may be unsure of that task’s current state.  It could be it was never started, or is currently running, or you may have set a cancellation token to attempt to safely cancel, but it hasn’t yet had time to complete before the Dispose() is reached.

We could wait for the Task, of course, during our Consumer’s Dispose(), but the time it takes could be indefinite, which we may want to avoid.  We could also propagate the exception, of course, but then it would bleeds out details of the Task to users of our class who probably don’t care or shouldn’t know about the details.

So what do we do?  Looking at advice from the MSDN forums, it seems the general consensus is you should Dispose() a Task when possible, but that you shouldn’t go out of your way to ensure a Dispose() succeeds because the resources it holds (a wait handle) are fairly small and the finalizer will eventually clean them up.

Given this, we could have our Consumer’s Dispose() call the Task Dispose() in a try/catch block and consume the exception.

   1: // Dispose the instance by stopping the consumption, etc.
   2: public void Dispose()
   3: {
   4:     // attempt to stop the process if its currently running
   5:     Stop(DefaultTimeSpanOnStop);
   6:  
   7:     try 
   8:     {
   9:         _consumer.Dispose();
  10:     }
  11:     catch (Exception) 
  12:     {
  13:         // Consume exception, if we can't dispose not a big worry since
  14:         // finalizer will reclaim it.
  15:     }
  16:  
  17:     // dispose other stuff...
  18: }

Doing this enough times for several individual tasks or an array of tasks would quickly become bulky and repetitive. 

In addition, if the Task failed due to a Faulted state (threw an unhandled exception), then when the Task is finalized, it may throw again (I noticed this during unit testing, where a Task that threw an exception would hiccup the unit test agent when it was finalized).  To avoid this, we may want to check the Task for an Exception and handle it (if we’re disposing, after all, it’s a fair assumption we don’t intend to do anything further with the exceptions).

Solution: The TryDispose() extension method

So, let’s create a fairly simple TryDispose() extension method that will attempt a Dispose(), and will return a bool result based on whether the Dispose() succeeds or not.

We can create this extension method off of Task, which covers both Task and it’s sub-class Task<T> as well, so that it can be called as naturally as if it were a member of Task itself.

   1: // Helper extension methods on the TPL Task class
   2: public static class TaskExtensions
   3: {
   4:     // Attempts to dispose of a Task, but will not propagate the exception.  
   5:     // Returns false instead if the Task could not be disposed.
   6:     public static bool TryDispose(this Task source, bool shouldMarkExceptionsHandled = true)
   7:     {
   8:         if (source == null)
   9:         {
  10:             throw new ArgumentNullException("source");                
  11:         }
  12:     
  13:         try
  14:         {
  15:             // no sense attempting to dispose unless we are completed, otherwise we know we'll throw
  16:             // and why add the overhead.
  17:             if (source.IsCompleted)
  18:             {
  19:                 if (shouldMarkExceptionsHandled && source.Exception != null)
  20:                 {
  21:                     // handle all parts of aggregate exception (true == handled)
  22:                     source.Exception.Flatten().Handle(x => true);
  23:                 }
  24:     
  25:                 source.Dispose();
  26:                 return true;
  27:             }
  28:         }
  29:     
  30:         catch (Exception)
  31:         {                
  32:             // consume any other possible exception on dispose so dispose is as safe as possible
  33:         }
  34:     
  35:         // return false if any exception occurred or because task has not yet completed.
  36:         return false;
  37:     }
  38: }

UPDATE: I updated the method so it now checks for IsCompleted before attempting to Dispose() to avoid any unnecessary exceptions.  Also added code to mark any exceptions as handled.

It looks so trivial you may think, “what’s the value”?  Well, the value is that it can really clean up repetative code in classes that use Task and we won’t be as likely to leak our abstraction outside of the class.

Now we can create our Consumer’s Dispose() like this:

   1: // Dispose the instance by stopping the consumption, etc.
   2: public void Dispose()
   3: {
   4:     // attempt to stop the process if its currently running
   5:     Stop(DefaultTimeSpanOnStop);
   6:  
   7:     // Try to dispose of consumer task, won't throw if can't.
   8:     _consumer.TryDispose();
   9:  
  10:     // cleanup other stuff...
  11: }

This simplifies our own Dispose() logic by removing the ugly try/catch block.  In addition, we could take the return value from that Task and determine if we need to do anything special if the Task faulted, etc.  But even better, consider if we had an array of Task we wanted to Dispose().

   1: // now we can safely attempt to dispose every task in the array
   2: Array.ForEach(_myTasks, t => t.TryDispose());

If we wanted to do the same without our method, we would have had to try/catch in the delegate on each Dispose().  Or wrap the whole ForEach() call in a try/catch, but then we’d not Dispose() anything after any one throws which would wouldn’t clean up as much as we could immediately.

Summary

This is just one of many extension methods I’ve thrown together for the TPL Task class.  It’s not earth-shattering by any means, but it can help clean up code and avoid accidentally bleeding exceptions unexpectedly if Dispose() throws.

Next time, I’ll talk about the TPL Task family’s Wait() family of methods, and how we can develop TryWait(), TryWaitAny(), and TryWaitAll() extension methods that handle some of the various exceptions that can happen when using those methods.

 

Technorati Tags: , , , , , ,

Print | posted on Thursday, October 6, 2011 7:41 PM | Filed Under [ My Blog C# Software .NET Toolbox ]

Feedback

Gravatar

# re: C#/.NET Toolbox: Creating a "Safer" Task Dispose()

Usually there is no need to dispose the Task. See this thread
http://social.msdn.microsoft.com/Forums/en-US/parallelextensions/thread/7b3a42e5-4ebf-405a-8ee6-bcd2f0214f85

There is a good answer from Stephen Toub.

Otherwise good read. I would most likely remove the check for null in the extension method. Otherwise I have to keep on checking if the task is null before I call TryDispose().
10/8/2011 12:30 AM | Toni
Gravatar

# re: C#/.NET Toolbox: Creating a "Safer" Task Dispose()

@Toni:

Yep, when i first ran into exceptions during Dispose() i ran across that thread, it wasactually the inspiration of my TryDispose(). In particular his line:

"In short, as is typically the case in .NET, dispose aggressively *if* it's easy and correct to do based on the structure of your code."

So this Dispose() will attempt to do so, but also allow you to keep going if it fails. Thus it lets you somewhat aggressively Dispose() but won't fail you if the Task isn't in the right state.

In retrospect I could also put an if check in it to make sure the task is completed before attempting to avoid the exception in most bad cases.

As for the null check, that seems to be a big matter of style and taste for many people. Obviously, you can call an extension method on null references, but for some people this feels like a violation of the method acting like an instance method.

There are cases where I do check and don't check. Generally, my rule of thumb is if the extension method name indicates it could be null, i let it accept null (like if I made a NullSafeLength() on Array).

I could argue TryDispose() should accept it as well since it's a "Try" and should accept failures...

Thanks for the input!
10/8/2011 8:01 AM | James Michael Hare
Post A Comment
Title:
Name:
Email:
Comment:
Verification:
 
 

Powered by: