From 0526339a302da4918699901d5fc403044374651b Mon Sep 17 00:00:00 2001 From: Stephen Toub Date: Tue, 17 Oct 2017 16:01:57 -0400 Subject: [PATCH] Avoid TaskWaitBegin/End events in Task.Wait on already completed task We are unnecessarily firing the TaskWaitBegin/End EventSource event when the task has already completed if it's faulted/canceled. We should only fire it if the task hasn't completed by the time we check. --- src/mscorlib/src/System/Threading/Tasks/Task.cs | 45 +++++++++++++------------ 1 file changed, 23 insertions(+), 22 deletions(-) diff --git a/src/mscorlib/src/System/Threading/Tasks/Task.cs b/src/mscorlib/src/System/Threading/Tasks/Task.cs index aed2c3b..2745f3a 100644 --- a/src/mscorlib/src/System/Threading/Tasks/Task.cs +++ b/src/mscorlib/src/System/Threading/Tasks/Task.cs @@ -2879,6 +2879,13 @@ namespace System.Threading.Tasks // to be able to see the method on the stack and inspect arguments). private bool InternalWaitCore(int millisecondsTimeout, CancellationToken cancellationToken) { + // If the task has already completed, there's nothing to wait for. + bool returnValue = IsCompleted; + if (returnValue) + { + return true; + } + // ETW event for Task Wait Begin var etwLog = TplEtwProvider.Log; bool etwIsEnabled = etwLog.IsEnabled(); @@ -2890,30 +2897,24 @@ namespace System.Threading.Tasks this.Id, TplEtwProvider.TaskWaitBehavior.Synchronous, 0); } - bool returnValue = IsCompleted; + // Alert a listening debugger that we can't make forward progress unless it slips threads. + // We call NOCTD for two reasons: + // 1. If the task runs on another thread, then we'll be blocked here indefinitely. + // 2. If the task runs inline but takes some time to complete, it will suffer ThreadAbort with possible state corruption, + // and it is best to prevent this unless the user explicitly asks to view the value with thread-slipping enabled. + Debugger.NotifyOfCrossThreadDependency(); - // If the event hasn't already been set, we will wait. - if (!returnValue) + // We will attempt inline execution only if an infinite wait was requested + // Inline execution doesn't make sense for finite timeouts and if a cancellation token was specified + // because we don't know how long the task delegate will take. + if (millisecondsTimeout == Timeout.Infinite && !cancellationToken.CanBeCanceled && + WrappedTryRunInline() && IsCompleted) // TryRunInline doesn't guarantee completion, as there may be unfinished children. { - // Alert a listening debugger that we can't make forward progress unless it slips threads. - // We call NOCTD for two reasons: - // 1. If the task runs on another thread, then we'll be blocked here indefinitely. - // 2. If the task runs inline but takes some time to complete, it will suffer ThreadAbort with possible state corruption, - // and it is best to prevent this unless the user explicitly asks to view the value with thread-slipping enabled. - Debugger.NotifyOfCrossThreadDependency(); - - // We will attempt inline execution only if an infinite wait was requested - // Inline execution doesn't make sense for finite timeouts and if a cancellation token was specified - // because we don't know how long the task delegate will take. - if (millisecondsTimeout == Timeout.Infinite && !cancellationToken.CanBeCanceled && - WrappedTryRunInline() && IsCompleted) // TryRunInline doesn't guarantee completion, as there may be unfinished children. - { - returnValue = true; - } - else - { - returnValue = SpinThenBlockingWait(millisecondsTimeout, cancellationToken); - } + returnValue = true; + } + else + { + returnValue = SpinThenBlockingWait(millisecondsTimeout, cancellationToken); } Debug.Assert(IsCompleted || millisecondsTimeout != Timeout.Infinite); -- 2.7.4