3

I don't quite fully understand this situation, where AsyncLocal instance is set at a certain point in the AuthenticationHandler, but does not reach the controller, when it is injected into the constructor.

I've made it similar to how IHttpContextAccessor works, but still nowhere near. However, if I set the AsyncLocal from a Middleware, it reaches the controller. Also, setting the HttpContext.Items property from AuthenticationHandler works just fine.

Question: How is HttpContext able to retain Items property contents all the way, and is ASP.NET runtime disposing the captured ExecutionContext of my DomainContextAccessor for some security reason because of where it is being set at?

I've made a sample app to demo this use case. I'd really appreciate someone shedding the light on this problem.

Ostati
  • 4,623
  • 3
  • 44
  • 48
  • 1
    Everything is HTTP404. I can't see any of your links. – Andy Mar 06 '21 at 00:38
  • Strange... It's not 404 for me. – Ostati Mar 06 '21 at 00:40
  • Yeah, it was private.. Sorry about that. Try now. – Ostati Mar 06 '21 at 00:42
  • 3
    Actually you shouldn't link to external sites and post the relevant sections here on StackOverflow, because this is not your private problem solving platform but meant to be helpful for everybody who comes here at some point in future. But external links/examples that may become invalid or dead links, are useless for future visitors looking for an answer to their problem – Tseng Mar 06 '21 at 04:12

3 Answers3

6

You already have a good answer on "how should I fix this?" Here's more of a description of why it's behaving this way.

AsyncLocal<T> has the same semantics as logging scopes. Because it has those same semantics, I always prefer to use it with an IDisposable, so that the scope is clear and explicit, and there's no weird rules around whether a method is marked async or not.

For specifics on the weird rules, see this. In summary:

  • Writing a new value to an AsyncLocal<T> sets that value in the current scope.
  • Methods marked async will copy their scope to a new scope the first time it's written to (and it's the new scope that is modified).

I've made it similar to how IHttpContextAccessor works, but still nowhere near.

I don't recommend copying the design of IHttpContextAccessor. It works... for that very specific use case. If you want to use AsyncLocal<T>, then use a design like this:

static class MyImplicitValue
{
  private static readonly AsyncLocal<T> Value = new();

  public static T Get() => Value.Value;

  public static IDisposable Set(T newValue)
  {
    var oldValue = Value.Value;
    Value.Value = newValue;
    return new Disposable(() => Value.Value = oldValue);
  }
}

usage:

using (MyImplicitValue.Set(myValue))
{
  // Code in here can get myValue from MyImplicitValue.Get().
}

You can wrap that into an IMyImplicitValueAccessor if desired, but note that any "setter" logic should be using the IDisposable pattern as shown.

AsyncLocal instance is set at a certain point in the AuthenticationHandler, but does not reach the controller

That's because your AuthenticationHandler sets the value but doesn't call the controller after setting that value (and it shouldn't).

However, if I set the AsyncLocal from a Middleware, it reaches the controller.

That's because middleware is calls the next middleware (eventually getting to the controller). I.e., middleware is structured like this:

public async Task InvokeAsync(HttpContext context)
{
  using (implicitValue.Set(myValue))
  {
    await _next(context);
  }
}

So the controllers are in the scope of when that AsyncLocal<T> value was set.

How is HttpContext able to retain Items property contents all the way

Items is just a property bag. It doesn't have anything to do with AsyncLocal<T>. It exists because it's a property on HttpContext, and it persists because the same HttpContext instance is used throughout the request.

is ASP.NET runtime disposing the captured ExecutionContext of my DomainContextAccessor for some security reason because of where it is being set at?

Not exactly. The AsyncLocal<T> is being set just fine; it's just that the controllers are not called within the scope of that AsyncLocal<T> being set.

Stephen Cleary
  • 437,863
  • 77
  • 675
  • 810
3

So what must be happening is there is a execution context change which wipes that value out. It works with in the middleware because your controller is in the same execution context as your middleware.

Change your code to this:

private static void DomainContextChangeHandler(AsyncLocalValueChangedArgs<DomainContextHolder> args)
{
    Trace.WriteLine($"ThreadContextChanged: {args.ThreadContextChanged}");
    Trace.WriteLine($"Current: {args.CurrentValue?.GetHashCode()}");
    Trace.WriteLine($"Previous: {args.PreviousValue?.GetHashCode()}");
    Trace.WriteLine($"Thread Id: {Thread.CurrentThread.ManagedThreadId}");
}

Now you can see when the context changes.

Here is something you could do:

private static void DomainContextChangeHandler(AsyncLocalValueChangedArgs<DomainContextHolder> args)
{
    if (args.ThreadContextChanged && (args.PreviousValue != null) && (args.CurrentValue == null))
    {
        Trace.WriteLine(
            "***** Detected context change with a previous value but setting current " +
            "value to null. Resetting value to previous.");
        _domainContextCurrent.Value = args.PreviousValue;
        return;
    }
    Trace.WriteLine($"ThreadContextChanged: {args.ThreadContextChanged}");
    Trace.WriteLine($"Current: {args.CurrentValue?.GetHashCode()}");
    Trace.WriteLine($"Previous: {args.PreviousValue?.GetHashCode()}");
    Trace.WriteLine($"Thread Id: {Thread.CurrentThread.ManagedThreadId}");
}

But, that kinda defeats the purpose of using AsyncLocal as your backing store.

My suggestion is you drop the AsyncLocal and use normal class-scoped storage:

namespace WebApp.Models
{
    public interface IDomainContextAccessor
    {
        DomainContext DomainContext { get; set; }
    }

    public sealed class DomainContextAccessor : IDomainContextAccessor
    {
        public DomainContext DomainContext { get; set; }
    }
}

And inject it as scoped instead of singleton:

services.AddScoped<IDomainContextAccessor, DomainContextAccessor>();

It will do exactly what you want without any kludges -- AND, the future you (or devs) will absolutely understand what's going on and why it is the way it is.

No middleware, no AsyncLocal funny-business. It just works.

Andy
  • 12,859
  • 5
  • 41
  • 56
1

Your answer is here: .net core AsyncLocal loses its value

In your DomainContextAccessor class when you set new value in this line: _domainContextCurrent.Value = new DomainContextHolder { Context = value };

you create NEW ExecutionContext in current thread and child threads.

So I suppose that mvc runs like this: Middleware thread => you set value => some child thread with Controller execution which sees parent changes

But for UserAuthenticationHandler it feels it works like this: Some controller factory creates controller with injected IDomainContextAccessor (1 context) => mvc executes auth handler in child task where you set value and create 2 context. But it's value does not go UP to parent (where controller 1 context exists) because you create new context when you set value. Even more your code gets parents context, gets reference to its value and makes property Context = null, so you will get null in Controller.

So to fix this you need to change your code:

public class DomainContext
    {
        private static AsyncLocal<DomainContext> _contextHolder = new AsyncLocal<DomainContext>();

        public static DomainContext Current
        {
            get
            {
                return _contextHolder.Value;
            }
        }

        public Job JobInfo { get; set; }

        public static void InitContext()
        {
           _contextHolder.Value = new DomainContext();
        }
    }

//using in middleware:
DomainContext.InitContext();

//using in auth handler:
DomainContext.Current.JobInfo = ...

In example above you don't change DomainContext reference in _contextHolder.Value; It remains the same but you only change value of JobInfo in it later in auth handler

miii aaa
  • 11
  • 1