Skip to content

Skipping EDI in the runtime-async case as well#128735

Open
rcj1 wants to merge 1 commit into
dotnet:mainfrom
rcj1:fix-128723
Open

Skipping EDI in the runtime-async case as well#128735
rcj1 wants to merge 1 commit into
dotnet:mainfrom
rcj1:fix-128723

Conversation

@rcj1
Copy link
Copy Markdown
Contributor

@rcj1 rcj1 commented May 29, 2026

In stacktrace pretty-printing we skip the EDI boundary for async methods, so as not to clutter stacktraces with --- End of stack trace from previous location ---. We should do the same for runtime-async.

Fixes #128723

Copilot AI review requested due to automatic review settings May 29, 2026 00:22
@dotnet-policy-service
Copy link
Copy Markdown
Contributor

Tagging subscribers to this area: @steveisok, @dotnet/area-system-diagnostics
See info in area-owners.md if you want to be subscribed.

@rcj1 rcj1 requested review from jkotas and richlander May 29, 2026 00:23
@@ -358,7 +358,7 @@ internal void ToString(TraceFormat traceFormat, StringBuilder sb)
}

// Skip EDI boundary for async
if (sf.IsLastFrameFromForeignExceptionStackTrace && !isAsync)
if (sf.IsLastFrameFromForeignExceptionStackTrace && !isAsync && (mb.MethodImplementationFlags & MethodImplAttributes.Async) == 0)
Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

You can move this check to line 240, and update the condition in line 244 to add an !isAsync at the beginning. This way, we can skip some reflection if the method is runtime-async.

Copy link
Copy Markdown
Contributor

Copilot AI left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Pull request overview

Suppresses the --- End of stack trace from previous location --- EDI boundary line for runtime-async methods, matching the existing behavior for compiler-generated async state machines. Fixes #128723 where .NET 11 with runtime-async produced a stack trace differing from the .NET 10 baseline.

Changes:

  • Extend the "skip EDI boundary for async" condition in StackTrace.ToString to also skip when the frame's method has MethodImplAttributes.Async (i.e. a runtime-async method).
Show a summary per file
File Description
src/libraries/System.Private.CoreLib/src/System/Diagnostics/StackTrace.cs Adds MethodImplAttributes.Async check to suppress the EDI boundary marker for runtime-async frames.

Copilot's findings

  • Files reviewed: 1/1 changed files
  • Comments generated: 0

@jkotas
Copy link
Copy Markdown
Member

jkotas commented May 29, 2026

Is it possible to add a test for this?

@jakobbotsch
Copy link
Copy Markdown
Member

What EDI is this marker coming from?

@rcj1
Copy link
Copy Markdown
Contributor Author

rcj1 commented May 29, 2026

What EDI is this marker coming from?

It is from the outer GetResult of the asyncv1 wrapper method.

@jakobbotsch
Copy link
Copy Markdown
Member

This change looks fine and seems equivalent to async v1 behavior. But I wonder if the ideal behavior here would distinguish between the user using EDI inside an async method and our infrastructure use of it. For example, would a check for an async thunk method instead of all async methods work for this case?

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Projects

None yet

Development

Successfully merging this pull request may close these issues.

Exception.ToString is different with .NET 11

5 participants