From aa4197400ed4ea1d4ffb421e55712a01b40673fa Mon Sep 17 00:00:00 2001 From: Stephen Toub Date: Tue, 20 Nov 2018 05:08:00 -0500 Subject: [PATCH] Improve async iterator exception stack traces (#21103) - The current stack trace prettying logic assumes that a method only has one StateMachineAttribute, but that's no longer true with async iterators, which (at least with the current implementation) can have both an AsyncStateMachineAttribute and an IteratorStateMachineAttribute. As such, we change the prettying logic to look through all StateMachineAttributes rather than stopping at the first one. - ManualResetValueTaskSourceCore.GetResult is responsible for propagating exceptions, but it's infrastructure and isn't particularly meaningful in a call stack. Hide it from the stack trace, as we do for other such GetResult methods in Task and ValueTask. --- .../Tasks/Sources/ManualResetValueTaskSourceCore.cs | 2 ++ .../src/System/Diagnostics/Stacktrace.cs | 18 +++++++++++++----- 2 files changed, 15 insertions(+), 5 deletions(-) diff --git a/src/System.Private.CoreLib/shared/System/Threading/Tasks/Sources/ManualResetValueTaskSourceCore.cs b/src/System.Private.CoreLib/shared/System/Threading/Tasks/Sources/ManualResetValueTaskSourceCore.cs index ff9d3df..b9e58d9 100644 --- a/src/System.Private.CoreLib/shared/System/Threading/Tasks/Sources/ManualResetValueTaskSourceCore.cs +++ b/src/System.Private.CoreLib/shared/System/Threading/Tasks/Sources/ManualResetValueTaskSourceCore.cs @@ -88,6 +88,7 @@ namespace System.Threading.Tasks.Sources /// Gets the result of the operation. /// Opaque value that was provided to the 's constructor. + [StackTraceHidden] public TResult GetResult(short token) { ValidateToken(token); @@ -265,6 +266,7 @@ namespace System.Threading.Tasks.Sources internal static class ManualResetValueTaskSourceCoreShared // separated out of generic to avoid unnecessary duplication { + [StackTraceHidden] internal static void ThrowInvalidOperationException() => throw new InvalidOperationException(); internal static readonly Action s_sentinel = CompletionSentinel; diff --git a/src/System.Private.CoreLib/src/System/Diagnostics/Stacktrace.cs b/src/System.Private.CoreLib/src/System/Diagnostics/Stacktrace.cs index 8389494..1679531 100644 --- a/src/System.Private.CoreLib/src/System/Diagnostics/Stacktrace.cs +++ b/src/System.Private.CoreLib/src/System/Diagnostics/Stacktrace.cs @@ -508,17 +508,25 @@ namespace System.Diagnostics continue; } + bool foundAttribute = false, foundIteratorAttribute = false; foreach (StateMachineAttribute asma in attributes) { if (asma.StateMachineType == declaringType) { - method = candidateMethod; - declaringType = candidateMethod.DeclaringType; - // Mark the iterator as changed; so it gets the + annotation of the original method - // async statemachines resolve directly to their builder methods so aren't marked as changed - return asma is IteratorStateMachineAttribute; + foundAttribute = true; + foundIteratorAttribute |= asma is IteratorStateMachineAttribute; } } + + if (foundAttribute) + { + // If this is an iterator (sync or async), mark the iterator as changed, so it gets the + annotation + // of the original method. Non-iterator async state machines resolve directly to their builder methods + // so aren't marked as changed. + method = candidateMethod; + declaringType = candidateMethod.DeclaringType; + return foundIteratorAttribute; + } } return false; -- 2.7.4