diff --git a/src/Microsoft.VisualStudio.Threading/AsyncAutoResetEvent.cs b/src/Microsoft.VisualStudio.Threading/AsyncAutoResetEvent.cs index cb4e2e095..aa0d5d6a8 100644 --- a/src/Microsoft.VisualStudio.Threading/AsyncAutoResetEvent.cs +++ b/src/Microsoft.VisualStudio.Threading/AsyncAutoResetEvent.cs @@ -4,8 +4,6 @@ using System; using System.Collections.Generic; using System.Diagnostics; -using System.Linq; -using System.Text; using System.Threading; using System.Threading.Tasks; @@ -162,7 +160,7 @@ private void OnCancellationRequest(object state) /// /// Tracks someone waiting for a signal from the event. /// - private class WaiterCompletionSource : TaskCompletionSourceWithoutInlining + private class WaiterCompletionSource : TaskCompletionSource { /// /// Initializes a new instance of the class. @@ -171,7 +169,7 @@ private class WaiterCompletionSource : TaskCompletionSourceWithoutInlining to allow continuations to be inlined upon the completer's callstack. /// The cancellation token associated with the waiter. internal WaiterCompletionSource(AsyncAutoResetEvent owner, bool allowInliningContinuations, CancellationToken cancellationToken) - : base(allowInliningContinuations) + : base(allowInliningContinuations ? TaskCreationOptions.None : TaskCreationOptions.RunContinuationsAsynchronously) { this.CancellationToken = cancellationToken; this.Registration = cancellationToken.Register(NullableHelpers.AsNullableArgAction(owner.onCancellationRequestHandler), this); diff --git a/src/Microsoft.VisualStudio.Threading/AsyncManualResetEvent.cs b/src/Microsoft.VisualStudio.Threading/AsyncManualResetEvent.cs index 4bbab53a0..10f081c03 100644 --- a/src/Microsoft.VisualStudio.Threading/AsyncManualResetEvent.cs +++ b/src/Microsoft.VisualStudio.Threading/AsyncManualResetEvent.cs @@ -2,12 +2,9 @@ // Licensed under the MIT license. See LICENSE file in the project root for full license information. using System; -using System.Collections.Generic; using System.ComponentModel; using System.Diagnostics; -using System.Linq; using System.Runtime.CompilerServices; -using System.Text; using System.Threading; using System.Threading.Tasks; @@ -20,9 +17,9 @@ namespace Microsoft.VisualStudio.Threading; public class AsyncManualResetEvent { /// - /// Whether the task completion source should allow executing continuations synchronously. + /// Options to use while creating the to return from . /// - private readonly bool allowInliningAwaiters; + private readonly TaskCreationOptions options; /// /// The object to lock when accessing fields. @@ -36,21 +33,7 @@ public class AsyncManualResetEvent /// This should not need the volatile modifier because it is /// always accessed within a lock. /// - private TaskCompletionSourceWithoutInlining taskCompletionSource; - - /// - /// A flag indicating whether the event is signaled. - /// When this is set to true, it's possible that - /// .Task.IsCompleted is still false - /// if the completion has been scheduled asynchronously. - /// Thus, this field should be the definitive answer as to whether - /// the event is signaled because it is synchronously updated. - /// - /// - /// This should not need the volatile modifier because it is - /// always accessed within a lock. - /// - private bool isSet; + private TaskCompletionSource taskCompletionSource; /// /// Initializes a new instance of the class. @@ -58,17 +41,14 @@ public class AsyncManualResetEvent /// A value indicating whether the event should be initially signaled. /// /// A value indicating whether to allow callers' continuations to execute - /// on the thread that calls before the call returns. - /// callers should not hold private locks if this value is to avoid deadlocks. - /// When , the task returned from may not have fully transitioned to - /// its completed state by the time returns to its caller. + /// on the thread that calls before the call returns. + /// callers should not hold private locks if this value is to avoid deadlocks. /// public AsyncManualResetEvent(bool initialState = false, bool allowInliningAwaiters = false) { - this.allowInliningAwaiters = allowInliningAwaiters; + this.options = allowInliningAwaiters ? TaskCreationOptions.None : TaskCreationOptions.RunContinuationsAsynchronously; - this.taskCompletionSource = this.CreateTaskSource(); - this.isSet = initialState; + this.taskCompletionSource = new(this.options); if (initialState) { this.taskCompletionSource.SetResult(EmptyStruct.Instance); @@ -84,7 +64,7 @@ public bool IsSet { lock (this.syncObject) { - return this.isSet; + return this.taskCompletionSource.Task.IsCompleted; } } } @@ -112,40 +92,21 @@ public Task WaitAsync() /// /// A task that completes when the signal has been set. /// - /// - /// On .NET versions prior to 4.6: - /// This method may return before the signal set has propagated (so may return for a bit more if called immediately). - /// The returned task completes when the signal has definitely been set. - /// - /// - /// On .NET 4.6 and later: /// This method is not asynchronous. The returned Task is always completed. - /// /// [Obsolete("Use Set() instead."), EditorBrowsable(EditorBrowsableState.Never)] public Task SetAsync() { - TaskCompletionSourceWithoutInlining? tcs = null; - bool transitionRequired = false; + TaskCompletionSource? tcs = null; lock (this.syncObject) { - transitionRequired = !this.isSet; tcs = this.taskCompletionSource; - this.isSet = true; } - // Snap the Task that is exposed to the outside so we return that one. - // Once we complete the TaskCompletionSourceWithoutInlinining's task, - // the Task property will return the inner Task. - // SetAsync should return the same Task that WaitAsync callers would have observed previously. - Task result = tcs.Task; - - if (transitionRequired) - { - tcs.TrySetResult(default(EmptyStruct)); - } + tcs.TrySetResult(default); - return result; + // SetAsync should return the same Task that WaitAsync callers would have observed previously. + return tcs.Task; } /// @@ -165,10 +126,9 @@ public void Reset() { lock (this.syncObject) { - if (this.isSet) + if (this.taskCompletionSource.Task.IsCompleted) { - this.taskCompletionSource = this.CreateTaskSource(); - this.isSet = false; + this.taskCompletionSource = new(this.options); } } } @@ -178,20 +138,12 @@ public void Reset() /// /// A task that completes when the signal has been set. /// - /// - /// On .NET versions prior to 4.6: - /// This method may return before the signal set has propagated (so may return for a bit more if called immediately). - /// The returned task completes when the signal has definitely been set. - /// - /// - /// On .NET 4.6 and later: /// This method is not asynchronous. The returned Task is always completed. - /// /// [Obsolete("Use PulseAll() instead."), EditorBrowsable(EditorBrowsableState.Never)] public Task PulseAllAsync() { - TaskCompletionSourceWithoutInlining? tcs = null; + TaskCompletionSource? tcs = null; lock (this.syncObject) { // Atomically replace the completion source with a new, uncompleted source @@ -200,17 +152,13 @@ public Task PulseAllAsync() // continue to return completed Tasks due to a Pulse method which should // execute instantaneously. tcs = this.taskCompletionSource; - this.taskCompletionSource = this.CreateTaskSource(); - this.isSet = false; + this.taskCompletionSource = new(this.options); } - // Snap the Task that is exposed to the outside so we return that one. - // Once we complete the TaskCompletionSourceWithoutInlinining's task, - // the Task property will return the inner Task. + tcs.TrySetResult(default); + // PulseAllAsync should return the same Task that WaitAsync callers would have observed previously. - Task result = tcs.Task; - tcs.TrySetResult(default(EmptyStruct)); - return result; + return tcs.Task; } /// @@ -231,12 +179,4 @@ public TaskAwaiter GetAwaiter() { return this.WaitAsync().GetAwaiter(); } - - /// - /// Creates a new TaskCompletionSource to represent an unset event. - /// - private TaskCompletionSourceWithoutInlining CreateTaskSource() - { - return new TaskCompletionSourceWithoutInlining(this.allowInliningAwaiters); - } } diff --git a/src/Microsoft.VisualStudio.Threading/AsyncQueue`1.cs b/src/Microsoft.VisualStudio.Threading/AsyncQueue`1.cs index 10c7624ba..61affeb04 100644 --- a/src/Microsoft.VisualStudio.Threading/AsyncQueue`1.cs +++ b/src/Microsoft.VisualStudio.Threading/AsyncQueue`1.cs @@ -5,8 +5,6 @@ using System.Collections.Generic; using System.Diagnostics; using System.Diagnostics.CodeAnalysis; -using System.Linq; -using System.Text; using System.Threading; using System.Threading.Tasks; @@ -298,7 +296,7 @@ public T Peek() this.FreeCanceledDequeuers(); } - var waiterTcs = new TaskCompletionSourceWithoutInlining(allowInliningContinuations: false); + var waiterTcs = new TaskCompletionSource(TaskCreationOptions.RunContinuationsAsynchronously); waiterTcs.AttachCancellation(cancellationToken, this); this.dequeuingWaiters.Enqueue(waiterTcs); return waiterTcs.Task; diff --git a/src/Microsoft.VisualStudio.Threading/JoinableTask.cs b/src/Microsoft.VisualStudio.Threading/JoinableTask.cs index 2cb35d17c..2cf498fdc 100644 --- a/src/Microsoft.VisualStudio.Threading/JoinableTask.cs +++ b/src/Microsoft.VisualStudio.Threading/JoinableTask.cs @@ -867,28 +867,28 @@ internal void Post(SendOrPostCallback d, object? state, bool mainThreadAffinitiz } /// - /// Instantiate a that can track the ultimate result of . + /// Instantiate a that can track the ultimate result of . /// /// The new task completion source. /// /// The implementation should be sure to instantiate a that will /// NOT inline continuations, since we'll be completing this ourselves, potentially while holding a private lock. /// - internal virtual object CreateTaskCompletionSource() => new TaskCompletionSourceWithoutInlining(allowInliningContinuations: false); + internal virtual object CreateTaskCompletionSource() => new TaskCompletionSource(TaskCreationOptions.RunContinuationsAsynchronously); /// - /// Retrieves the from a . + /// Retrieves the from a . /// /// The task completion source. - /// The that will complete with this . - internal virtual Task GetTaskFromCompletionSource(object taskCompletionSource) => ((TaskCompletionSourceWithoutInlining)taskCompletionSource).Task; + /// The that will complete with this . + internal virtual Task GetTaskFromCompletionSource(object taskCompletionSource) => ((TaskCompletionSource)taskCompletionSource).Task; /// - /// Completes a . + /// Completes a . /// /// The task to read a result from. - /// The created earlier with to apply the result to. - internal virtual void CompleteTaskSourceFromWrappedTask(Task wrappedTask, object taskCompletionSource) => wrappedTask.ApplyResultTo((TaskCompletionSourceWithoutInlining)taskCompletionSource); + /// The created earlier with to apply the result to. + internal virtual void CompleteTaskSourceFromWrappedTask(Task wrappedTask, object taskCompletionSource) => wrappedTask.ApplyResultTo((TaskCompletionSource)taskCompletionSource); internal void SetWrappedTask(Task wrappedTask) { diff --git a/src/Microsoft.VisualStudio.Threading/JoinableTask`1.cs b/src/Microsoft.VisualStudio.Threading/JoinableTask`1.cs index c367d5752..9affc39a1 100644 --- a/src/Microsoft.VisualStudio.Threading/JoinableTask`1.cs +++ b/src/Microsoft.VisualStudio.Threading/JoinableTask`1.cs @@ -2,12 +2,8 @@ // Licensed under the MIT license. See LICENSE file in the project root for full license information. using System; -using System.Collections.Generic; using System.Diagnostics; -using System.Linq; -using System.Reflection; using System.Runtime.CompilerServices; -using System.Text; using System.Threading; using System.Threading.Tasks; @@ -135,10 +131,10 @@ static async Task JoinSlowAsync(JoinableTask me, bool continueOnCapturedCo } /// - internal override object CreateTaskCompletionSource() => new TaskCompletionSourceWithoutInlining(allowInliningContinuations: false); + internal override object CreateTaskCompletionSource() => new TaskCompletionSource(TaskCreationOptions.RunContinuationsAsynchronously); /// - internal override Task GetTaskFromCompletionSource(object taskCompletionSource) => ((TaskCompletionSourceWithoutInlining)taskCompletionSource).Task; + internal override Task GetTaskFromCompletionSource(object taskCompletionSource) => ((TaskCompletionSource)taskCompletionSource).Task; /// internal override void CompleteTaskSourceFromWrappedTask(Task wrappedTask, object taskCompletionSource) => ((Task)wrappedTask).ApplyResultTo((TaskCompletionSource)taskCompletionSource); diff --git a/src/Microsoft.VisualStudio.Threading/TaskCompletionSourceWithoutInlining`1.cs b/src/Microsoft.VisualStudio.Threading/TaskCompletionSourceWithoutInlining`1.cs deleted file mode 100644 index 5e5002ce2..000000000 --- a/src/Microsoft.VisualStudio.Threading/TaskCompletionSourceWithoutInlining`1.cs +++ /dev/null @@ -1,63 +0,0 @@ -// Copyright (c) Microsoft Corporation. All rights reserved. -// Licensed under the MIT license. See LICENSE file in the project root for full license information. - -using System; -using System.Diagnostics.CodeAnalysis; -using System.Threading; -using System.Threading.Tasks; - -namespace Microsoft.VisualStudio.Threading; - -/// -/// A -derivative that -/// does not inline continuations if so configured. -/// -/// The type of the task's resulting value. -internal class TaskCompletionSourceWithoutInlining : TaskCompletionSource -{ - /// - /// The Task that we expose to others that may not inline continuations. - /// - private readonly Task exposedTask; - - /// - /// Initializes a new instance of the class. - /// - /// - /// to allow continuations to be inlined; otherwise . - /// - /// - /// TaskCreationOptions to pass on to the base constructor. - /// - /// The state to set on the Task. - internal TaskCompletionSourceWithoutInlining(bool allowInliningContinuations, TaskCreationOptions options = TaskCreationOptions.None, object? state = null) - : base(state, AdjustFlags(options, allowInliningContinuations)) - { - this.exposedTask = base.Task; - } - - /// - /// Gets the that may never complete inline with completion of this . - /// - /// - /// Return the base.Task if it is already completed since inlining continuations - /// on the completer is no longer a concern. Also, when we are not inlining continuations, - /// this.exposedTask completes slightly later than base.Task, and callers expect - /// the Task we return to be complete as soon as they call TrySetResult. - /// - internal new Task Task => base.Task.IsCompleted ? base.Task : this.exposedTask; - - /// - /// Modifies the specified flags to include RunContinuationsAsynchronously - /// if wanted by the caller and supported by the platform. - /// - /// The base options supplied by the caller. - /// to allow inlining continuations. - /// The possibly modified flags. - private static TaskCreationOptions AdjustFlags(TaskCreationOptions options, bool allowInliningContinuations) - { - return allowInliningContinuations - ? (options & ~TaskCreationOptions.RunContinuationsAsynchronously) - : (options | TaskCreationOptions.RunContinuationsAsynchronously); - } -} diff --git a/test/Microsoft.VisualStudio.Threading.Tests/AsyncManualResetEventTests.cs b/test/Microsoft.VisualStudio.Threading.Tests/AsyncManualResetEventTests.cs index 556bc7c20..beaf94e5b 100644 --- a/test/Microsoft.VisualStudio.Threading.Tests/AsyncManualResetEventTests.cs +++ b/test/Microsoft.VisualStudio.Threading.Tests/AsyncManualResetEventTests.cs @@ -59,6 +59,87 @@ public void SetReturnsBeforeInlinedContinuations() Assert.True(inlinedContinuation.Wait(UnexpectedTimeout)); } + /// + /// Verifies that inlining continuations do not delay the Task returned from SetAsync() from completing. + /// + [Fact] + public void SetAsyncReturnsCompletedTaskBeforeNonInlinedContinuationsComplete() + { + ManualResetEventSlim setReturned = new(); + Task? inlinedContinuation = this.evt.WaitAsync() + .ContinueWith( + delegate + { + // Arrange to synchronously block the continuation until released, + // which would deadlock if SetAsync's Task does not complete until inlined continuations complete. + Assert.True(setReturned.Wait(UnexpectedTimeout)); + }, + TaskContinuationOptions.ExecuteSynchronously); +#pragma warning disable CS0618 // Type or member is obsolete + Task setTask = this.evt.SetAsync(); +#pragma warning restore CS0618 // Type or member is obsolete + Assert.True(setTask.IsCompleted); + Assert.True(this.evt.IsSet); + + // Release the continuation so the test doesn't leak a thread. + setReturned.Set(); + + Assert.True(inlinedContinuation.Wait(UnexpectedTimeout)); + } + + /// + /// Verifies that inlining continuations do not delay the Task returned from SetAsync() from completing. + /// + [Fact] + public void SetAsyncAndWaitAsyncReturnsCompletedTaskBeforeInlinedContinuationsComplete() + { + // Reconfigure the object to allow inlining awaiters. + this.evt = new(allowInliningAwaiters: true); + + // Arrange for an awaiter that is not only inlined, but synchronously blocks + // its caller, such that the SetAsync method that completes the Task cannot immediately return. + ManualResetEventSlim continuationStarted = new(); + ManualResetEventSlim setReturned = new(); + Task? inlinedContinuation = this.evt.WaitAsync() + .ContinueWith( + delegate + { + continuationStarted.Set(); + + // Arrange to synchronously block the continuation until released, + // which would deadlock if SetAsync's Task does not complete until inlined continuations complete. + Assert.True(setReturned.Wait(UnexpectedTimeout)); + }, + TaskContinuationOptions.ExecuteSynchronously); + +#pragma warning disable CS0618 // Type or member is obsolete + // First, spin off a thread to call SetAsync. This will be blocked. + Task.Run(() => this.evt.SetAsync(), TestContext.Current.CancellationToken); + + // Now wait for the continuation to start, to verify that SetAsync has done its work, + // even if it can't return to us. + Assert.True(continuationStarted.Wait(UnexpectedTimeout)); + + // Verify that the event is set, even though SetAsync has not yet returned. + Assert.True(this.evt.IsSet); + + // Now call SetAsync again. This time it should return immediately + // even though the first call hasn't yet, and it should return a completed Task. + Task setTask = this.evt.SetAsync(); +#pragma warning restore CS0618 // Type or member is obsolete + + Assert.True(setTask.IsCompleted); + + // Also verify that WaitAsync returns a completed Task, even though the first SetAsync call hasn't yet returned. + Assert.True(this.evt.WaitAsync().IsCompleted); + + // Release the continuation so the test doesn't leak a thread. + setReturned.Set(); + + // Verify that the inlined continuation has completed and didn't throw. + Assert.True(inlinedContinuation.Wait(UnexpectedTimeout)); + } + [Fact] public async Task Blocking() {