Post navigation

Real world async/await defects

The headliner feature for C# 5 was of course the await operator for asynchronous methods. The intention of the feature was to make it easier to write asynchronous programs where the program text is not a mass of inside-out code that emphasizes the mechanisms of asynchrony, but rather straightforward, easy-to-read code; the compiler can deal with the mess of turning the code into a form of continuation passing style.

However, asynchrony is still hard to understand, and making methods that allow other code to run before their postconditions are met makes for all manner of interesting possible bugs. There are a lot of great articles out there giving good advice on how to avoid some of the pitfalls, such as:

These are all great advice, but it is not always clear which of these potential defects are merely theoretical, and which you have seen and fixed in actual production code. That’s what I am very interested to learn from you all: what mistakes were made by real people, how were they discovered, and what was the fix?

Here’s an example of what I mean. This defect was found in real-world code; obviously the extraneous details have been removed:

The network device synchronously downloads the serialized state of a Frob. When it is done, the delegate stored in OnDownload runs synchronously, and is passed the state that was just downloaded. But since it is itself asynchronous, the event handler starts deserializing the state asynchronously, and returns immediately. We now have a race between GetFrob returning null, and the mutation of closed-over local result, a race almost certainly won by returning null.

If you’d rather not leave comments here — and frankly, the comment system isn’t much good for code snippets anyways — feel free to email me at eric@lippert.com. If I get some good examples I’ll follow up with a post describing the defects.

31 thoughts on “Real world async/await defects”

Deadlocking when using HttpClient in an ASP.net application that isn’t async all the way through. Basically a non-async filter that calls an aync method (using await), which then in turns calls the HttpClient.PostAsync (again using await).

Apparently it’s because of the SynchronizationContext that’s running the ASP.net thread being captured and thus becoming a contested resource.

Had issues with deadlocking an MVC app. Often appeared when using Task-returning methods where results in the method are not yet needed (so no async modifier). Higher up someone forgets to await: task fired, never returns to thread; deadlock.

Also had some problems with HttpContext.Current being null in TaskFactory.StartNew and Task.Run blocks. Those run on a new thread without the use of sync-context. Proper use of async C# is without those methods. If you need a Task.Run/TaskFactory.StartNew: you’re doing something wrong.

Indeed, I’m mining those as well. But something I’ve noticed about the bugs on SO is that someone will post code which (1) illustrates their question, and (2) contains an async bug. When the async bug is pointed out they frequently say “oh, I know, this was just test code that I mocked up to get code for the real question working”. On the one hand, that’s a real mistake someone made, but on the other hand, it’s not production-quality code. If you have a specific example of a “distinct issue” I’d love to hear it.

Deadlock from blocking on the result of an async operation (several hours to track down).

In my case it was because a dependency being injected through the IoC container was loaded through an async operation (i.e. read current user from DB), but since the IoC container doesn’t understand/support async we needed the actual result. Hence there was a call to Task.Result. That leads to a deadlock due to the ASP.NET synchronization context as described in the “Don’t block on async code” article you linked. In fact, that article is part of what we read to understand the problem.

I’ve noticed people people awaiting Task.WhenAny on a collection, removing the returned task, and repeating that until the collection is empty. Gives the right result, but has quadratic costs. The correct solution is to use OrderByCompletion or a variant thereof, which is linear time.

An example of it happening in the wild: Jon Skeet’s “EduAsync part 16” post [1]. I emailed him about it, and he did a great follow up explaining OrderByCompletion in part 19 [2].

I really don’t understand why calling async API synchronously is such a pain. I had to do that in a legacy app and I can’t even imagine how many potential deadlocks I have created. Sometimes you already have a thread or the background worker rolling and there is no sync API in a new library. So what do we do?

ran into point 3. “My async method never completes.”, from Psychic debugging of async methods – basically c# xaml rt app with its mvvm commands (non async) defined in a pcl library, that in turn called an async await method that posted to a web service, that article would have saved me hours! – bookmarking them now… thanks

Not strictly related to async, but more a general thing: With async/await being so easy to create, there is no easy way to pass state “on the side”.

Example, existing code assume it’s run in a single thread. Caller puts stuff into a global static class, then calls service. Service grabs stuff from the global static class and uses it in the process.

Does this sound horrible? Global mutable state? It is, but it has been a cornerstone of many ASP.net Applications for a long while – HttpContext.Current. (Or anything like that, like CallContext.Get/SetData)

If I change the service to be async, everything may appear to work. Until the scheduler decided to run the async task on another thread, at which point HttpContext.Current may be null, or even another request (not sure if that last can happen), but only some of the time, possibly even only under high load, thus it’s a nightmare to troubleshoot.

In fact, that’s what I’ve been looking at for a day or two, because with async Web Applications, anything static to get the current request is unusable. So the application has to be structured top to bottom to support callers passing through Per-Request information all the way down the chain.

I have opened a StackOverflow question regarding my specific issue, but in general, async/await requires a big rethinking when used in ASP.net apps that are used to statics.

Not exactly an `async/await` issue but one related to the task cancellation status propagation and implicit C# type inference between Func and Action for Task.Run (which probably could lead to obscure bugs): http://stackoverflow.com/q/24359761/1768303

* Impossible to have a (immutable – or copy-on-write) context, which flows through awaits but not any other thread-jumping constructs (like Task.Run) – essentially we needed an async equivalent of thread-local storage, which would be flown only through awaits. The only approach is to abuse SynchronizationContext (e.g. like ASP.NET does for httpcontext.current) – which means disallowing ConfigureAwait(false) everywhere OR writing our own awaiter, which would flow such context – which means writing a build time style-check rule to never await a simple Task, which isn’t wrapped in the custom awaiter.

* Impossible to share the same code for synchronous and asynchronous clients – e.g. for large codebase, it’s not possible to update all callers to be async at the same time. So if I create a new library, which returns Task, the synchronous code must call it using Task.Run to not get into the dead-lock mentioned above (possibly flowing any “thread-local-storage”-like context manually). Essentially I wish the compiler (or runtime) would just automagically convert all async functions into synchronous ones by deleting the “async” and “await” keywords and run the code synchronously without spinning up a new thread (I know it’s impossible 🙂 )

In Windows Forms, we use async/await to retrieve data from a WCF service. Typically these calls are triggered from event handlers and the UI is updated when a response is received.

This seems like a natural use of async/await (actually it’s just like the Example in http://msdn.microsoft.com/en-us/library/hh156528.aspx) and it works fine in most cases. However, when the Control / Form is closed and disposed before the await call returns, the UI modification might fail with ObjectDisposedException or worse (NullReferenceException etc.)

Because this only occurs when the user closes the Form while the await call is running, and also because most UI modification complete without exception even on a disposed Form, it is quite rare to spot this problem, but with hundreds of such calls in place, it’s quite an issue.

Possible (but problematic) solutions:
– check if the form is disposed after every await.
– disallow closing/disposing the Form during every await.

How about doing the UI updates through a `TryBeginInvoke` extension method which will silently do nothing (swallowing if necessary the not-always avoidable exception) if invoked upon a form or control which has been disposed?

If already using a special method for UI modification, I believe it is better to check whether the form is disposed before doing the modification, rather than trying it blindly and catching System.Exception.

Problems of this approach:
1. Doing all UI modifications by calling a special method explicitly every time adds same level of obscurity to the code as the classical await-less approach. What’s the point of using async/await?
2. Too easy to forget and do UI modifications directly (since it works in 99,9% cases) – some custom syntax checking might help.

The obscurity of this approach could be reduced a bit by transforming the TaskAwaiter with an extension method, something like await SomethingAsync().ContinueIfNotDisposed(this), but not all the code executed after await does UI modifications in the form alone, some code might have side-effects outside the disposed form and some of these side effects might be desired, even when the form is disposed. So there should be a distinction between UI modifications and other post-processing.

I realize this thread is a little old, but I am dealing with similar issues right now. Asynchronous delegates are problematic in my mind because of disposal, application closing, and exceptions when such delegates are essentially “fire-and-forget” and have no inherent means of dealing with exceptions. Unfortunately, use of async in an application typically propagates all the way up to an event handler which ends up being … an async delegate. I feel like something is missing in the language or the standard async patterns and I rarely see these issues discussed. Perhaps I am missing something fundamental.