Fix AsyncInstrumentation to honor s_asyncDebuggingEnabled#127233
Fix AsyncInstrumentation to honor s_asyncDebuggingEnabled#127233tommcdon wants to merge 1 commit intodotnet:mainfrom
Conversation
…rDebug When the debugger sets Task.s_asyncDebuggingEnabled directly via ICorDebug (SetManagedTaskEtwEventsEnabled), the AsyncInstrumentation flags system added by PR dotnet#126091 was not aware of this. The new flags only got set via ETW OnEventCommand, so non-ETW RuntimeAsync configs had s_asyncDebuggerActiveFlags stuck at Disabled, preventing NotifyDebuggerOfRuntimeAsyncState from being called. Fix: In InitializeFlags(), check Task.s_asyncDebuggingEnabled as a fallback when s_asyncDebuggerActiveFlags is still Disabled. This ensures the debugger's request is honored regardless of whether ETW events have been enabled. Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
|
Tagging subscribers to this area: @steveisok, @tommcdon, @dotnet/dotnet-diag |
There was a problem hiding this comment.
Pull request overview
This PR updates CoreLib’s AsyncInstrumentation initialization so RuntimeAsync debugging instrumentation is enabled when the debugger toggles Task.s_asyncDebuggingEnabled directly (e.g., via ICorDebug) even if no ETW session ever triggers TplEventSource.OnEventCommand.
Changes:
- In
AsyncInstrumentation.InitializeFlags(), if debugger flags are stillDisabledbutTask.s_asyncDebuggingEnabledistrue, enableDefaultFlags | Flags.AsyncDebuggeras a fallback. - Preserve the existing ETW-driven flag update path (the fallback only applies when
s_asyncDebuggerActiveFlagsis stillDisabled).
| // The debugger may have set Task.s_asyncDebuggingEnabled directly via ICorDebug | ||
| // before any ETW session is established. In that case, s_asyncDebuggerActiveFlags | ||
| // will still be Disabled because OnEventCommand was never called. Honor the | ||
| // debugger's request by enabling default flags when s_asyncDebuggingEnabled is set. | ||
| if (s_asyncDebuggerActiveFlags == Flags.Disabled && Task.s_asyncDebuggingEnabled) | ||
| { | ||
| s_asyncDebuggerActiveFlags = DefaultFlags | Flags.AsyncDebugger; | ||
| } |
There was a problem hiding this comment.
This new fallback behavior (enabling AsyncInstrumentation when only Task.s_asyncDebuggingEnabled is set, without an ETW session) doesn’t appear to be covered by existing runtime-async tests. Consider adding a test that sets Task.s_asyncDebuggingEnabled via reflection without creating a TplEventSource listener, then triggers flag initialization (e.g., via a RuntimeAsyncMethodGeneration async method) and asserts AsyncInstrumentation.s_activeFlags becomes enabled (and that the relevant runtime-async collections are initialized as expected).
| if (s_asyncDebuggerActiveFlags == Flags.Disabled && Task.s_asyncDebuggingEnabled) | ||
| { | ||
| s_asyncDebuggerActiveFlags = DefaultFlags | Flags.AsyncDebugger; | ||
| } |
There was a problem hiding this comment.
With this change, if Task.s_asyncDebuggingEnabled is later cleared (e.g., debugger detach) in a non-ETW scenario where OnEventCommand never calls UpdateAsyncDebuggerFlags(Disabled), s_asyncDebuggerActiveFlags/s_activeFlags will remain non-Disabled and keep routing RuntimeAsyncTask through the instrumented path (even though AsyncDebugger hooks will be gated by Task.s_asyncDebuggingEnabled). If detach/toggling is expected to work without ETW, consider adding a corresponding “disable” path (e.g., re-check Task.s_asyncDebuggingEnabled in SyncActiveFlags/InstrumentCheckPoint or another explicit update mechanism).
|
Let's fix this at debugger side, debugger can set the debugger flag directly in s_asyncDebuggerActiveFlags as done in this fix and it will get picked up when flags initialize synchronize in first async method running through the dispatch continuations loop. Once set o the debugger side we can drop the use of s_asyncDebuggingEnabled inside the asyncv2 instrumentation paths. |
When the debugger sets Task.s_asyncDebuggingEnabled directly via ICorDebug (SetManagedTaskEtwEventsEnabled), the AsyncInstrumentation flags system added by PR #126091 was not aware of this. The new flags only got set via ETW OnEventCommand, so non-ETW RuntimeAsync configs had s_asyncDebuggerActiveFlags stuck at Disabled, preventing NotifyDebuggerOfRuntimeAsyncState from being called.
Fix: In InitializeFlags(), check Task.s_asyncDebuggingEnabled as a fallback when s_asyncDebuggerActiveFlags is still Disabled. This ensures the debugger's request is honored regardless of whether ETW events have been enabled.