Kestrel is causing AsyncLocal to bleed context data across requests on same HTTP/1.1 connection

[passuied]

I have raised this issue with the coreclr repo but it seems to be caused by Kestrel as I cannot reproduce it with IISExpress or TestHost.

https://github.com/dotnet/coreclr/issues/26713

Under the scenario described in the issue above and recreated in the provided repo, data stored within an AsyncLocal instance within a middleware is bleeding across requests.

Is this a bug?

[davidfowl]

It is an issue that we'll consider patching. The async local state is usually reverted before leaving It's happening in your case because the async local is being set lazily in your SecurityTokenAccessor here.

I'd change this logic to only set the async local in the middleware, not lazily as that leads to a pattern where the context may bleed out of the method doing the set of the async local (which is the case here).

Kestrel should be consistently exposing a clean ExecutionContext per request but in some cases it's possible for the context from the application to bleed into Kestrel and be re-used on the next request.

This doesn't usually show up because async locals are usually:

• Unconditionally overridden in middleware to the new value
• Reset on the exit of middleware

What makes this even trickier is that it won't show up if there's any async method (method with the async keyword) that runs in your code's call chain. Async methods properly revert the ExecutionContext before the method exits.

[passuied]

Interesting.

So is it not recommended to wrap AsyncLocal?
Isn't this how HttpContextAccessor deals with it?

Also, on your last point, since the middleware is async doesnt it contradict your statement?

[davidfowl]

So is it not recommended to wrap AsyncLocal?

I'm not sure what you mean. I just meant that you should set it in the middle instead of in the property getter. That way the lifetime is well defined.

Isn't this how HttpContextAccessor deals with it?

The HttpContextAccessor is set on request start and cleared on request end.

Also, on your last point, since the middleware is async doesnt it contradict your statement?

No, I was being very specific, the method that sets the async local should be async. In your case its getting set here as part of resolving the ILog in your middleware Invoke method (https://github.com/passuied/TestAsyncLocalMiddleware/blob/36e038cf17e16453e9e9f916c14f13ceeea3de51/TestAsyncLocalMiddleware/Infrastructure/ContextBuilder.cs#L27)

[passuied]

Gotcha, thanks for the clarification.
I have changed the code in this branch and it resolved the issue: https://github.com/passuied/TestAsyncLocalMiddleware/pull/1/files