Reputation: 3451
Environment: Windows Server 2012, .net 4.5, Visual Studio 2013.
Note: not UI Application (so not related to famous async/await/synchronizationcontext problem)(reference: http://channel9.msdn.com/Series/Three-Essential-Tips-for-Async/Async-library-methods-should-consider-using-Task-ConfigureAwait-false-)
It was a typo which caused deadlock. I have pasted below sample snippet (pseudo) which caused deadlock. Basically, instead of composing whenall based on 'childtasks', I did it on 'outer tasks' :(. Looks like I shouldn't write 'async' code while watching TV :).
And I have left the original code snippets as it is to give the context for Kirill's response as it indeed answers my first question (difference with async/await and unwrap in my previous two code snippets). The deadlock kind of distracted me to see the actual issue :).
static void Main(string[] args)
{
Task t = IndefinitelyBlockingTask();
t.Wait();
}
static Task IndefinitelyBlockingTask()
{
List<Task> tasks = new List<Task>();
Task task = FooAsync();
tasks.Add(task);
Task<Task> continuationTask = task.ContinueWith(t =>
{
Task.Delay(10000);
List<Task> childtasks = new List<Task>();
////get child tasks
//now INSTEAD OF ADDING CHILD TASKS, i added outer method TASKS. Typo :(:)!
Task wa = Task.WhenAll(tasks/*TYPO*/);
return wa;
}, TaskContinuationOptions.OnlyOnRanToCompletion);
tasks.Add(continuationTask);
Task unwrappedTask = continuationTask.Unwrap();
tasks.Add(unwrappedTask);
Task whenall = Task.WhenAll(tasks.ToArray());
return whenall;
}
Code Snippet which waits indefinitely when 'unwrapped' continuation task has been added to aggregated/chain of tasks I have pasted below pseudo (pattern/idiom blocks in my actual app - not on sample) code snippet (#1), which indefinitely waits when 'unwrapped' task has been added to the list. And VS Debugger 'Debugger + windows + threads' window shows the thread is simply blocking on ManualResetEventSlim.Wait.
Code Snippet which works with async/await, and removal of unwrapped task Then I removed (randomly while debugging), this unwrap statement and used async/await in the lambda (please see below). Surprisingly it works. But I am not sure why :(?.
Questions
Aren't using unwrap and async/await serve the same purpose in below code snippets? I simply preferred snippet #1 initially as I just want to avoid too much generated code as the debugger is not so friendly (especially in error scenarios where the exception is propagating via chained tasks - the callstack in exception shows movenext rather than my actual code). If it is, then is it a bug in TPL?
What am I missing? which approach is preferred if they are same?
Note on Debugger + Tasks windows 'Debugger + Tasks' window does not show any details (note it is not properly (at least my understanding) working in my environment as it never shows unscheduled tasks and oribitarily shows active tasks)
Code snippet which indefinitely waits on ManualResetEventSlim.Wait
static Task IndefinitelyBlockingTask()
{
List<Task> tasks = new List<Task>();
Task task = FooAsync();
tasks.Add(task);
Task<Task> continuationTask = task.ContinueWith(t =>
{
List<Task> childTasks = new List<Task>();
for (int i = 1; i <= 5; i++)
{
var ct = FooAsync();
childTasks.Add(ct);
}
Task wa = Task.WhenAll(childTasks.ToArray());
return wa;
}, TaskContinuationOptions.OnlyOnRanToCompletion);
tasks.Add(continuationTask);
Task unwrappedTask = continuationTask.Unwrap();
//commenting below code and using async/await in lambda works (please see below code snippet)
tasks.Add(unwrappedTask);
Task whenall = Task.WhenAll(tasks.ToArray());
return whenall;
}
Code snippet which works with async/await in lambda rather than unwrap
static Task TaskWhichWorks()
{
List<Task> tasks = new List<Task>();
Task task = FooAsync();
tasks.Add(task);
Task<Task> continuationTask = task.ContinueWith(async t =>
{
List<Task> childTasks = new List<Task>();
for (int i = 1; i <= 5; i++)
{
var ct = FooAsync();
childTasks.Add(ct);
}
Task wa = Task.WhenAll(childTasks.ToArray());
await wa.ConfigureAwait(continueOnCapturedContext: false);
}, TaskContinuationOptions.OnlyOnRanToCompletion);
tasks.Add(continuationTask);
Task whenall = Task.WhenAll(tasks.ToArray());
return whenall;
}
Callstack which shows blocking code
mscorlib.dll!System.Threading.ManualResetEventSlim.Wait(int millisecondsTimeout, System.Threading.CancellationToken cancellationToken) Unknown
mscorlib.dll!System.Threading.Tasks.Task.SpinThenBlockingWait(int millisecondsTimeout, System.Threading.CancellationToken cancellationToken) Unknown
mscorlib.dll!System.Threading.Tasks.Task.InternalWait(int millisecondsTimeout, System.Threading.CancellationToken cancellationToken) Unknown
mscorlib.dll!System.Threading.Tasks.Task.Wait(int millisecondsTimeout, System.Threading.CancellationToken cancellationToken) Unknown
mscorlib.dll!System.Threading.Tasks.Task.Wait() Unknown
Upvotes: 6
Views: 3432
Reputation: 2195
Your scheduler can schedule intermediate continuation on the blocked thread.
Original Source: http://blog.stephencleary.com/2012/07/dont-block-on-async-code.html
Deadlock
Here’s the situation: remember from my intro post that after you await a Task, when the method continues it will continue in a context.
In the first case, this context is a UI context (which applies to any UI except Console applications). In the second case, this context is an ASP.NET request context.
One other important point: an ASP.NET request context is not tied to a specific thread (like the UI context is), but it does only allow one thread in at a time. This interesting aspect is not officially documented anywhere AFAIK, but it is mentioned in my MSDN article about SynchronizationContext.
So this is what happens, starting with the top-level method (Button1_Click for UI / MyController.Get for ASP.NET):
The top-level method calls GetJsonAsync (within the UI/ASP.NET context). GetJsonAsync starts the REST request by calling HttpClient.GetStringAsync (still within the context). GetStringAsync returns an uncompleted Task, indicating the REST request is not complete. GetJsonAsync awaits the Task returned by GetStringAsync. The context is captured and will be used to continue running the GetJsonAsync method later. GetJsonAsync returns an uncompleted Task, indicating that the GetJsonAsync method is not complete. The top-level method synchronously blocks on the Task returned by GetJsonAsync. This blocks the context thread. … Eventually, the REST request will complete. This completes the Task that was returned by GetStringAsync. The continuation for GetJsonAsync is now ready to run, and it waits for the context to be available so it can execute in the context. Deadlock. The top-level method is blocking the context thread, waiting for GetJsonAsync to complete, and GetJsonAsync is waiting for the context to be free so it can complete. For the UI example, the “context” is the UI context; for the ASP.NET example, the “context” is the ASP.NET request context. This type of deadlock can be caused for either “context”.
Preventing the Deadlock
There are two best practices (both covered in my intro post) that avoid this situation:
In your “library” async methods, use ConfigureAwait(false) wherever possible. Don’t block on Tasks; use async all the way down.
Upvotes: 0
Reputation: 3451
I have accepted Kirill's answer as the actual answer as it helped me to resolve the problem. Here I am adding few details which probably directly address both questions in concise manner as now I have concise repro for deadlock as well (please see edited version of the question):
a. deadlock is happening because contianuation task is waiting on all outer tasks which contains proxy of the 'continuation task:)'
b. I have pasted the await version of deadlock for reference.
static void Main(string[] args)
{
Task withUnwrap = Unwrap_IndefinitelyBlockingTask();
Task<Task> withAwait = AwaitVersion_IndefinitelyBlockingTask();
withAwait.Wait();
//withUnwrap.Wait();
}
static async Task<Task> AwaitVersion_IndefinitelyBlockingTask()
{
List<Task> tasks = new List<Task>();
Task task = FooAsync();
tasks.Add(task);
Task<Task<Task>> continuationTask = task.ContinueWith(async t =>
{
//immediately returns with generated Task<Task> return type task
await Task.Delay(10000);
List<Task> childtasks = new List<Task>();
////get child tasks
//now INSTEAD OF ADDING CHILD TASKS, i added outer method TASKS. Typo :(:)!
//!!since we added compiler generated task to outer task its deadlock!!
Task wa = Task.WhenAll(tasks/*TYPO*/);
await wa.ConfigureAwait(continueOnCapturedContext: false);
return wa;
}, TaskContinuationOptions.OnlyOnRanToCompletion);
tasks.Add(continuationTask);
//Task unwrappedTask = continuationTask.Unwrap();
Task<Task> awaitedComiplerGeneratedTaskOfContinuationTask = await continuationTask;
tasks.Add(awaitedComiplerGeneratedTaskOfContinuationTask);
Task whenall = Task.WhenAll(tasks.ToArray());
return whenall;
}
static async Task FooAsync()
{
await Task.Delay(20000);
}
Upvotes: 2
Reputation: 9587
Okay, let's try to get to the bottom of what's happening here.
First things first: the difference in the lambda passed to your ContinueWith
is insignificant: functionally that part is identical in the two examples (at least as far as I can see).
Here's the FooAsync
implementation which I used for testing:
static Task FooAsync()
{
return Task.Delay(500);
}
What I found curious is that using this implementation your IndefinitelyBlockingTask
took twice as long as TaskWhichWorks
(1 second vs ~500 ms respectively). Obviously the behaviour has changed due to Unwrap
.
Someone with a keen eye would probably spot the problem right away, but personally I don't use task continuations or Unwrap
that much, so it took a little while to sink in.
Here's the kicker: unless you use Unwrap
on the continuation in both cases the task scheduled by ContinueWith
completes synchronously (and immediately - regardless of how long the tasks created inside the loop take). The task created inside the lambda (Task.WhenAll(childTasks.ToArray())
, let's call it inner task) is scheduled in a fire-and-forget fashion and runs onobserved.
Unwrap
ping the task returned from ContinueWith
means that the inner task is no longer fire-and-forget - it is now part of the execution chain, and when you add it to the list, the outer task (Task.WhenAll(tasks.ToArray())
) cannot complete until the inner task has completed).
Using ContinueWith(async () => { })
does not change the behaviour described above, because the task returned by the async lambda is not auto-unwrapped (think
// These two have similar behaviour and
// are interchangeable for our purposes.
Task.Run(() => Task.Delay(500))
Task.Run(async () => await Task.Delay(500));
vs
Task.Factory.StartNew(() => Task.Delay(500))
The Task.Run
call has Unwrap
built-in (see http://referencesource.microsoft.com/#mscorlib/system/threading/Tasks/Task.cs#0fb2b4d9262599b9#references); the StartNew
call doesn't and the task that it returns just completes immediately without waiting for the inner task. ContinueWith
is similar to StartNew
in that regard.
Side note
Another way to reproduce the behaviour observed when you use Unwrap
is to make sure that tasks created inside the loop (or their continuations) are attached to the parent causing the parent task (created by ContinueWith
) not to transition to the completed state until all child tasks have finished.
for (int i = 1; i <= 5; i++)
{
var ct = FooAsync().ContinueWith(_ => { }, TaskContinuationOptions.AttachedToParent);
childTasks.Add(ct);
}
Back to original problem
In your current implementation even if you had await Task.WhenAll(tasks.ToArray())
as the last line of the outer method, the method would still return before the tasks created inside the ContinueWith
lambda have completed. Even if the tasks created inside ContinueWith
never complete (my guess is that's exactly what's happening in your production code), the outer method will still return just fine.
So there it is, all things unexpected with the above code are caused by the silly ContinueWith
which essentially "falls through" unless you use Unwrap
. async
/await
is in no way the cause or the cure (although, admittedly, it can and probably should be used to rewrite your method in a more sensible way - continuations are difficult to work with leading to problems such as this one).
So what's happening in production
All of the above leads me to believe that there is a deadlock inside one of the tasks spun up inside your ContinueWith
lambda causing that innner Task.WhenAll
to never complete in production trim.
Unfortunately you have not posted a concise repro of the problem (I suppose I could do it for you armed with the above information, but it's really not my job to do so) or even the production code, so this is as much of a solution as I can give.
The fact that you weren't observing the described behaviour with your pseudo code should have hinted that you probably ended up stripping out the bit which was causing the problem. If you think that sounds silly, it's because it is, which is why I ended up retracting my original upvote for the question despite the fact that it is was the single most curious async problem I came across in a while.
CONCLUSION: Look at your ContinueWith
lambda.
Final edit
You insist that Unwrap
and await
do similar things, which is true (not really as it ultimately messes with task composition, but kind of true - at least for the purpose of this example). However, having said that, you never fully recreated the Unwrap
semantics using await
, so is there really a big surprise that the method behaves differently? Here's TaskWhichWorks
with an await
that will behave similarly to the Unwrap
example (it is also vulnerable to the deadlocking issues when applied to your production code):
static async Task TaskWhichUsedToWorkButNotAnymore()
{
List<Task> tasks = new List<Task>();
Task task = FooAsync();
tasks.Add(task);
Task<Task> continuationTask = task.ContinueWith(async t =>
{
List<Task> childTasks = new List<Task>();
for (int i = 1; i <= 5; i++)
{
var ct = FooAsync();
childTasks.Add(ct);
}
Task wa = Task.WhenAll(childTasks.ToArray());
await wa.ConfigureAwait(continueOnCapturedContext: false);
}, TaskContinuationOptions.OnlyOnRanToCompletion);
tasks.Add(continuationTask);
// Let's Unwrap the async/await way.
// Pay attention to the return type.
// The resulting task represents the
// completion of the task started inside
// (and returned by) the ContinueWith delegate.
// Without this you have no reference, and no
// way of waiting for, the inner task.
Task unwrappedTask = await continuationTask;
// Boom! This method now has the
// same behaviour as the other one.
tasks.Add(unwrappedTask);
await Task.WhenAll(tasks.ToArray());
// Another way of "unwrapping" the
// continuation just to drive the point home.
// This will complete immediately as the
// continuation task as well as the task
// started inside, and returned by the continuation
// task, have both completed at this point.
await await continuationTask;
}
Upvotes: 10