async/await Pitfalls: Premature Disposal

Since the introduction of async/await, plenty of pitfalls could cause all sorts of errors and deadlocks. Often, these little mistakes were subtle and hard to detect for developers less experienced (less burned) with async/await.

For that reason, Particular Software started early shipping a custom code analyzer with NServiceBus to help detect the very common mistake of missing await statements. Luckily, over the past few years, the tooling for .NET has drastically improved and can more reliably detect many of these mistakes. But still, it’s relatively easy to fall into the remaining pitfalls when not paying close attention for a short moment. I’d argue that some new (fantastic) language features make this even more accessible.

Old mistake, new packaging

Consider this standard error as old as async/await but looks slightly different thanks to the new syntax. In isolation, you might spot the error very quickly:

static Task Main()
{
    using var disposableInstance = new MyDisposableClass();

    return DoSomething(disposableInstance);
}

This code can throw an ObjectDisposedException. Calling await Main() is roughly semantically equivalent to the following snippet, which makes the problem stand out more clearly:

Task result;
using (var disposableInstance = new MyDisposableClass())
{
    result = DoSomething(disposableInstance);
}

await result;

The problem starts with not awaiting the call to DoSomething (one might argue that the problem begins at not naming the method DoSomethingAsync ;-) ) but instead returning the Task. It is a widespread and popular code optimization in intermediary code to avoid paying the price of the compiler-generated state machine for handling async code.

When a method only calls a single asynchronous method, returning the Task is much more efficient than marking the method with async and awaiting the invocation. However, in this case, it will cause the Main method to return immediately after the first asynchronous operation.

With the method completed, the using will also dispose of the MyDisposableClass instance. If the asynchronous code path inside DoSomething now continues and tries to access the reference of MyDisposableClass, it will already be disposed of and therefore raise an exception.

Unpredictable behavior

Whether DoSomething runs into this problem depends on what it does internally. As with throwing Exceptions, the behavior between methods that return Tasks but are actually synchronous (e.g. Task SyncMethod() => Task.CompletedTask;) and truly asynchronous methods (e.g. async Task AsyncMethod() => await Task.Yield();) varies. Consider these two implementations of DoSomething:

static Task DoSomething(MyDisposableClass myDisposableClass)
{
    myDisposableClass.DoStuff();
    return Task.CompletedTask;
}

and

static async Task DoSomething(MyDisposableClass myDisposableClass)
{
    await Task.Yield();

    myDisposableClass.DoStuff();
}

The first implementation will run without any exception in the initially shown Main method because DoSomething will run to completion before it returns to Main to dispose the passed object reference.

The second version will throw an ObjectDisposedException because Task.Yield() will trigger the return to the caller and finish the remaining code of the asynchronous method later.

We need to assume that any real Task returning method will have an asynchronous code exception (like the 2nd version). In reality, a single method can even show both behaviors, entirely depending on runtime conditions:

static Task DoSomething(MyDisposableClass myDisposableClass)
{
    if (myDisposableClass.Validate)
    {
        return InvokeWithExpensiveValidation();

        async Task InvokeWithExpensiveValidation()
        {
            await ExpensiveValidationAsync();
            myDisposableClass.DoStuff();
        }
    }
    else
    {
        myDisposableClass.DoStuff();
        return Task.CompletedTask;
    }
}

An implementation like the presented one can avoid the cost of asynchronous methods when it is expected that the async path is only rarely used. However, it makes it even harder to notice the initial mistake of not awaiting the method call because the code might work just fine most of the times. Until it doesn’t.

Conclusion

The using var language feature is fantastic and much more convenient than having to define using(...){ ... } blocks all over the place. But with asynchronous code, it can be easy to miss the additional using keyword (imagine a scenario where the disposable instance is created at the very beginning).

No compiler or code analyzer detects such cases of “premature disposals” (or missing awaits). It’s crucial to keep an eye out for this and properly await any method that receives a reference to disposable objects.