A site devoted to discussing techniques that promote quality and ethical practices in software development.

Tuesday, June 14, 2011

Safe way to close down a ClientBase<T> object

Recently a good friend of drew my attention to a situation I consider a bad disposable object implement and pointed me kindly to an MSDN article offering some words of advice and work around. This saved me considerable research effort. I will address the bad disposable object implementation later.

The problem is that when using the using statement to dispose a ClientBase(of T) object, the base class of WCF proxy stub, you can run into a problem where leaving the using scope the exception thrown in the Dispose() can mask out application exception that causes the execution to leave the scope in the first place. This denies the client the knowledge of the application exception.

The MSDN article explains this phenomenon very clearly and provides an excellent recommendation to handle this problem. The recommended work around places higher value on the application exception as compared to the one that is likely to happen when the Dispose(), or rather Close(), is called. The recommendation uses a multi-level of catches and then use either ClientBase(of T).Close() or its no-throw equivalent ClientBase(of T).Abort() strategically.

While the work around code presented in the article is written in a style that shows clearly how to tackle the problem, it is rather clumsy to use in real life. It expects too much from developer to know when to called Abort() and when to call Close(). Furthermore, it changes the shape of the code drastically to a somewhat awkward and unfamiliar form. It therefore leaves room for improvement and a simple class designed to meet the following objectives is described below:
  • Perform automatic closure of the ICommunicationObject using the same elegant form of the using statement.
  • Prevent the application exception, the one that is thrown inside the using statement scope, from being masked and passing it to the client unhindered.
  • If no application exception, it allows exception that can be generated during the disposable of the ICommunicationObject to pass to the client.
I want to use a pattern like this:
public void GetSomeData( .... ) {
   using( SomeHelper helper = new SomeHelper( new CalculatorClient() ) ) {
      GetData( helper.Instance() );
   }

This SomeHelper class must meet the above objectives.

Recognizing the issue at hand is how to handle the System.ServiceModel.ICommunicationObject, one of the interfaces implemented by ClientBase(of T), I construct the following disposable generic class called SafeCommunicationDisposal.

using System;
using System.ServiceModel;

namespace DisposingCommunicationObject
{
  public class SafeCommunicationDisposal<T> : IDisposable where T:ICommunicationObject
  {
    public T Instance { get; private set; }
    public bool IsSafeToClose{ get; private set; }
    public void SafeToClose() {
      this.IsSafeToClose = true;
    }
    
    public SafeCommunicationDisposal (T client) {
      this.Instance = client;
    }
    
    bool disposed;
    public void Dispose() {
      Dispose( true );
      GC.SuppressFinalize( this );
    }
    
    private void Dispose( bool disposing ) {
      if( !this.disposed ) {
        if( disposing ) {
          Close();
        }
        this.disposed = true;
      }
    }
    
    private void Close() {
      if( IsSafeToClose ) {
        Instance.Close();
      } else {
        Instance.Abort();
      }
    }
  }
}

With this class I can rewrite the naive usage of using the using statment from this:
using( CalculatorClient client = new CalculatorClient() ) {
      // use client to do work
  }
to this form which does not mask out application exception on leaving the using scope and yet closing down the communication object properly:
using( SafeCommunicationDisposal<CalculatorClient> d = 
            new SafeCommunicationDisposal<CalculatorClient> ( new CalculatorClient() ) ) {
     CalculatorClient client = d.Instance;
     // use the client to do work. If it throws exception
     // it will be passed over to the client and not masked by exception 
     // when client.Close() is executed.
     d.SafeToClose(); // This tells SafeCommunicationDisposal to use ICommunicationObject.Close()
} 

The fundamental principle is to tell SafeCommunicationDisposal what to use to close down the ICommunicationObject when execution leaves the using scope.

If it leaves by normal means, the SafeCommunicationDisposal.SafeToClose() sets a flag to tell the SafeCommunicationDisposal.Dispose() to use ICommunicationObject.Close(). If Close() throws an exception, it is then passed to the user.

If it leaves as a result of an application exception, SafeCommunicationDisposal.SafeToClose() will not be executed and SafeCommunicationDisposal.Dispose() will use ICommunicationObject.Abort(), a no-throw method to shut down the communication gracefully. This does not mask out the application exception.

If a developer forgets to call SafeCommunicationDisposal.SafeToClose(), the SafeCommunicationDisposal.Dispose() takes a more defensive route of calling the ICommunicationObject.Abort().

I believe this class can help to make the code more readable, maintainable and using a familiar using pattern to correct the  naive usage that minimize code change.  

The moral of this story is that if you are designing a disposable object, it is wise to heed the following advice "Avoid throwing an exception from within Dispose except under critical situations where the containing process has been corrupted (leaks, inconsistent shared state, etc.).". This is a well known no-no in unmanaged C++.

No comments:

Blog Archive