Improve async iterator exception stack traces (#21103)
authorStephen Toub <stoub@microsoft.com>
Tue, 20 Nov 2018 10:08:00 +0000 (05:08 -0500)
committerGitHub <noreply@github.com>
Tue, 20 Nov 2018 10:08:00 +0000 (05:08 -0500)
- 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.

src/System.Private.CoreLib/shared/System/Threading/Tasks/Sources/ManualResetValueTaskSourceCore.cs
src/System.Private.CoreLib/src/System/Diagnostics/Stacktrace.cs

index ff9d3df..b9e58d9 100644 (file)
@@ -88,6 +88,7 @@ namespace System.Threading.Tasks.Sources
 
         /// <summary>Gets the result of the operation.</summary>
         /// <param name="token">Opaque value that was provided to the <see cref="ValueTask"/>'s constructor.</param>
+        [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<object> s_sentinel = CompletionSentinel;
index 8389494..1679531 100644 (file)
@@ -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;