API Proposal: ExecutionContext.Restore(ExecutionContext? executionContext)

[benaadams]

Background and Motivation

Alternative to the approved api #30867 "ExecutionContext.Run ref overloads"

awaiting a Task returning method that is itself not async does not guarantee undoing AsyncLocal/ExecutionContext changes (as it is the responsibility of the callee)

ExecutionContext does provide an callback based method that acts as a stack:

public sealed class ExecutionContext
{
    ExecutionContext.Run(ExecutionContext executionContext, ContextCallback callback, object? state)
}

However using it to just undo changes is clunky (involves rearranging code into a callback); as well as applying the current context to run on, when its already on the current context.

It also doesn't provide a return value; and if it was guarding multiple awaits arranging to return a single Task (via using state as a reference) becomes complicated.

Rather than using it in a stack based "apply=>undo" manner; it would be useful if it could be restored directly inline.

Related ASP.NET issues dotnet/aspnetcore#15384, dotnet/aspnetcore#13991

Proposed API

namespace System.Threading
{
    public sealed class ExecutionContext
    {
        public static void Restore(ExecutionContext? executionContext);
    }
}

Usage Examples

var ec = ExecutionContext.Capture();
while (!cancelled)
{
    await NextRequest();

    await OnStartAsync();

    await ProcessRequestAsync();

    await OnCompleteAsync();

    ExecutionContext.Restore(ec);
}

Alternative Designs

#30867 "ExecutionContext.Run ref overloads"; this again requires contorting the code to the callback based pattern and is problematic for multiple awaits as regular .Run as well as introducing a large method that has generic code expansion.

Also guard the multiple awaits behind an explicitly async method:

while (!cancelled)
{
    await NextRequest();

    await ProcessUserCode();
}

async ValueTask ProcessUserCode()
{
    await OnStartAsync();

    await ProcessRequestAsync();

    await OnCompleteAsync();
}

However this introduces an extra statemachine per request and an additional allocation per request; as well as causing the ExecutionContext to be captured and restored per request; when really we just want to capture it at the start of request processing and only restore/reset it per request.

/cc @davidfowl @stephentoub

[Dotnet-GitSync-Bot]

I couldn't figure out the best area label to add to this issue. Please help me learn by adding exactly one area label.

[benaadams]

Related change in ASP.NET that would make use of this api dotnet/aspnetcore#23175

[benaadams]

@stephentoub are you happy for this to be for review?

[stephentoub]

@kouvel, any concerns with this being exposed?

[benaadams]

Was a naming question raised #37942 (comment)

Would it make sense to call this method Apply or something similar instead of Restore given how it's used?

[stephentoub]

Either seems fine to me.

[kouvel]

Looks fine to me. I was also wondering if it should be prefixed with Unsafe since it would allow changing the EC arbitrarily which may not have been possible to do before, but can determine in the review.

[terrajobst]

@stephentoub @kouvel This is marked as 6.0. If we were to approve this this week, would it still make sense for 5.0?

[benaadams]

This is marked as 6.0. If we were to approve this this week, would it still make sense for 5.0?

Would mean the upstream change in aspnet core could be merged for 5.0 dotnet/aspnetcore#23175

/cc @davidfowl

[stephentoub]

If we were to approve this this week, would it still make sense for 5.0?

Yes

[terrajobst]

Looks good as proposed.

namespace System.Threading
{
    public sealed class ExecutionContext
    {
        public static void Restore(ExecutionContext? executionContext);
    }
}