diff --git a/src/mscorlib/src/System/Runtime/CompilerServices/AsyncMethodBuilder.cs b/src/mscorlib/src/System/Runtime/CompilerServices/AsyncMethodBuilder.cs index e81823eb1607..704628690a48 100644 --- a/src/mscorlib/src/System/Runtime/CompilerServices/AsyncMethodBuilder.cs +++ b/src/mscorlib/src/System/Runtime/CompilerServices/AsyncMethodBuilder.cs @@ -10,17 +10,10 @@ // // =-=-=-=-=-=-=-=-=-=-=-=-=-=-=-=-=-=-=-=-=-=-=-=-=-=-=-=-=-=-=-=-=-=-=-=-=-=-=-=-=-=-=- -using System; -using System.Collections.Concurrent; -using System.Collections.Generic; using System.Diagnostics; -using System.Diagnostics.CodeAnalysis; -using System.Diagnostics.Contracts; using System.Runtime.ExceptionServices; -using System.Security; using System.Threading; using System.Threading.Tasks; - #if FEATURE_COMINTEROP using System.Runtime.InteropServices.WindowsRuntime; #endif // FEATURE_COMINTEROP @@ -34,23 +27,17 @@ namespace System.Runtime.CompilerServices public struct AsyncVoidMethodBuilder { /// The synchronization context associated with this operation. - private SynchronizationContext m_synchronizationContext; - /// State related to the IAsyncStateMachine. - private AsyncMethodBuilderCore m_coreState; // mutable struct: must not be readonly - /// Task used for debugging and logging purposes only. Lazily initialized. - private Task m_task; + private SynchronizationContext _synchronizationContext; + /// The builder this void builder wraps. + private AsyncTaskMethodBuilder _builder; // mutable struct: must not be readonly /// Initializes a new . /// The initialized . public static AsyncVoidMethodBuilder Create() { - // Capture the current sync context. If there isn't one, use the dummy s_noContextCaptured - // instance; this allows us to tell the state of no captured context apart from the state - // of an improperly constructed builder instance. SynchronizationContext sc = SynchronizationContext.CurrentNoFlow; - if (sc != null) - sc.OperationStarted(); - return new AsyncVoidMethodBuilder() { m_synchronizationContext = sc }; + sc?.OperationStarted(); + return new AsyncVoidMethodBuilder() { _synchronizationContext = sc }; } /// Initiates the builder's execution with the associated state machine. @@ -58,39 +45,15 @@ public static AsyncVoidMethodBuilder Create() /// The state machine instance, passed by reference. /// The argument was null (Nothing in Visual Basic). [DebuggerStepThrough] - public void Start(ref TStateMachine stateMachine) where TStateMachine : IAsyncStateMachine - { - // See comment on AsyncMethodBuilderCore.Start - // AsyncMethodBuilderCore.Start(ref stateMachine); - - if (stateMachine == null) ThrowHelper.ThrowArgumentNullException(ExceptionArgument.stateMachine); - Contract.EndContractBlock(); - - // Run the MoveNext method within a copy-on-write ExecutionContext scope. - // This allows us to undo any ExecutionContext changes made in MoveNext, - // so that they won't "leak" out of the first await. - - Thread currentThread = Thread.CurrentThread; - ExecutionContextSwitcher ecs = default(ExecutionContextSwitcher); - try - { - ExecutionContext.EstablishCopyOnWriteScope(currentThread, ref ecs); - stateMachine.MoveNext(); - } - finally - { - ecs.Undo(currentThread); - } - } + public void Start(ref TStateMachine stateMachine) where TStateMachine : IAsyncStateMachine => + _builder.Start(ref stateMachine); /// Associates the builder with the state machine it represents. /// The heap-allocated state machine object. /// The argument was null (Nothing in Visual Basic). /// The builder is incorrectly initialized. - public void SetStateMachine(IAsyncStateMachine stateMachine) - { - m_coreState.SetStateMachine(stateMachine); // argument validation handled by AsyncMethodBuilderCore - } + public void SetStateMachine(IAsyncStateMachine stateMachine) => + _builder.SetStateMachine(stateMachine); /// /// Schedules the specified state machine to be pushed forward when the specified awaiter completes. @@ -102,41 +65,8 @@ public void SetStateMachine(IAsyncStateMachine stateMachine) public void AwaitOnCompleted( ref TAwaiter awaiter, ref TStateMachine stateMachine) where TAwaiter : INotifyCompletion - where TStateMachine : IAsyncStateMachine - { - try - { - AsyncMethodBuilderCore.MoveNextRunner runnerToInitialize = null; - var continuation = m_coreState.GetCompletionAction(AsyncCausalityTracer.LoggingOn ? this.Task : null, ref runnerToInitialize); - Debug.Assert(continuation != null, "GetCompletionAction should always return a valid action."); - - // If this is our first await, such that we've not yet boxed the state machine, do so now. - if (m_coreState.m_stateMachine == null) - { - if (AsyncCausalityTracer.LoggingOn) - AsyncCausalityTracer.TraceOperationCreation(CausalityTraceLevel.Required, this.Task.Id, "Async: " + stateMachine.GetType().Name, 0); - - // Box the state machine, then tell the boxed instance to call back into its own builder, - // so we can cache the boxed reference. NOTE: The language compiler may choose to use - // a class instead of a struct for the state machine for debugging purposes; in such cases, - // the stateMachine will already be an object. - m_coreState.PostBoxInitialization(stateMachine, runnerToInitialize, null); - } - - awaiter.OnCompleted(continuation); - } - catch (Exception exc) - { - // Prevent exceptions from leaking to the call site, which could - // then allow multiple flows of execution through the same async method - // if the awaiter had already scheduled the continuation by the time - // the exception was thrown. We propagate the exception on the - // ThreadPool because we can trust it to not throw, unlike - // if we were to go to a user-supplied SynchronizationContext, - // whose Post method could easily throw. - AsyncMethodBuilderCore.ThrowAsync(exc, targetContext: null); - } - } + where TStateMachine : IAsyncStateMachine => + _builder.AwaitOnCompleted(ref awaiter, ref stateMachine); /// /// Schedules the specified state machine to be pushed forward when the specified awaiter completes. @@ -148,45 +78,23 @@ public void AwaitOnCompleted( public void AwaitUnsafeOnCompleted( ref TAwaiter awaiter, ref TStateMachine stateMachine) where TAwaiter : ICriticalNotifyCompletion - where TStateMachine : IAsyncStateMachine - { - try - { - AsyncMethodBuilderCore.MoveNextRunner runnerToInitialize = null; - var continuation = m_coreState.GetCompletionAction(AsyncCausalityTracer.LoggingOn ? this.Task : null, ref runnerToInitialize); - Debug.Assert(continuation != null, "GetCompletionAction should always return a valid action."); - - // If this is our first await, such that we've not yet boxed the state machine, do so now. - if (m_coreState.m_stateMachine == null) - { - if (AsyncCausalityTracer.LoggingOn) - AsyncCausalityTracer.TraceOperationCreation(CausalityTraceLevel.Required, this.Task.Id, "Async: " + stateMachine.GetType().Name, 0); - - // Box the state machine, then tell the boxed instance to call back into its own builder, - // so we can cache the boxed reference. NOTE: The language compiler may choose to use - // a class instead of a struct for the state machine for debugging purposes; in such cases, - // the stateMachine will already be an object. - m_coreState.PostBoxInitialization(stateMachine, runnerToInitialize, null); - } - - awaiter.UnsafeOnCompleted(continuation); - } - catch (Exception e) - { - AsyncMethodBuilderCore.ThrowAsync(e, targetContext: null); - } - } + where TStateMachine : IAsyncStateMachine => + _builder.AwaitUnsafeOnCompleted(ref awaiter, ref stateMachine); /// Completes the method builder successfully. public void SetResult() { if (AsyncCausalityTracer.LoggingOn) + { AsyncCausalityTracer.TraceOperationCompletion(CausalityTraceLevel.Required, this.Task.Id, AsyncCausalityStatus.Completed); + } - if (m_synchronizationContext != null) + if (_synchronizationContext != null) { NotifySynchronizationContextOfCompletion(); } + + // No need to call _builder.SetResult, as no one pays attention to the task's completion. } /// Faults the method builder with an exception. @@ -195,19 +103,23 @@ public void SetResult() /// The builder is not initialized. public void SetException(Exception exception) { - if (exception == null) throw new ArgumentNullException(nameof(exception)); - Contract.EndContractBlock(); + if (exception == null) + { + ThrowHelper.ThrowArgumentNullException(ExceptionArgument.exception); + } if (AsyncCausalityTracer.LoggingOn) + { AsyncCausalityTracer.TraceOperationCompletion(CausalityTraceLevel.Required, this.Task.Id, AsyncCausalityStatus.Error); + } - if (m_synchronizationContext != null) + if (_synchronizationContext != null) { // If we captured a synchronization context, Post the throwing of the exception to it // and decrement its outstanding operation count. try { - AsyncMethodBuilderCore.ThrowAsync(exception, targetContext: m_synchronizationContext); + AsyncMethodBuilderCore.ThrowAsync(exception, targetContext: _synchronizationContext); } finally { @@ -221,15 +133,17 @@ public void SetException(Exception exception) // file or a CLR host. AsyncMethodBuilderCore.ThrowAsync(exception, targetContext: null); } + + // No need to call _builder.SetException, as no one pays attention to the task's completion. } /// Notifies the current synchronization context that the operation completed. private void NotifySynchronizationContextOfCompletion() { - Debug.Assert(m_synchronizationContext != null, "Must only be used with a non-null context."); + Debug.Assert(_synchronizationContext != null, "Must only be used with a non-null context."); try { - m_synchronizationContext.OperationCompleted(); + _synchronizationContext.OperationCompleted(); } catch (Exception exc) { @@ -240,7 +154,7 @@ private void NotifySynchronizationContextOfCompletion() } /// Lazily instantiate the Task in a non-thread-safe manner. - private Task Task => m_task ?? (m_task = new Task()); + private Task Task => _builder.Task; /// /// Gets an object that may be used to uniquely identify this builder to the debugger. @@ -249,7 +163,7 @@ private void NotifySynchronizationContextOfCompletion() /// This property lazily instantiates the ID in a non-thread-safe manner. /// It must only be used by the debugger and AsyncCausalityTracer in a single-threaded manner. /// - private object ObjectIdForDebugger { get { return this.Task; } } + internal object ObjectIdForDebugger => _builder.ObjectIdForDebugger; } /// @@ -267,54 +181,25 @@ public struct AsyncTaskMethodBuilder private readonly static Task s_cachedCompleted = AsyncTaskMethodBuilder.s_defaultResultTask; /// The generic builder object to which this non-generic instance delegates. - private AsyncTaskMethodBuilder m_builder; // mutable struct: must not be readonly + private AsyncTaskMethodBuilder m_builder; // mutable struct: must not be readonly. Debugger depends on the exact name of this field. /// Initializes a new . /// The initialized . - public static AsyncTaskMethodBuilder Create() - { - return default(AsyncTaskMethodBuilder); - // Note: If ATMB.Create is modified to do any initialization, this - // method needs to be updated to do m_builder = ATMB.Create(). - } + public static AsyncTaskMethodBuilder Create() => default(AsyncTaskMethodBuilder); /// Initiates the builder's execution with the associated state machine. /// Specifies the type of the state machine. /// The state machine instance, passed by reference. [DebuggerStepThrough] - public void Start(ref TStateMachine stateMachine) where TStateMachine : IAsyncStateMachine - { - // See comment on AsyncMethodBuilderCore.Start - // AsyncMethodBuilderCore.Start(ref stateMachine); - - if (stateMachine == null) ThrowHelper.ThrowArgumentNullException(ExceptionArgument.stateMachine); - Contract.EndContractBlock(); - - // Run the MoveNext method within a copy-on-write ExecutionContext scope. - // This allows us to undo any ExecutionContext changes made in MoveNext, - // so that they won't "leak" out of the first await. - - Thread currentThread = Thread.CurrentThread; - ExecutionContextSwitcher ecs = default(ExecutionContextSwitcher); - try - { - ExecutionContext.EstablishCopyOnWriteScope(currentThread, ref ecs); - stateMachine.MoveNext(); - } - finally - { - ecs.Undo(currentThread); - } - } + public void Start(ref TStateMachine stateMachine) where TStateMachine : IAsyncStateMachine => + m_builder.Start(ref stateMachine); /// Associates the builder with the state machine it represents. /// The heap-allocated state machine object. /// The argument was null (Nothing in Visual Basic). /// The builder is incorrectly initialized. - public void SetStateMachine(IAsyncStateMachine stateMachine) - { - m_builder.SetStateMachine(stateMachine); // argument validation handled by AsyncMethodBuilderCore - } + public void SetStateMachine(IAsyncStateMachine stateMachine) => + m_builder.SetStateMachine(stateMachine); /// /// Schedules the specified state machine to be pushed forward when the specified awaiter completes. @@ -326,10 +211,8 @@ public void SetStateMachine(IAsyncStateMachine stateMachine) public void AwaitOnCompleted( ref TAwaiter awaiter, ref TStateMachine stateMachine) where TAwaiter : INotifyCompletion - where TStateMachine : IAsyncStateMachine - { - m_builder.AwaitOnCompleted(ref awaiter, ref stateMachine); - } + where TStateMachine : IAsyncStateMachine => + m_builder.AwaitOnCompleted(ref awaiter, ref stateMachine); /// /// Schedules the specified state machine to be pushed forward when the specified awaiter completes. @@ -341,10 +224,8 @@ public void AwaitOnCompleted( public void AwaitUnsafeOnCompleted( ref TAwaiter awaiter, ref TStateMachine stateMachine) where TAwaiter : ICriticalNotifyCompletion - where TStateMachine : IAsyncStateMachine - { - m_builder.AwaitUnsafeOnCompleted(ref awaiter, ref stateMachine); - } + where TStateMachine : IAsyncStateMachine => + m_builder.AwaitUnsafeOnCompleted(ref awaiter, ref stateMachine); /// Gets the for this builder. /// The representing the builder's asynchronous operation. @@ -352,7 +233,7 @@ public void AwaitUnsafeOnCompleted( public Task Task { [MethodImpl(MethodImplOptions.AggressiveInlining)] - get { return m_builder.Task; } + get => m_builder.Task; } /// @@ -361,12 +242,7 @@ public Task Task /// /// The builder is not initialized. /// The task has already completed. - public void SetResult() - { - // Accessing AsyncTaskMethodBuilder.s_cachedCompleted is faster than - // accessing AsyncTaskMethodBuilder.s_defaultResultTask. - m_builder.SetResult(s_cachedCompleted); - } + public void SetResult() => m_builder.SetResult(s_cachedCompleted); // Using s_cachedCompleted is faster than using s_defaultResultTask. /// /// Completes the in the @@ -376,7 +252,7 @@ public void SetResult() /// The argument is null (Nothing in Visual Basic). /// The builder is not initialized. /// The task has already completed. - public void SetException(Exception exception) { m_builder.SetException(exception); } + public void SetException(Exception exception) => m_builder.SetException(exception); /// /// Called by the debugger to request notification when the first wait operation @@ -385,10 +261,7 @@ public void SetResult() /// /// true to enable notification; false to disable a previously set notification. /// - internal void SetNotificationForWaitCompletion(bool enabled) - { - m_builder.SetNotificationForWaitCompletion(enabled); - } + internal void SetNotificationForWaitCompletion(bool enabled) => m_builder.SetNotificationForWaitCompletion(enabled); /// /// Gets an object that may be used to uniquely identify this builder to the debugger. @@ -398,7 +271,7 @@ internal void SetNotificationForWaitCompletion(bool enabled) /// It must only be used by the debugger and tracing purposes, and only in a single-threaded manner /// when no other threads are in the middle of accessing this property or this.Task. /// - private object ObjectIdForDebugger { get { return this.Task; } } + internal object ObjectIdForDebugger => m_builder.ObjectIdForDebugger; } /// @@ -415,16 +288,8 @@ public struct AsyncTaskMethodBuilder /// A cached task for default(TResult). internal readonly static Task s_defaultResultTask = AsyncTaskCache.CreateCacheableTask(default(TResult)); - // WARNING: For performance reasons, the m_task field is lazily initialized. - // For correct results, the struct AsyncTaskMethodBuilder must - // always be used from the same location/copy, at least until m_task is - // initialized. If that guarantee is broken, the field could end up being - // initialized on the wrong copy. - - /// State related to the IAsyncStateMachine. - private AsyncMethodBuilderCore m_coreState; // mutable struct: must not be readonly /// The lazily-initialized built task. - private Task m_task; // lazily-initialized: must not be readonly + private Task m_task; // lazily-initialized: must not be readonly. Debugger depends on the exact name of this field. /// Initializes a new . /// The initialized . @@ -442,11 +307,10 @@ public static AsyncTaskMethodBuilder Create() [DebuggerStepThrough] public void Start(ref TStateMachine stateMachine) where TStateMachine : IAsyncStateMachine { - // See comment on AsyncMethodBuilderCore.Start - // AsyncMethodBuilderCore.Start(ref stateMachine); - - if (stateMachine == null) ThrowHelper.ThrowArgumentNullException(ExceptionArgument.stateMachine); - Contract.EndContractBlock(); + if (stateMachine == null) // TStateMachines are generally non-nullable value types, so this check will be elided + { + ThrowHelper.ThrowArgumentNullException(ExceptionArgument.stateMachine); + } // Run the MoveNext method within a copy-on-write ExecutionContext scope. // This allows us to undo any ExecutionContext changes made in MoveNext, @@ -471,7 +335,20 @@ public void Start(ref TStateMachine stateMachine) where TStateMac /// The builder is incorrectly initialized. public void SetStateMachine(IAsyncStateMachine stateMachine) { - m_coreState.SetStateMachine(stateMachine); // argument validation handled by AsyncMethodBuilderCore + if (stateMachine == null) + { + ThrowHelper.ThrowArgumentNullException(ExceptionArgument.stateMachine); + } + + if (m_task != null) + { + ThrowHelper.ThrowInvalidOperationException(ExceptionResource.AsyncMethodBuilder_InstanceNotInitialized); + } + + // SetStateMachine was originally needed in order to store the boxed state machine reference into + // the boxed copy. Now that a normal box is no longer used, SetStateMachine is also legacy. We need not + // do anything here, and thus assert to ensure we're not calling this from our own implementations. + Debug.Fail("SetStateMachine should not be used."); } /// @@ -488,25 +365,7 @@ public void AwaitOnCompleted( { try { - AsyncMethodBuilderCore.MoveNextRunner runnerToInitialize = null; - var continuation = m_coreState.GetCompletionAction(AsyncCausalityTracer.LoggingOn ? this.Task : null, ref runnerToInitialize); - Debug.Assert(continuation != null, "GetCompletionAction should always return a valid action."); - - // If this is our first await, such that we've not yet boxed the state machine, do so now. - if (m_coreState.m_stateMachine == null) - { - // Force the Task to be initialized prior to the first suspending await so - // that the original stack-based builder has a reference to the right Task. - Task builtTask = this.Task; - - // Box the state machine, then tell the boxed instance to call back into its own builder, - // so we can cache the boxed reference. NOTE: The language compiler may choose to use - // a class instead of a struct for the state machine for debugging purposes; in such cases, - // the stateMachine will already be an object. - m_coreState.PostBoxInitialization(stateMachine, runnerToInitialize, builtTask); - } - - awaiter.OnCompleted(continuation); + awaiter.OnCompleted(GetMoveNextDelegate(ref stateMachine)); } catch (Exception e) { @@ -528,30 +387,122 @@ public void AwaitUnsafeOnCompleted( { try { - AsyncMethodBuilderCore.MoveNextRunner runnerToInitialize = null; - var continuation = m_coreState.GetCompletionAction(AsyncCausalityTracer.LoggingOn ? this.Task : null, ref runnerToInitialize); - Debug.Assert(continuation != null, "GetCompletionAction should always return a valid action."); + awaiter.UnsafeOnCompleted(GetMoveNextDelegate(ref stateMachine)); + } + catch (Exception e) + { + AsyncMethodBuilderCore.ThrowAsync(e, targetContext: null); + } + } + + /// Gets the "boxed" state machine object. + /// Specifies the type of the async state machine. + /// The state machine. + /// The "boxed" state machine. + private Action GetMoveNextDelegate( + ref TStateMachine stateMachine) + where TStateMachine : IAsyncStateMachine + { + ExecutionContext currentContext = ExecutionContext.Capture(); - // If this is our first await, such that we've not yet boxed the state machine, do so now. - if (m_coreState.m_stateMachine == null) + // Check first for the most common case: not the first yield in an async method. + // In this case, the first yield will have already "boxed" the state machine in + // a strongly-typed manner into an AsyncStateMachineBox. It will already contain + // the state machine as well as a MoveNextDelegate and a context. The only thing + // we might need to do is update the context if that's changed since it was stored. + if (m_task is AsyncStateMachineBox stronglyTypedBox) + { + if (stronglyTypedBox.Context != currentContext) { - // Force the Task to be initialized prior to the first suspending await so - // that the original stack-based builder has a reference to the right Task. - Task builtTask = this.Task; - - // Box the state machine, then tell the boxed instance to call back into its own builder, - // so we can cache the boxed reference. NOTE: The language compiler may choose to use - // a class instead of a struct for the state machine for debugging purposes; in such cases, - // the stateMachine will already be an object. - m_coreState.PostBoxInitialization(stateMachine, runnerToInitialize, builtTask); + stronglyTypedBox.Context = currentContext; + } + return stronglyTypedBox.MoveNextAction; + } + + // The least common case: we have a weakly-typed boxed. This results if the debugger + // or some other use of reflection accesses a property like ObjectIdForDebugger or a + // method like SetNotificationForWaitCompletion prior to the first await happening. In + // such situations, we need to get an object to represent the builder, but we don't yet + // know the type of the state machine, and thus can't use TStateMachine. Instead, we + // use the IAsyncStateMachine interface, which all TStateMachines implement. This will + // result in a boxing allocation when storing the TStateMachine if it's a struct, but + // this only happens in active debugging scenarios where such performance impact doesn't + // matter. + if (m_task is AsyncStateMachineBox weaklyTypedBox) + { + // If this is the first await, we won't yet have a state machine, so store it. + if (weaklyTypedBox.StateMachine == null) + { + Debugger.NotifyOfCrossThreadDependency(); // same explanation as with usage below + weaklyTypedBox.StateMachine = stateMachine; } - awaiter.UnsafeOnCompleted(continuation); + // Update the context. This only happens with a debugger, so no need to spend + // extra IL checking for equality before doing the assignment. + weaklyTypedBox.Context = currentContext; + return weaklyTypedBox.MoveNextAction; } - catch (Exception e) + + // Alert a listening debugger that we can't make forward progress unless it slips threads. + // If we don't do this, and a method that uses "await foo;" is invoked through funceval, + // we could end up hooking up a callback to push forward the async method's state machine, + // the debugger would then abort the funceval after it takes too long, and then continuing + // execution could result in another callback being hooked up. At that point we have + // multiple callbacks registered to push the state machine, which could result in bad behavior. + Debugger.NotifyOfCrossThreadDependency(); + + // At this point, m_task should really be null, in which case we want to create the box. + // However, in a variety of debugger-related (erroneous) situations, it might be non-null, + // e.g. if the Task property is examined in a Watch window, forcing it to be lazily-intialized + // as a Task rather than as an AsyncStateMachineBox. The worst that happens in such + // cases is we lose the ability to properly step in the debugger, as the debugger uses that + // object's identity to track this specific builder/state machine. As such, we proceed to + // overwrite whatever's there anyway, even if it's non-null. + var box = new AsyncStateMachineBox(); + m_task = box; // important: this must be done before storing stateMachine into box.StateMachine! + box.StateMachine = stateMachine; + box.Context = currentContext; + return box.MoveNextAction; + } + + /// A strongly-typed box for Task-based async state machines. + /// Specifies the type of the state machine. + /// Specifies the type of the Task's result. + private sealed class AsyncStateMachineBox : + Task, IDebuggingAsyncStateMachineAccessor + where TStateMachine : IAsyncStateMachine + { + /// Delegate used to invoke on an ExecutionContext when passed an instance of this box type. + private static readonly ContextCallback s_callback = s => ((AsyncStateMachineBox)s).StateMachine.MoveNext(); + + /// A delegate to the method. + public readonly Action MoveNextAction; + /// The state machine itself. + public TStateMachine StateMachine; // mutable struct; do not make this readonly + /// Captured ExecutionContext with which to invoke ; may be null. + public ExecutionContext Context; + + public AsyncStateMachineBox() { - AsyncMethodBuilderCore.ThrowAsync(e, targetContext: null); + var mn = new Action(MoveNext); + MoveNextAction = AsyncCausalityTracer.LoggingOn ? AsyncMethodBuilderCore.OutputAsyncCausalityEvents(this, mn) : mn; } + + /// Call MoveNext on . + private void MoveNext() + { + if (Context == null) + { + StateMachine.MoveNext(); + } + else + { + ExecutionContext.Run(Context, s_callback, this); + } + } + + /// Gets the state machine as a boxed object. This should only be used for debugging purposes. + IAsyncStateMachine IDebuggingAsyncStateMachineAccessor.GetStateMachineObject() => StateMachine; // likely boxes, only use for debugging } /// Gets the for this builder. @@ -559,12 +510,33 @@ public void AwaitUnsafeOnCompleted( public Task Task { [MethodImpl(MethodImplOptions.AggressiveInlining)] - get { return m_task ?? InitializeTask(); } + get => m_task ?? InitializeTaskAsPromise(); + } + + /// + /// Initializes the task, which must not yet be initialized. Used only when the Task is being forced into + /// existence when no state machine is needed, e.g. when the builder is being synchronously completed with + /// an exception, when the builder is being used out of the context of an async method, etc. + /// + [MethodImpl(MethodImplOptions.NoInlining)] + private Task InitializeTaskAsPromise() + { + Debug.Assert(m_task == null); + return (m_task = new Task()); } - /// Initializes the task, which must not yet be initialized. + /// + /// Initializes the task, which must not yet be initialized. Used only when the Task is being forced into + /// existence due to the debugger trying to enable step-out/step-over/etc. prior to the first await yielding + /// in an async method. In that case, we don't know the actual TStateMachine type, so we're forced to + /// use IAsyncStateMachine instead. + /// [MethodImpl(MethodImplOptions.NoInlining)] - private Task InitializeTask() => (m_task = new Task()); + private Task InitializeTaskAsStateMachineBox() + { + Debug.Assert(m_task == null); + return (m_task = new AsyncStateMachineBox()); + } /// /// Completes the in the @@ -579,7 +551,7 @@ public void SetResult(TResult result) if (m_task == null) { m_task = GetTaskForResult(result); - Debug.Assert(m_task != null, "GetTaskForResult should never return null"); + Debug.Assert(m_task != null, $"{nameof(GetTaskForResult)} should never return null"); } else { @@ -630,8 +602,8 @@ private void LogExistingTaskCompletion() /// The task has already completed. internal void SetResult(Task completedTask) { - Contract.Requires(completedTask != null, "Expected non-null task"); - Contract.Requires(completedTask.Status == TaskStatus.RanToCompletion, "Expected a successfully completed task"); + Debug.Assert(completedTask != null, "Expected non-null task"); + Debug.Assert(completedTask.IsCompletedSuccessfully, "Expected a successfully completed task"); // Get the currently stored task, which will be non-null if get_Task has already been accessed. // If there isn't one, store the supplied completed task. @@ -655,30 +627,26 @@ internal void SetResult(Task completedTask) /// The task has already completed. public void SetException(Exception exception) { - if (exception == null) throw new ArgumentNullException(nameof(exception)); - Contract.EndContractBlock(); - - - var task = m_task; - if (task == null) + if (exception == null) { - // Get the task, forcing initialization if it hasn't already been initialized. - task = this.Task; + ThrowHelper.ThrowArgumentNullException(ExceptionArgument.exception); } + // Get the task, forcing initialization if it hasn't already been initialized. + Task task = this.Task; + // If the exception represents cancellation, cancel the task. Otherwise, fault the task. var oce = exception as OperationCanceledException; bool successfullySet = oce != null ? task.TrySetCanceled(oce.CancellationToken, oce) : task.TrySetException(exception); - // Unlike with TaskCompletionSource, we do not need to spin here until m_task is completed, + // Unlike with TaskCompletionSource, we do not need to spin here until _taskAndStateMachine is completed, // since AsyncTaskMethodBuilder.SetException should not be immediately followed by any code // that depends on the task having completely completed. Moreover, with correct usage, // SetResult or SetException should only be called once, so the Try* methods should always // return true, so no spinning would be necessary anyway (the spinning in TCS is only relevant // if another thread completes the task first). - if (!successfullySet) { ThrowHelper.ThrowInvalidOperationException(ExceptionResource.TaskT_TransitionToFinal_AlreadyCompleted); @@ -699,7 +667,17 @@ public void SetException(Exception exception) internal void SetNotificationForWaitCompletion(bool enabled) { // Get the task (forcing initialization if not already initialized), and set debug notification - this.Task.SetNotificationForWaitCompletion(enabled); + (m_task ?? InitializeTaskAsStateMachineBox()).SetNotificationForWaitCompletion(enabled); + + // NOTE: It's important that the debugger use builder.SetNotificationForWaitCompletion + // rather than builder.Task.SetNotificationForWaitCompletion. Even though the latter will + // lazily-initialize the task as well, it'll initialize it to a Task (which is important + // to minimize size for cases where an ATMB is used directly by user code to avoid the + // allocation overhead of a TaskCompletionSource). If that's done prior to the first await, + // the GetMoveNextDelegate code, which needs an AsyncStateMachineBox, will end up creating + // a new box and overwriting the previously created task. That'll change the object identity + // of the task being used for wait completion notification, and no notification will + // ever arrive, breaking step-out behavior when stepping out before the first yielding await. } /// @@ -708,9 +686,9 @@ internal void SetNotificationForWaitCompletion(bool enabled) /// /// This property lazily instantiates the ID in a non-thread-safe manner. /// It must only be used by the debugger and tracing purposes, and only in a single-threaded manner - /// when no other threads are in the middle of accessing this property or this.Task. + /// when no other threads are in the middle of accessing this or other members that lazily initialize the task. /// - private object ObjectIdForDebugger { get { return this.Task; } } + internal object ObjectIdForDebugger => m_task ?? InitializeTaskAsStateMachineBox(); /// /// Gets a task for the specified result. This will either @@ -721,10 +699,6 @@ internal void SetNotificationForWaitCompletion(bool enabled) [MethodImpl(MethodImplOptions.AggressiveInlining)] // method looks long, but for a given TResult it results in a relatively small amount of asm internal static Task GetTaskForResult(TResult result) { - Contract.Ensures( - EqualityComparer.Default.Equals(result, Contract.Result>().Result), - "The returned task's Result must return the same value as the specified result value."); - // The goal of this function is to be give back a cached task if possible, // or to otherwise give back a new task. To give back a cached task, // we need to be able to evaluate the incoming result value, and we need @@ -837,144 +811,46 @@ private static Task[] CreateInt32Tasks() /// Specifies the result type. /// The result for the task. /// The cacheable task. - internal static Task CreateCacheableTask(TResult result) - { - return new Task(false, result, (TaskCreationOptions)InternalTaskOptions.DoNotDispose, default(CancellationToken)); - } + internal static Task CreateCacheableTask(TResult result) => + new Task(false, result, (TaskCreationOptions)InternalTaskOptions.DoNotDispose, default(CancellationToken)); } - /// Holds state related to the builder's IAsyncStateMachine. - /// This is a mutable struct. Be very delicate with it. - internal struct AsyncMethodBuilderCore + /// + /// An interface implemented by to allow access + /// non-generically to state associated with a builder and state machine. + /// + interface IDebuggingAsyncStateMachineAccessor { - /// A reference to the heap-allocated state machine object associated with this builder. - internal IAsyncStateMachine m_stateMachine; - /// A cached Action delegate used when dealing with a default ExecutionContext. - internal Action m_defaultContextAction; - - // This method is copy&pasted into the public Start methods to avoid size overhead of valuetype generic instantiations. - // Ideally, we would build intrinsics to get the raw ref address and raw code address of MoveNext, and just use the shared implementation. - - /// Associates the builder with the state machine it represents. - /// The heap-allocated state machine object. - /// The argument was null (Nothing in Visual Basic). - /// The builder is incorrectly initialized. - public void SetStateMachine(IAsyncStateMachine stateMachine) - { - if (stateMachine == null) ThrowHelper.ThrowArgumentNullException(ExceptionArgument.stateMachine); - Contract.EndContractBlock(); - if (m_stateMachine != null) ThrowHelper.ThrowInvalidOperationException(ExceptionResource.AsyncMethodBuilder_InstanceNotInitialized); - m_stateMachine = stateMachine; - } - - /// - /// Gets the Action to use with an awaiter's OnCompleted or UnsafeOnCompleted method. - /// On first invocation, the supplied state machine will be boxed. - /// - /// Specifies the type of the method builder used. - /// Specifies the type of the state machine used. - /// The builder. - /// The state machine. - /// An Action to provide to the awaiter. - internal Action GetCompletionAction(Task taskForTracing, ref MoveNextRunner runnerToInitialize) - { - Debug.Assert(m_defaultContextAction == null || m_stateMachine != null, - "Expected non-null m_stateMachine on non-null m_defaultContextAction"); - - // Alert a listening debugger that we can't make forward progress unless it slips threads. - // If we don't do this, and a method that uses "await foo;" is invoked through funceval, - // we could end up hooking up a callback to push forward the async method's state machine, - // the debugger would then abort the funceval after it takes too long, and then continuing - // execution could result in another callback being hooked up. At that point we have - // multiple callbacks registered to push the state machine, which could result in bad behavior. - Debugger.NotifyOfCrossThreadDependency(); - - // The builder needs to flow ExecutionContext, so capture it. - var capturedContext = ExecutionContext.Capture(); - - // If the ExecutionContext is the default context, try to use a cached delegate, creating one if necessary. - Action action; - MoveNextRunner runner; - if (capturedContext == ExecutionContext.Default) - { - // Get the cached delegate, and if it's non-null, return it. - action = m_defaultContextAction; - if (action != null) - { - Debug.Assert(m_stateMachine != null, "If the delegate was set, the state machine should have been as well."); - return action; - } - - // There wasn't a cached delegate, so create one and cache it. - // The delegate won't be usable until we set the MoveNextRunner's target state machine. - runner = new MoveNextRunner(m_stateMachine); - - action = new Action(runner.RunWithDefaultContext); - if (taskForTracing != null) - { - action = OutputAsyncCausalityEvents(taskForTracing, action); - } - m_defaultContextAction = action; - } - // Otherwise, create an Action that flows this context. The context may be null. - // The delegate won't be usable until we set the MoveNextRunner's target state machine. - else - { - var runnerWithContext = new MoveNextRunnerWithContext(capturedContext, m_stateMachine); - runner = runnerWithContext; - action = new Action(runnerWithContext.RunWithCapturedContext); - - if (taskForTracing != null) - { - action = OutputAsyncCausalityEvents(taskForTracing, action); - } - - // NOTE: If capturedContext is null, we could create the Action to point directly - // to m_stateMachine.MoveNext. However, that follows a much more expensive - // delegate creation path. - } - - if (m_stateMachine == null) - runnerToInitialize = runner; - - return action; - } + /// Gets the state machine as a boxed object. This should only be used for debugging purposes. + IAsyncStateMachine GetStateMachineObject(); + } - private Action OutputAsyncCausalityEvents(Task innerTask, Action continuation) - { - return CreateContinuationWrapper(continuation, () => + /// Shared helpers for manipulating state related to async state machines. + internal static class AsyncMethodBuilderCore // debugger depends on this exact name + { + internal static Action OutputAsyncCausalityEvents(Task task, Action continuation) => + CreateContinuationWrapper(continuation, (innerContinuation, innerTask) => { AsyncCausalityTracer.TraceSynchronousWorkStart(CausalityTraceLevel.Required, innerTask.Id, CausalitySynchronousWork.Execution); - - // Invoke the original continuation - continuation.Invoke(); - + innerContinuation.Invoke(); // Invoke the original continuation AsyncCausalityTracer.TraceSynchronousWorkCompletion(CausalityTraceLevel.Required, CausalitySynchronousWork.Execution); - }, innerTask); - } - - internal void PostBoxInitialization(IAsyncStateMachine stateMachine, MoveNextRunner runner, Task builtTask) - { - if (builtTask != null) - { - if (AsyncCausalityTracer.LoggingOn) - AsyncCausalityTracer.TraceOperationCreation(CausalityTraceLevel.Required, builtTask.Id, "Async: " + stateMachine.GetType().Name, 0); - - if (System.Threading.Tasks.Task.s_asyncDebuggingEnabled) - System.Threading.Tasks.Task.AddToActiveTasks(builtTask); - } - - m_stateMachine = stateMachine; - m_stateMachine.SetStateMachine(m_stateMachine); + }, task); - Debug.Assert(runner.m_stateMachine == null, "The runner's state machine should not yet have been populated."); - Debug.Assert(m_stateMachine != null, "The builder's state machine field should have been initialized."); + internal static Action CreateContinuationWrapper(Action continuation, Action invokeAction, Task innerTask) => + new ContinuationWrapper(continuation, invokeAction, innerTask).Invoke; - // Now that we have the state machine, store it into the runner that the action delegate points to. - // And return the action. - runner.m_stateMachine = m_stateMachine; // only after this line is the Action delegate usable + internal static Action TryGetStateMachineForDebugger(Action action) // debugger depends on this exact name/signature + { + object target = action.Target; + return + target is IDebuggingAsyncStateMachineAccessor sm ? sm.GetStateMachineObject().MoveNext : + target is ContinuationWrapper cw ? TryGetStateMachineForDebugger(cw._continuation) : + action; } + internal static Task TryGetContinuationTask(Action continuation) => + (continuation?.Target as ContinuationWrapper)?._innerTask; + /// Throws the exception on the ThreadPool. /// The exception to propagate. /// The target context on which to propagate the exception. Null to use the ThreadPool. @@ -1010,59 +886,6 @@ internal static void ThrowAsync(Exception exception, SynchronizationContext targ } } - /// Provides the ability to invoke a state machine's MoveNext method under a supplied ExecutionContext. - internal sealed class MoveNextRunnerWithContext : MoveNextRunner - { - /// The context with which to run MoveNext. - private readonly ExecutionContext m_context; - - /// Initializes the runner. - /// The context with which to run MoveNext. - internal MoveNextRunnerWithContext(ExecutionContext context, IAsyncStateMachine stateMachine) : base(stateMachine) - { - m_context = context; - } - - /// Invokes MoveNext under the provided context. - internal void RunWithCapturedContext() - { - Debug.Assert(m_stateMachine != null, "The state machine must have been set before calling Run."); - - if (m_context != null) - { - // Use the context and callback to invoke m_stateMachine.MoveNext. - ExecutionContext.Run(m_context, InvokeMoveNextCallback, m_stateMachine); - } - else - { - m_stateMachine.MoveNext(); - } - } - } - - /// Provides the ability to invoke a state machine's MoveNext method. - internal class MoveNextRunner - { - /// The state machine whose MoveNext method should be invoked. - internal IAsyncStateMachine m_stateMachine; - - /// Initializes the runner. - internal MoveNextRunner(IAsyncStateMachine stateMachine) - { - m_stateMachine = stateMachine; - } - - /// Invokes MoveNext under the default context. - internal void RunWithDefaultContext() - { - Debug.Assert(m_stateMachine != null, "The state machine must have been set before calling Run."); - ExecutionContext.Run(ExecutionContext.Default, InvokeMoveNextCallback, m_stateMachine); - } - - /// Gets a delegate to the InvokeMoveNext method. - protected static readonly ContextCallback InvokeMoveNextCallback = sm => ((IAsyncStateMachine)sm).MoveNext(); - } - /// /// Logically we pass just an Action (delegate) to a task for its action to 'ContinueWith' when it completes. /// However debuggers and profilers need more information about what that action is. (In particular what @@ -1071,66 +894,23 @@ internal void RunWithDefaultContext() /// (like the action after that (which is also a ContinuationWrapper and thus form a linked list). // We also store that task if the action is associate with at task. /// - private class ContinuationWrapper - { - internal readonly Action m_continuation; // This is continuation which will happen after m_invokeAction (and is probably a ContinuationWrapper) - private readonly Action m_invokeAction; // This wrapper is an action that wraps another action, this is that Action. - internal readonly Task m_innerTask; // If the continuation is logically going to invoke a task, this is that task (may be null) - - internal ContinuationWrapper(Action continuation, Action invokeAction, Task innerTask) - { - Contract.Requires(continuation != null, "Expected non-null continuation"); - - // If we don't have a task, see if our continuation is a wrapper and use that. - if (innerTask == null) - innerTask = TryGetContinuationTask(continuation); - - m_continuation = continuation; - m_innerTask = innerTask; - m_invokeAction = invokeAction; - } - - internal void Invoke() - { - m_invokeAction(); - } - } - - internal static Action CreateContinuationWrapper(Action continuation, Action invokeAction, Task innerTask = null) + private sealed class ContinuationWrapper { - return new ContinuationWrapper(continuation, invokeAction, innerTask).Invoke; - } + private readonly Action _invokeAction; // This wrapper is an action that wraps another action, this is that Action. + internal readonly Action _continuation; // This is continuation which will happen after m_invokeAction (and is probably a ContinuationWrapper) + internal readonly Task _innerTask; // If the continuation is logically going to invoke a task, this is that task (may be null) - internal static Action TryGetStateMachineForDebugger(Action action) - { - object target = action.Target; - var runner = target as AsyncMethodBuilderCore.MoveNextRunner; - if (runner != null) + internal ContinuationWrapper(Action continuation, Action invokeAction, Task innerTask) { - return new Action(runner.m_stateMachine.MoveNext); - } + Debug.Assert(continuation != null, "Expected non-null continuation"); + Debug.Assert(invokeAction != null, "Expected non-null continuation"); - var continuationWrapper = target as ContinuationWrapper; - if (continuationWrapper != null) - { - return TryGetStateMachineForDebugger(continuationWrapper.m_continuation); + _invokeAction = invokeAction; + _continuation = continuation; + _innerTask = innerTask ?? TryGetContinuationTask(continuation); // if we don't have a task, see if our continuation is a wrapper and use that. } - return action; - } - - /// - /// Given an action, see if it is a continuation wrapper and has a Task associated with it. If so return it (null otherwise) - /// - internal static Task TryGetContinuationTask(Action action) - { - if (action != null) - { - var asWrapper = action.Target as ContinuationWrapper; - if (asWrapper != null) - return asWrapper.m_innerTask; - } - return null; + internal void Invoke() => _invokeAction(_continuation, _innerTask); } } } diff --git a/src/mscorlib/src/System/Runtime/CompilerServices/TaskAwaiter.cs b/src/mscorlib/src/System/Runtime/CompilerServices/TaskAwaiter.cs index c35658e54cdc..93bedd62b6f3 100644 --- a/src/mscorlib/src/System/Runtime/CompilerServices/TaskAwaiter.cs +++ b/src/mscorlib/src/System/Runtime/CompilerServices/TaskAwaiter.cs @@ -249,39 +249,42 @@ private static Action OutputWaitEtwEvents(Task task, Action continuation) // is enabled, and in doing so it allows us to pass the awaited task's information into the end event // in a purely pay-for-play manner (the alternatively would be to increase the size of TaskAwaiter // just for this ETW purpose, not pay-for-play, since GetResult would need to know whether a real yield occurred). - return AsyncMethodBuilderCore.CreateContinuationWrapper(continuation, () => + return AsyncMethodBuilderCore.CreateContinuationWrapper(continuation, (innerContinuation,innerTask) => { if (Task.s_asyncDebuggingEnabled) { - Task.RemoveFromActiveTasks(task.Id); + Task.RemoveFromActiveTasks(innerTask.Id); } + TplEtwProvider innerEtwLog = TplEtwProvider.Log; + // ETW event for Task Wait End. Guid prevActivityId = new Guid(); - bool bEtwLogEnabled = etwLog.IsEnabled(); + bool bEtwLogEnabled = innerEtwLog.IsEnabled(); if (bEtwLogEnabled) { var currentTaskAtEnd = Task.InternalCurrent; - etwLog.TaskWaitEnd( + innerEtwLog.TaskWaitEnd( (currentTaskAtEnd != null ? currentTaskAtEnd.m_taskScheduler.Id : TaskScheduler.Default.Id), (currentTaskAtEnd != null ? currentTaskAtEnd.Id : 0), - task.Id); + innerTask.Id); // Ensure the continuation runs under the activity ID of the task that completed for the // case the antecendent is a promise (in the other cases this is already the case). - if (etwLog.TasksSetActivityIds && (task.Options & (TaskCreationOptions)InternalTaskOptions.PromiseTask) != 0) - EventSource.SetCurrentThreadActivityId(TplEtwProvider.CreateGuidForTaskID(task.Id), out prevActivityId); + if (innerEtwLog.TasksSetActivityIds && (innerTask.Options & (TaskCreationOptions)InternalTaskOptions.PromiseTask) != 0) + EventSource.SetCurrentThreadActivityId(TplEtwProvider.CreateGuidForTaskID(innerTask.Id), out prevActivityId); } + // Invoke the original continuation provided to OnCompleted. - continuation(); + innerContinuation(); if (bEtwLogEnabled) { - etwLog.TaskWaitContinuationComplete(task.Id); - if (etwLog.TasksSetActivityIds && (task.Options & (TaskCreationOptions)InternalTaskOptions.PromiseTask) != 0) + innerEtwLog.TaskWaitContinuationComplete(innerTask.Id); + if (innerEtwLog.TasksSetActivityIds && (innerTask.Options & (TaskCreationOptions)InternalTaskOptions.PromiseTask) != 0) EventSource.SetCurrentThreadActivityId(prevActivityId); } - }); + }, task); } } diff --git a/src/mscorlib/src/System/Runtime/CompilerServices/YieldAwaitable.cs b/src/mscorlib/src/System/Runtime/CompilerServices/YieldAwaitable.cs index f1c777252632..3d9e6df673d4 100644 --- a/src/mscorlib/src/System/Runtime/CompilerServices/YieldAwaitable.cs +++ b/src/mscorlib/src/System/Runtime/CompilerServices/YieldAwaitable.cs @@ -124,26 +124,26 @@ private static Action OutputCorrelationEtwEvent(Action continuation) // fire the correlation ETW event TplEtwProvider.Log.AwaitTaskContinuationScheduled(TaskScheduler.Current.Id, (currentTask != null) ? currentTask.Id : 0, continuationId); - return AsyncMethodBuilderCore.CreateContinuationWrapper(continuation, () => + return AsyncMethodBuilderCore.CreateContinuationWrapper(continuation, (innerContinuation,continuationIdTask) => { var etwLog = TplEtwProvider.Log; - etwLog.TaskWaitContinuationStarted(continuationId); + etwLog.TaskWaitContinuationStarted(((Task)continuationIdTask).Result); // ETW event for Task Wait End. Guid prevActivityId = new Guid(); // Ensure the continuation runs under the correlated activity ID generated above if (etwLog.TasksSetActivityIds) - EventSource.SetCurrentThreadActivityId(TplEtwProvider.CreateGuidForTaskID(continuationId), out prevActivityId); + EventSource.SetCurrentThreadActivityId(TplEtwProvider.CreateGuidForTaskID(((Task)continuationIdTask).Result), out prevActivityId); // Invoke the original continuation provided to OnCompleted. - continuation(); + innerContinuation(); // Restore activity ID if (etwLog.TasksSetActivityIds) EventSource.SetCurrentThreadActivityId(prevActivityId); - etwLog.TaskWaitContinuationComplete(continuationId); - }); + etwLog.TaskWaitContinuationComplete(((Task)continuationIdTask).Result); + }, Task.FromResult(continuationId)); // pass the ID in a task to avoid a closure } /// WaitCallback that invokes the Action supplied as object state.