Today I’m once again asking for your help.
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:
- Psychic debugging of async methods
- Pitfalls to avoid when passing async lambdas
- Async gotchas
- Don’t mix await and compound assignment
- Don’t block on async code
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:
Frob GetFrob()
{
Frob result = null;
var networkDevice = new NetworkDevice();
networkDevice.OnDownload +=
async (state) => { result = await Frob.DeserializeStateAsync(state); };
networkDevice.GetSerializedState(); // Synchronous
return result;
}
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.
Fabulous adventures in coding 
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.
Basically, exactly this:
http://stackoverflow.com/questions/17859898/async-await-handler-deadlock
Awesome, thanks!
I found issues when using OperationContextScope to call async WCF services:
http://stackoverflow.com/questions/18284998/pattern-for-calling-wcf-service-using-async-await
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.
Aren’t the many async questions per day on Stack Overflow enough material? I answer multiple a week. It’s like 10 distinct issues make for 95% of the bugs.
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.
http://stackoverflow.com/search?q=%5Basync-await%5D+wait+deadlock
That’s a common one. It boils down to people mistaking await to be a “new and cool” way to invoke Task.Result.
Going through http://stackoverflow.com/questions/tagged/async-await most questions are irrelevant for you. I agree. I’m surprised because I find myself pointing out the same set of issues all day.
One common thing is people hooking up a continuation and throwing away the continuation task. They wanted to wait for it or await it but they threw it away and now they have a race.
The sync-over-async and async-over-sync anti-patterns are very common.
—
A method like async Task X() { return ComputeIntensive(); }.
Here, he’s mistaking async for starting a thread. More often, they see await as starting a thread.
—
var t1 = GetTask();
var t2 = GetTask();
await t1;
await t2;
Here, he has lost error information from t2 if t1 faults. Might be a bug hidden.
—
Using new Task() which is almost always a mistake. Often, the task stays cold.
Using Thread.Abort to cancel tasks.
Using a non-volatile bool to cancel tasks.
Not sure if you are interested in those because some of them are only accidental async/await defects. Await really doesn’t cause too many common defects. I can list more TPL-based defects if you like.
tobi, I am a researcher and building various code analyzers to detect async and TPL anti-patterns. Could you please send me a ‘Hi email’ to kuyardim@gmail.com? I have some questions for you.
Defining fake async methods and not using ‘real’ async methods:
// Fake async
var task = Task.Factory.StartNew(()=> ReadFromDisk(fname,s));
//…
private string ReadFromDisk(string fname, string s)
{
return System.IO.File.ReadAllText(AppDomain.CurrentDomain.BaseDirectory + fname);
}
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.
The lack of natural support for async + yield in C# and LINQ. E.g.: http://stackoverflow.com/q/23854102/1768303, or http://stackoverflow.com/q/24227235/1768303. One other thing, apparently the exception propagation behavior is not quite intuitive, especially for “async void” methods (http://stackoverflow.com/a/21082631/1768303, http://stackoverflow.com/a/22395161/1768303). That said, I personally find it logical and well designed. Also, while learning async/await, I myself posted quite a few questions, some of them might be interested: http://stackoverflow.com/search?tab=votes&q=user%3a1768303%20%5basync-await%5d%20is%3aquestion
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].
1: http://msmvps.com/blogs/jon_skeet/archive/2011/11/22/eduasync-part-16-example-of-composition-majority-voting.aspx
2: http://msmvps.com/blogs/jon_skeet/archive/2012/01/16/eduasync-part-19-ordering-by-completion-ahead-of-time.aspx
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?
Pingback: Dew Drop – June 17, 2014 (#1799) | Morning Dew
Simplified:
public Task Foo()
{
…
return new TaskCompletionSource().Task;
}
Somewhere else
await Foo(); // wait forever, never being able to see pending tasks in VS.
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
I went with option 2 from ‘Don’t block on async code’
It’s been a while since it happened, but code like this caused me a night of work a while ago:
private void CopyWithCancellation(Stream from, Stream to, CancellationToken ct)
{
var task = Task.Run(async () => await from.CopyToAsync(to, 8192, ct));
task.Wait(); // potential deadlock
}
(solution was “await from.CopyToAsync(to, 8192, ct).ConfigureAwait(false)”)
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.
http://stackoverflow.com/questions/24295653/
Pingback: Top 10 links of the Week #20 | 06/20/2014 | The SoftFluent Blog
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
Some issues working with large legacy mainly synchronous code:
* 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 🙂 )
Don’t know if this is what you mean. Maybe it just exemplifies my stupidity/ignorance of how TAP works. I found the switch from TPL more difficult than I’d anticipated:
http://stackoverflow.com/questions/24389549/is-async-await-inherently-slower-than-tpl-task-continuewith
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.
It was supposed to make things easier, right? 🙂
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.
In the method GetFrob, you just defined networkDevice.OnDownload. When does it start to run? That’s the key to your question.
Pingback: Async and await in C# 5.0: purpose, practical considerations, and links for further reading | the coding ladder
Pingback: Software development bookmarks | Ordo Rerum
Pingback: Asynchronous : async await Task | Ordo Rerum
Pingback: Jazeera Airways modifies bank financing needs for Airbus deal to $1… - Her Game Life