From 130a4c62a79b3b73b7efdb6d9e082bf0d217d927 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 (dotnet/coreclr#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. Commit migrated from https://github.com/dotnet/coreclr/commit/aa4197400ed4ea1d4ffb421e55712a01b40673fa --- .../src/System/Diagnostics/Stacktrace.cs | 18 +++++++++++++----- .../Tasks/Sources/ManualResetValueTaskSourceCore.cs | 2 ++ 2 files changed, 15 insertions(+), 5 deletions(-) diff --git a/src/coreclr/src/System.Private.CoreLib/src/System/Diagnostics/Stacktrace.cs b/src/coreclr/src/System.Private.CoreLib/src/System/Diagnostics/Stacktrace.cs index 8389494..1679531 100644 --- a/src/coreclr/src/System.Private.CoreLib/src/System/Diagnostics/Stacktrace.cs +++ b/src/coreclr/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; diff --git a/src/libraries/System.Private.CoreLib/src/System/Threading/Tasks/Sources/ManualResetValueTaskSourceCore.cs b/src/libraries/System.Private.CoreLib/src/System/Threading/Tasks/Sources/ManualResetValueTaskSourceCore.cs index ff9d3df..b9e58d9 100644 --- a/src/libraries/System.Private.CoreLib/src/System/Threading/Tasks/Sources/ManualResetValueTaskSourceCore.cs +++ b/src/libraries/System.Private.CoreLib/src/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; -- 2.7.4