Fix SemaphoreSlim handling of canceled continuation invocations (#34036)
authorStephen Toub <stoub@microsoft.com>
Wed, 25 Mar 2020 02:20:12 +0000 (22:20 -0400)
committerGitHub <noreply@github.com>
Wed, 25 Mar 2020 02:20:12 +0000 (22:20 -0400)
When a SemaphoreSlim.WaitAsync(CancellationToken) completes because the token has cancellation requested, we end up invoking any continuations off of the returned Task synchronously as part of the cancellation registration.  That in turn means that a thread is blocked waiting for that cancelation callback to complete, which can lead to deadlock in niche situations.  We need to force this continuation to be async.

src/libraries/System.Private.CoreLib/src/System/Threading/SemaphoreSlim.cs
src/libraries/System.Threading/tests/SemaphoreSlimCancellationTests.cs

index 0fd065d..ae4494e 100644 (file)
@@ -728,7 +728,7 @@ namespace System.Threading
             else // millisecondsTimeout == Timeout.Infinite
             {
                 // Wait until either the task is completed or cancellation is requested.
-                var cancellationTask = new Task();
+                var cancellationTask = new Task(null, TaskCreationOptions.RunContinuationsAsynchronously, promiseStyle: true);
                 using (cancellationToken.UnsafeRegister(s => ((Task)s!).TrySetResult(), cancellationTask))
                 {
                     if (asyncWaiter == await TaskFactory.CommonCWAnyLogic(new Task[] { asyncWaiter, cancellationTask }).ConfigureAwait(false))
index 6aa8600..cbd68d3 100644 (file)
@@ -50,6 +50,34 @@ namespace System.Threading.Tests
             // currently we don't expose this.. but it was verified manually
         }
 
+        [Fact]
+        public static async Task Cancel_WaitAsync_ContinuationInvokedAsynchronously()
+        {
+            await Task.Run(async () => // escape xunit's SynchronizationContext
+            {
+                var cts = new CancellationTokenSource();
+                var tl = new ThreadLocal<object>();
+
+                var sentinel = new object();
+
+                var sem = new SemaphoreSlim(0);
+                Task continuation = sem.WaitAsync(cts.Token).ContinueWith(prev =>
+                {
+                    Assert.Equal(TaskStatus.Canceled, prev.Status);
+                    Assert.NotSame(sentinel, tl.Value);
+                }, CancellationToken.None, TaskContinuationOptions.ExecuteSynchronously, TaskScheduler.Default);
+
+                Assert.Equal(TaskStatus.WaitingForActivation, continuation.Status);
+                Assert.Equal(0, sem.CurrentCount);
+
+                tl.Value = sentinel;
+                cts.Cancel();
+                tl.Value = null;
+
+                await continuation;
+            });
+        }
+
         private static void EnsureOperationCanceledExceptionThrown(Action action, CancellationToken token)
         {
             OperationCanceledException operationCanceledEx =