Sharp Learning Curve Ahead

Sometimes I think a better title for my blog would be Behind The Learning Curve… but that’s a topic for another time.

Recently we started seeing rolling WCF channel request timeout exceptions in our production environment. It seemed the only way to get the services back in working order was to cycle the app pool the services were hosted in. You don’t have to be Juval Lowy to realize that this points to a misuse of WCF. But where exactly? Finding the issue was going to be challenging since this particular project has been a case study in feature-creep and has had a constantly changing team (we used a lot of consultants for temporary staffing) let’s just say there’s a significant amount of technical debt in the form of inconsistently applied architectural approaches and implementations.

A large part of this is entirely my fault. I didn’t take time to make sure every team member on the project was familiar with the proper way to close/abort/dispose the proxy. Not only that but I have been wrong (twice now) regarding how to make absolutely sure you’re not going to create memory leaks by not correctly handling the proxy when you’re finished using it.Each time my understanding of the pattern improved, I would update the team, update the code I’d written and try to make more helpers available to handle the issue.

My first mistake was thinking that all I had to do was close the proxy in a finally block, like this:

using (var service = new ServiceClientProxy())
{
    try
    {
        service.SomeCall();
    }
    catch
    {
        //handle the exception
    }
    finally
    {
        service.Close();
    }
}

It wasn’t long until I discovered on my own that this is very, very wrong. If for any reason the channel faults, you’ll get another exception in your finally block for trying to close a faulted channel. You have to call abort on the proxy once the state becomes faulted. So I updated my code and the team to let them know about that finding and (foolishly) made the assumption all would be well after updating my helper classes and direct uses of the clients as above to look like the pattern below:

using (var service = new ServiceClientProxy())
{
    try
    {
        service.SomeCall();    
    }    
    catch    
    {
        //handle the exception    
    }    
    finally    
    {        
        if(service.State != CommunicationState.Faulted)
        {
            service.Close();
        }
        else
        {
            service.Abort();
        }
    }
}

But as it turns out, that’s still not a correct/complete pattern because it still doesn’t address the fact that when you call Close on the service proxy, you can get an exception. (thanks Evan Hoff for pointing this one out to me recently) So the correct pattern is actually:

using (var service = new ServiceClientProxy())
{
    try
    {
        service.SomeCall();
    }
    catch
    {
        //handle the exception
    }
    finally
    {
        if(service.State != CommunicationState.Faulted)
        {
            try
            {
                service.Close();
            }
            catch
            {
                service.Abort();
            }
        }
        else
        {
            service.Abort();
        }
    }
}

Now, there are a lot of different ways to keep from repeating this finally block all over the place. For a temporary fix, I introduced the following extension method (because I’m lazy and shameless)

public static class ServiceHelperExtensions
{
    public static void DisposeCorrectly(this ICommunicationObject wcfProxy)
    {
        if(wcfProxy.State != CommunicationState.Faulted)
        {
            try
            {
                service.Close();
            }
            catch
            {
                service.Abort();
            }
        }
        else
        {
            service.Abort();
        }
    }
}

And while this was a quick way for me to find all uses of the service client proxies and introduce the following code very quickly (thanks to ReSharper)

using(var service = new ServiceClientProxy())
{
    try
    {
        service.Call();
    }
    finally
    {
        service.DisposeCorrectly();
    }
}

In the future I don’t plan on using this approach long term and instead have been working on a dynamic WCF proxy class so that we can kiss the generated proxies goodbye forever (or so I hope).

Why admit to all these pretty careless mistakes and effectively award myself oodles of doofus points? Well, for one, no one person on the team was using the correct pattern on their own. That’s not because we have a bad team. I actually believe we have a really great team. And if a team of good developers all make the same mistakes, chances are there are plenty developers out there who are making similar mistakes regarding the proper handling of the proxy and maybe they’ll stumble across this blog and realize “Hey, I’m not alone, Alex was making this mistake way before I was!”.

And of course, I’m also taking my lumps so I can pass some out. To the group of disconnected ne’er-do-wells at Microsoft who are responsible for the no-ass implementation of IDisposable in WCF:

Dear WCF Team,

Why do you hate me so much? I mean, so much of WCF is actually pretty flipping sweet. That whole two-way binding thing you guys did, that’s awesome! My buddy Jim is really excited about that. And making it possible to get the binding and endpoint from a meta data exchange on the service is brilliant. But why in the name of sweet baby Bill Gates didn’t you handle this mess of crap in your IDisposable implementation? Things that implement IDisposable (especially things in the FRAMEWORK) should in fact be disposed of and not create memory leaks if I don’t perform the secret hand-shake of try/if/try/catch/else/finally.

You crapped on my code and broke my heart WCF team.

Sincerely,
Alex

Tags: