Skip to content
This repository was archived by the owner on Jan 23, 2023. It is now read-only.

Commit 68633d3

Browse files
committed
Fix TaskExtensions.Unwrap stack dive
In strange but possible task configurations, it's possible to set up a series of Unwrap calls such that the continuations involved will form a very long chain of synchronously-executing calls. In such cases, without extra measure, the implementation can overflow the stack. The mscorlib Unwrap implementation takes precautions to avoid this, and ContinueWith (which was used in the previous, slower, standalone Unwrap implementation) has checks to avoid this as well, but currently await does not. Since our new Unwrap implementation uses the await infrastructure, it's susceptible again. This commit adds a check to the new Unwrap implementation to avoid the stack dive, as well as porting a test that triggers the stack dive prior to the fix. Some local benchmarks show little-to-no impact from the additional checks, and it's very rare to hit the fallback path that triggers the async queueing: while the actual numbers will depend on the actual code being run, the size of stack frames, etc., in the test we only hit the fallback case ~8 times out of 12000 iterations, and that's in code trying very hard to force this condition.
1 parent 2c4cac4 commit 68633d3

File tree

2 files changed

+43
-1
lines changed

2 files changed

+43
-1
lines changed

src/System.Threading.Tasks/src/System/Threading/Tasks/TaskExtensions.CoreCLR.cs

Lines changed: 25 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -2,6 +2,7 @@
22
// Licensed under the MIT license. See LICENSE file in the project root for full license information.
33

44
using System.Diagnostics;
5+
using System.Runtime.CompilerServices;
56

67
namespace System.Threading.Tasks
78
{
@@ -29,7 +30,7 @@ public static Task Unwrap(this Task<Task> task)
2930
{
3031
if (task == null)
3132
throw new ArgumentNullException("task");
32-
33+
3334
// Fast path for an already successfully completed outer task: just return the inner one.
3435
// As in the subsequent slower path, a null inner task is special-cased to mean cancellation.
3536
if (task.Status == TaskStatus.RanToCompletion && (task.CreationOptions & TaskCreationOptions.AttachedToParent) == 0)
@@ -160,6 +161,29 @@ private static bool TrySetFromTask<TResult>(this TaskCompletionSource<TResult> c
160161
{
161162
Debug.Assert(task.IsCompleted);
162163

164+
// Before transferring the results, check to make sure we're not too deep on the stack. Calling TrySet*
165+
// will cause any synchronous continuations to be invoked, which is fine unless we're so deep that doing
166+
// so overflows. ContinueWith has built-in support for avoiding such stack dives, but that support is not
167+
// (yet) part of await's infrastructure, so until it is we mimic it manually. This matches the behavior
168+
// employed by the Unwrap implementation in mscorlib. (If TryEnsureSufficientExecutionStack is made public
169+
// in the future, switch to using it to avoid the try/catch block and exception.)
170+
try
171+
{
172+
RuntimeHelpers.EnsureSufficientExecutionStack();
173+
}
174+
catch (InsufficientExecutionStackException)
175+
{
176+
// This is very rare. We're too deep to safely invoke
177+
// TrySet* synchronously, so do so asynchronously instead.
178+
Task.Factory.StartNew(s =>
179+
{
180+
var t = (Tuple<TaskCompletionSource<TResult>, Task>)s;
181+
TrySetFromTask(t.Item1, t.Item2);
182+
}, Tuple.Create(completionSource, task), CancellationToken.None, TaskCreationOptions.None, TaskScheduler.Default);
183+
return true;
184+
}
185+
186+
// Transfer the results from the supplied Task to the TaskCompletionSource.
163187
bool result = false;
164188
switch(task.Status)
165189
{

src/System.Threading.Tasks/tests/UnwrapTests.cs

Lines changed: 18 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -444,6 +444,24 @@ public void Generic_DefaultSchedulerUsed()
444444
}, CancellationToken.None, TaskCreationOptions.None, scheduler).GetAwaiter().GetResult();
445445
}
446446

447+
/// <summary>
448+
/// Test that a long chain of Unwraps can execute without overflowing the stack.
449+
/// </summary>
450+
[Fact]
451+
public void RunStackGuardTests()
452+
{
453+
const int DiveDepth = 12000;
454+
455+
Func<int, Task<int>> func = null;
456+
func = count =>
457+
++count < DiveDepth ?
458+
Task.Factory.StartNew(() => func(count), CancellationToken.None, TaskCreationOptions.None, TaskScheduler.Default).Unwrap() :
459+
Task.FromResult(count);
460+
461+
// This test will overflow if it fails.
462+
Assert.Equal(DiveDepth, func(0).Result);
463+
}
464+
447465
/// <summary>Gets an enumerable of already completed non-generic tasks.</summary>
448466
public static IEnumerable<object[]> CompletedNonGenericTasks
449467
{

0 commit comments

Comments
 (0)