Save/restore contexts for custom notifiers

[jakobbotsch]

With #121448 we are restoring contexts as we unwind during suspension. However, since we delay the call to OnCompleted, we need to restore the leaf contexts explicitly now.

[dotnet-policy-service]

Tagging subscribers to this area: @JulieLeeMSFT, @jakobbotsch
See info in area-owners.md if you want to be subscribed.

[VSadov]

The Push/Pop of the contexts in Task-returning thunks is not completely useless, then.

We can move the try/finally to be just around FinalizeTaskReturningThunk (as a separate change from this PR), but it seems still needed.

Wrapping only the FinalizeTaskReturningThunk in try/finally would also make sure that TaskFromException runs in the context that comes from the callee. I am not sure TaskFromException cares about the context, but better to not rely on that.

[Copilot]

Pull Request Overview

This PR implements context capture and restoration for custom notifiers in the runtime async system. Since PR #121448 introduced context restoration during unwinding, and the call to OnCompleted is now delayed, leaf contexts must be explicitly restored. The changes also enable runtime async support by default.

Key changes:

• Added context capture before suspension in AwaitAwaiter, UnsafeAwaitAwaiter, and TransparentAwait (for ValueTaskSource only)
• Added context restoration before calling OnCompleted/UnsafeOnCompleted on custom notifiers
• Enabled runtime async by default and removed test disabling configuration

Reviewed Changes

Copilot reviewed 4 out of 4 changed files in this pull request and generated no comments.

File	Description
src/tests/async/Directory.Build.targets	Removed file that was disabling async tests, enabling them to run now that runtime async is ready
src/libraries/System.Private.CoreLib/src/System/Runtime/CompilerServices/AsyncHelpers.cs	Added context capture calls in AwaitAwaiter and UnsafeAwaitAwaiter before suspension
src/coreclr/inc/clrconfigvalues.h	Changed RuntimeAsync config default from 0 to 1, enabling runtime async by default
src/coreclr/System.Private.CoreLib/src/System/Runtime/CompilerServices/AsyncHelpers.CoreCLR.cs	Added ExecutionContext/SynchronizationContext fields to state struct, implemented CaptureContexts() method, added context capture in TransparentAwait for ValueTaskSource, and added context restoration before notifier callbacks in HandleSuspended

[VSadov]

LGTM. Thanks!!

[jakobbotsch]

Now we have some failures in #119432 related to too many context restores. Specifically in SocketAsyncEventArgs_ExecutionContextFlowsAcrossSendAsyncOperation and System.Net.Sockets.Tests.ExecutionContextFlowTest.ExecutionContext_FlowsOnlyOnceAcrossAsyncOperations .

The former I think is just a bug. With the current approach CaptureContexts should be using ExecutionContext.Capture.

The latter makes assumptions about the upper bound on how many times the context is restored. It assumes at most 1 restore per suspension, while with this change I think the leaf context may be flowed twice.

I think the best fix to both is that we change the suspension restores to skip notifying callbacks for ExecutionContext. It means we will temporarily put an ExecutionContext inside Thread.CurrentThread that we did not notify about, but since we control all the code that runs during suspension I think that's ok.

We will avoid notifying for an ExecutionContext in the thread that we know is not going to be accessed and that we know will be replaced immediately anyway. That's the kind of optimization that we want to be able to make in runtime async anyway. In the future we are going to want to optimize away the context saves and restores when the JIT can prove that they are not accessed, and that too will result in changes in the restore notifications.

[jakobbotsch]

I applied the changes above and also pushed it to #119432 for testing.

[jakobbotsch]

It looks like libraries tests are passing in #119432 with this change now.

@VSadov Can you take another look at this PR with the change added above?

[VSadov]

In the future we are going to want to optimize away the context saves and restores when the JIT can prove that they are not accessed, and that too will result in changes in the restore notifications.

I think that is valid optimization already because if exec context is not accessed it cannot change. Notifications are always sent like:

if (previousExecCtx != currentExecCtx)
{
    ExecutionContext.RestoreChangedContextToThread(thread, previousExecCtx, currentExecCtx);
}

Regardless of that, context manipulations for the purpose of the infrastructure should not be visible to the user code.

[VSadov]

Also, I think I have seen failures related to exec contexts in sockets even before the restore moved from call site to callee. Not sure if that were exactly same tests, but we had some issues with exec contexts even before.
I just hopped they could be sideeffects of other known issues with contexts and would go away once known issues are fixed, so did not log them.

[jakobbotsch]

In the future we are going to want to optimize away the context saves and restores when the JIT can prove that they are not accessed, and that too will result in changes in the restore notifications.

I think that is valid optimization already because if exec context is not accessed it cannot change. Notifications are always sent like:

if (previousExecCtx != currentExecCtx)
{
    ExecutionContext.RestoreChangedContextToThread(thread, previousExecCtx, currentExecCtx);
}

Regardless of that, context manipulations for the purpose of the infrastructure should not be visible to the user code.

Hmm yes, I think that's true.