Designing an interface always requires careful considerations of how it will be used. Scott Meyers elegantly catches the entire problem in one sentence in his book Effective C++:

Make interfaces easy to use correctly and hard to use incorrectly.

The people at Microsoft who were in charge for the WCF client code generation either hadn’t read that book or didn’t understand it. They have made an interface that is counter intuitive and hard to use when it comes to disposing of the WCF client.

In my previous post IDisposable and using in C# I wrote that “Always call Dispose on objects implementing IDisposable“. This is true, as long as the class implements IDisposable in a reasonable way. Unfortunately the WCF client doesn’t. The MSDN docs presents the problem.

using (CalculatorClient client = new CalculatorClient())
{
    ...
} // <-- this line might throw
Console.WriteLine(
  "Hope this code wasn't important, because it might not happen.");

That is definitely an example of an interface that is hard to use. The hidden call to Dispose might throw with a strange exception which sometimes even hides the real error. There is a simple solution though that makes the above code work as expected.

The Problem

The problem with the generated code is that System.ServiceModel.ClientBase<T>.Dispose() unconditionally calls Close(). This will try to make a graceful shutdown of the communications channel with the server. If the channel is in a faulted state this is not allowed and an exception is thrown. The correct thing to do in that case is to call Abort() instead.

Requirements on a Solution

I think that there are a number of requirements that a good solution to this problem should fullfill in order to be easy to use correctly and hard to use incorrectly.

• An ordinary using statement should work and behave correctly.
• No special code with passing method names as lambdas should be needed from the client.
• Code adjustments should be done in one place to the client, not requiring any changes in the code utilizing the client.

The Solution

The solution is to create a partial class for the WCF client, containing an implementation of IDisposable. This is possible because the generated service client itself isn’t implementing IDisposable, it is implemented by the System.ServiceModel.ClientBase<T> base class.

1
2
3
4
5
6
7
8
9
10
11
12
13
14
15
16
17
18
19
20
21
22
23
24
25
26
27
28
29
30
31
32
33
34
35
36
37
38
39
40
41
42
43
44
45
46
47
48
49
50
51
52

/// <summary>
/// Calculator Client
/// </summary>
public partial class CalculatorClient : IDisposable
{
    #region IDisposable implementation
 
    /// <summary>
    /// IDisposable.Dispose implementation, calls Dispose(true).
    /// </summary>
    void IDisposable.Dispose()
    {
        Dispose(true);
    }
 
    /// <summary>
    /// Dispose worker method. Handles graceful shutdown of the
    /// client even if it is an faulted state.
    /// </summary>
    /// <param name="disposing">Are we disposing (alternative
    /// is to be finalizing)</param>
    protected virtual void Dispose(bool disposing)
    {
        if (disposing)
        {
            try
            {
                if (State != CommunicationState.Faulted)
                {
                    Close();
                }
            }
            finally
            {
                if (State != CommunicationState.Closed)
                {
                    Abort();
                }
            }
        }
    }
 
    /// <summary>
    /// Finalizer.
    /// </summary>
    ~CalculatorClient()
    {
        Dispose(false);
    }
 
    #endregion
}

The interesting parts starts on line 28. Don’t call Close() if we are in a faulted state. That would just throw an exception anyways. Then, in a finally block check if the state is anything but closed and in that case call Abort(). The code is inspired by a Stack Overflow answer by Matt Davis.

Reimplementing the IDisposable interface in a derived class is usually something to be avoided, but I think it is okay in this case as the inheritance is more of an implementation inheritance than an inheritance meant for the consumer of the class.

Motivation

With this solution I’ve tried to take everything needed into consideration.

• Anyone utilizing the client can wrap it into an ordinary using block.
• If the client is in a clean state a graceful close is done.
• If the client is in a faulted state a hard abort is done.
• If an exception is thrown by a service method the client is placed in the faulted state. The Dispose implementation above will close down the the channel without throwing another exception that hides the one thrown by the service method.
• If the client is in a clean state and the graceful close fails, an exception is thrown and the client is closed by abort.

If there is anything I’ve missed, please comment on it and help make the code better!

Other Alternatives

While I was looking for a solution to this problem I found a number of alternatives. The first one is Microsoft’s own suggestion at MSDN.

try
{
    ...
    client.Close();
}
catch (CommunicationException e)
{
    ...
    client.Abort();
}
catch (TimeoutException e)
{
    ...
    client.Abort();
}
catch (Exception e)
{
    ...
    client.Abort();
    throw;
}

It is a pain to write this every single time the client is used. I would try to avoid it and I know why it is important. In a larger team there will always be developers simply ignoring this.

Another suggestion is to write a helper taking a lambda expression. All calls to the service are replaced by calls to the helper function, passing in the call to the client as a lambda.

Service<IOrderService>.Use(orderService=>
{
    orderService.PlaceOrder(request);
}

I think that this obfuscates the calling code a bit too much, but it gives more freedom in designing the helper method. Handling a return value makes the code even harder to read. If you really want to fine tune the client session handling, this is probably the way to go, or you’d have to write a custom client wrapper class.