One particularly nasty problem though that we've hit due to inlining await continuations:
It creates an unwanted dependency between unrelated code when multiple folks await on the same task. We've seen very subtle deadlocks because the inlining behavior is such that when multiple continuations are added to a task and that task completes, the continuations are executed in order on the original completing thread. This means that the second continuation cannot run till the first continuation completes, and when the second one will free a resource that the first one requires (and synchronously blocks for) that means a deadlock will happen. It's particularly hard to discover because the second continuation isn't on any callstack. Only deep heap inspection reveals the problem.
It is extremely difficult in code inspection to predict these bugs in our code as well. We're developing roslyn Analyzers to help us, but their effectiveness is limited and I'm afraid we're somewhat bug driven to add await Task.Yield(); after awaiting on shared Tasks and before we do synchronously blocking things in order to avoid the problem.
@stephentoub: if I could change anything in the Framework, it would be that only the first continuation can be executed inline and all subsequent ones would execute on their appropriate TaskScheduler or SynchronizationContext. I believe this would preserve most of the perf wins that are so important because most scenarios (that I know of) have just one continuation, but it would avoid creating subtle dependencies between unrelated tasks.
@stephentoub said:
Out of all of the changes discussed in this thread, this one is I think the most viable. There are still possible breaks from it, but I expect they would be relatively rare. Something to think further about...
The VS IDE team has continued to hit such deadlocks several times since we first diagnosed the problem. Please make this change, or perhaps add an AppContext switch to opt into such behavior.
_Originally posted by @AArnott in https://github.com/dotnet/corefx/issues/2454#issuecomment-302120059_
We're developing roslyn Analyzers to help us
If people go to such length to deal with this then the problem must be very bad. This is my impression as well.
the first continuation can be executed inline and all subsequent ones would execute on their appropriate TaskScheduler or SynchronizationContext
The downside here is that reentrancy becomes even more non-deterministic. It now matters in what order continuations are being attached. This makes bugs more spurious and harder to reproduce.
Ordinary .NET developers cannot effectively deal with such bugs and might simply be unable to fix them. These bugs are too hard to find and understand.
Let's also conclude a fact that is evident by now: Professional, highly experienced developers are practically unable to correctly use the TPL. It is humanly impossible.
add an AppContext switch to opt into such behavior
Is it feasible to turn off reentrancy by default and have people opt in on a case-by-case basis?
Let's also conclude a fact that is evident by now: Professional, highly experienced developers are practically unable to correctly use the TPL. It is humanly impossible.
FWIW, it's hyperbole like this that causes me to start ignoring these discussions.
@stephentoub now that I read it again it seems I got a bit too excited :laughing:. I apologize for causing annoyance.
Most helpful comment
@stephentoub now that I read it again it seems I got a bit too excited :laughing:. I apologize for causing annoyance.