Is ThreadStatic a leaky abstraction?

Reading Ayende’s post on integrating Rhino Service Bus and RavenDB, one thing that caught my eye was the use of the ThreadStatic attribute to control the lifecycle of a private field:

public class RavenDbMessageModule : IMessageModule
{
    private readonly IDocumentStore documentStore;

    [ThreadStatic]
    private static IDocumentSession currentSession;

    public static IDocumentSession CurrentSession
    {
        get { return currentSession; }
    }

    public RavenDbMessageModule(IDocumentStore documentStore)
    {
        this.documentStore = documentStore;
    }

    public void Init(ITransport transport, IServiceBus serviceBus)
    {
        transport.MessageArrived += TransportOnMessageArrived;
        transport.MessageProcessingCompleted += TransportOnMessageProcessingCompleted;
    }

    public void Stop(ITransport transport, IServiceBus serviceBus)
    {
        transport.MessageArrived -= TransportOnMessageArrived;
        transport.MessageProcessingCompleted -= TransportOnMessageProcessingCompleted;
    }

    private static void TransportOnMessageProcessingCompleted(CurrentMessageInformation currentMessageInformation, Exception exception)
    {
        if (currentSession != null)
        {
            if (exception == null)
                currentSession.SaveChanges();
            currentSession.Dispose();
        }
        currentSession = null;
    }

    private bool TransportOnMessageArrived(CurrentMessageInformation currentMessageInformation)
    {
        if (currentSession == null)
            currentSession = documentStore.OpenSession();
        return false;
    }
}

One of the things that really bothers me about an implementation like this is that intimate knowledge of the threading model of Rhino Service Bus is required to properly implement an IMessageModule implementation properly. The CurrentSession property is static, yet the IDocumentStore member is an instance field. Why does an implementer of a message module need to be concerned about lifecycle of its dependencies?

I don’t really think it should – lifecycle of a dependency should at most be the responsibility of the dependency itself. Or even better, the responsibility of the container. Dependencies shouldn’t care where they are used, because that knowledge would violate the whole “D” in SOLID.

Instead, I’d much rather see something like FubuMVC behaviors. Here’s one with NHibernate to have a session always part of a request there:

    public class SimpleUnitOfWorkBehaviour : IActionBehavior
    {
        readonly IFubuRequest _fubuRequest;
        readonly ISessionFactory _sessionFactory;
        ISession _session;

        public IActionBehavior InnerBehavior { get; set; }

        public SimpleUnitOfWorkBehaviour(ISessionFactory sessionFactory, 
                                                IFubuRequest fubuRequest)
        {
            _sessionFactory = sessionFactory;
            _fubuRequest = fubuRequest;
        }

Note that the behavior doesn’t care about the lifecycle of itself. That’s completely managed by the FubuMVC infrastructure, it’s completely unimportant to the implementor. Then when it comes time to actually create a session, it looks like:

        public void Invoke()
        {
            _fubuRequest.Set(new Lazy<ISession>(() => 
                                  _session ?? (_session = CreateTransactionalSession())));

In the code above, the behavior chain sets a contextual dependency on the IFubuRequest, which, through the magic of child and nested containers, ensures that subsequent behaviors in the execution pipeline have the correct injected ISession from NHibernate. Nowhere in this code do you see any sort of lifecycle management.

The most you see is things like Lazy and in code that uses the dependency, perhaps a lazy Func<T>. But these are only in place because consumers want to direct the scope, not lifecycle.

Lifecycle of dependencies is best served in the framework. Barring that, the container configuration. Consumers and dependencies do not need to change their design to accommodate the lifecycle constraints of the hosted environment. Pushing that into these pieces leaks the abstraction of the hosting environment.

Related Articles:

Post Footer automatically generated by Add Post Footer Plugin for wordpress.

About Jimmy Bogard

I'm a technical architect with Headspring in Austin, TX. I focus on DDD, distributed systems, and any other acronym-centric design/architecture/methodology. I created AutoMapper and am a co-author of the ASP.NET MVC in Action books.
This entry was posted in Dependency Injection, Design. Bookmark the permalink. Follow any comments here with the RSS feed for this post.
  • http://twitter.com/PhatBoyG Chris Patterson

    Great post Jimmy, you make a lot of really great points in it.

    I agree that using a ThreadStatic in this way feels leaky. The are certainly reasons to use ThreadStatic values (particularly in transaction sensitive code) but if they are used they should be abstracted by the framework in question.

  • Corey Kaylor

    You will like the changes coming in RSB v3 then.

  • Mogens Heller Grabe

    I’d agree if we were talking about code that a) we had a lot of, or b) we needed to touch it again and again because stuff would change – like e.g. the requirements for the lifecycle or something like that.

    Ayende’s RavenDB message module, however, is just a tiny piece of infrastructure glue code that should be written once and then forgotten about, which is why the tight coupling to the hosting environment does not matter that much.

  • Pingback: The Morning Brew - Chris Alcock » The Morning Brew #998