Mediatr: Mediator breaks out of the logical CallContext

[vegar]

I'm having trouble with my mediator pipeline.
I'm using the IRequestPostProcessor to implement both some workflow behavior and generic logging tasks.

Some of my mediator handlers are requesting the current user from HttpContext.Current.User, and for the most part, this works just fine. But lately we have experienced that the http context is not available.

I'm not 100% sure about this, but I think it's a question of setting continueOnCapturedContext to false when calling next() in the RequestPostProcessorBehavior-implementation. At least, replacing the implementation with my own implementation that does not include the .ConfigureAwait(false) seems to solve the problem.

Is this a known problem?
Is there a known solution?

(We're still on MediatR 3.01)

[vegar]

Another side effect of this, is that LightInjects PerRequestLifetime()-scope breaks. When MediatR asks for RequestPostProcessor-instances, and the implementation depends on something with the per request lifetime, the IoC container throws an exception, saying that there is no scope manager available at this point. Again this is because HttpContext.Current is no longer available.

[vegar]

I would love to make a small repro-project, I'll see if I manage to do that later. But for now, I've made this image illustrating the problem:

This is from a background job. I've merged three pictures from three breakpoints. As you can see, the current thread has changed between the code lines. The IoC container can not keep up with this, meaning any form of 'scoped lifetime' for dependencies are broken.

[vegar]

OK, so I now understand that async/await wan't ensure you get synced back to the same thread, but it will ensure you have same context - e.g. HttpContext.Current. Still - it doesn't seem to work that way...

(http://vegetarianprogrammer.blogspot.no/2012/12/understanding-synchronizationcontext-in.html)

[jbogard]

What version of ASP.NET are you in? And no, this isn't a known problem as far as I've seen.

[vegar]

Version Information: Microsoft .NET Framework Version:4.0.30319; ASP.NET Version:4.7.2633.0

As you might understand, I'm a little confused about the problem.
I'm not sure I'm able to find out when things goes 'south'. But from my current understanding, it's perfectly normal that my code switch thread, and as long as I'm not having .ConfigureAwait(false) on my awaited call, it shouldn't matter what happens inside of that awaited call. So in theory, the code that I changed inside of the RequestPostProcessor shouldn't really change anything. But it did...

😕

[jbogard]

Ok so this is ASP.NET, not ASP.NET Core, right? If that’s the case then you need to explicitly capture context from HttpContext locally to make sure it’s preserved. That’s why Core moved away from this pattern into explicit context variables passed around.

[jbogard]

Right @danielmarbach? :D

[danielmarbach]

@jbogard yes. Since MediateR is a general purpose in-process messaging library I'd say ConfigureAwait(false) could be argued as a good default value. Especially considering that MediatR encourages you to have SRP handlers that can return responses and those responses can then be assigned to something like the ViewModel or the UI. With that design you should never need to access something that is context-aware inside a handler, thus ConfigureAwait(false) actually enforces that design decision.

[vegar]

you need to explicitly capture context from HttpContext locally to make sure it's preserved

Would you consider 'restoring' the context later? Is that even possible?
Or would you need to just pass along the current user to every dependency manually?

MediatR encourage you to have SRP handlers that can return responses and those responses can then be assigned to something like the ViewModel or the UI....

Would you by that consider it a bad choice to let handlers call handlers?
I my code, the controller method will typically send a request for an aggregated entity of some kind. The handler for that request will again use mediator to fetch pieces of data, do the calculations on it and then return the aggregated entity. Is that bad?

Would you consider using pipeline behaviors for 'cross cutting concerns' bad?
In my code, I use pipeline behaviors both for caching and logging on a generic level.
E.g. I can put an attribute on a request type to have the result cached, or if the request is some kind of 'business transaction', an attribute will ensure that it is logged in a special way to provide statistics.

We also uses pipeline behavior so publish events based on requests being handled (e.g. automatically publish a SomethingHappenedNotification after a MakeThingHappenRequest is handled)

It's one of these SomethingHappenedNotification-handlers and business transaction logging-handlers that now fails because they expect to get hold of the current user. The MakeThingHappenRequest-itself doesn't use the current user for anything.

Am I overusing/abusing MediatR?
If so, do you have suggestions for alternative strategies?

[joaomatossilva]

Would you consider using pipeline behaviors for 'cross cutting concerns' bad?
In my code, I use pipeline behaviors both for caching and logging on a generic level.
E.g. I can put an attribute on a request type to have the result cached, or if the request is some kind of 'business transaction', an attribute will ensure that it is logged in a special way to provide statistics.

I don't think cross cutting concerns are bad. but if they depend from context, then yes, it might be a wrong design issue. Can that context (namely HttpContext) be instead injected into those behaviours instead of explicitly depend on the HttpContext.Current ?

[vegar]

The implementation I have, is to use an IPrincipalProvider-service that is injected into the handler by the IoC container. The service is implemented by reading the HttpContext. But guess what: When the service is injected into the handler, HttpContext does no longer exist ;-)

The scoped lifetime-handling of the IoC container breaks down when the context is lost.

[joaomatossilva]

Does your service rely on reading the HttpContext.Current or is the HttpContextBase is injected into the service (this is a common setup on most IoC providers).

[vegar]

I have tried two different approaches:

1. Registering IPrincipalProvider as a transient service (always create a new instance), that will read HttpContext.Current when accessed
2. Registering IPrincipalProvider as a PerRequestLifetime service, that will capture HttpContext.Current in the constructor.

For the first option, at some point, the IPrincipalProvider will have no HttpContext.Current to read.
For the second option, at some point, the IoC container will throw because there is no RequestScope available, so it cannot inject a IPrincipalProvider to the handler.

I'm using LightInject.

[joaomatossilva]

I'm using structuremap now, and I've used autofac as well.
I have the dbcontext configure as per-request, and I have no issues on either the handlers or the behaviours.

I haven't used LightInject, yet, but I doubt that it would be a IoC framework issue.

[vegar]

And I've started to doubt that it is an MediatR issue as well, really.
DBContext sounds like a perfectly good example of context you would like to have available without having to explicitly pass it along through your call chain / pipeline. With MongoDb, transactions out of the question (for now), so we haven't gotten issues there.

[joaomatossilva]

DBContext sounds like a perfectly good example of context you would like to have available without having to explicitly pass it along through your call chain / pipeline

Actually, I do prefer to have it injected where needed rather than have it statictly available everywhere, like in a DbContext.Current way.

[vegar]

I wonder how DbContext.Current would work in my environment. Would that too breakdown? We'll never know...

[danielmarbach]

I'm pretty sure with lightinject you have to use the logica call context flow like this

ScopeManagerProvider = new PerLogicalCallContextScopeManagerProvider()

[vegar]

The way I understand things, that's correct outside of 'the web world'.

The Lightinject.Web-extension gives you a PerRequestLifetime with its own scope manager.

@seesharper - what's your take on this? How will the PerScopeLifetime with the PerLogicalCallContextScopeManagerProvider() work as an replacement for the PerRequestLifetime ?

[seesharper]

@vegar @jbogard @danielmarbach @joaomatossilva @fredimachado

There are just two lifetimes that requires a scope in LightInject.

• PerRequestLifetime -> new instance for every time it is requested from the container. Tracks disposables that are disposed when the scope ends.

• PerScopeLifetime -> a single instance per scope. Tracks disposables that are disposed when the scope ends.

These lifetimes are used in all types of applications. Web, Console and Desktop apps. You name it.

What changes in between application types is how the scopes are managed. By "managed" I mean how the scopes are stored.

• PerThreadScopeManagerProvider -> Scopes are stored per thread (This is the default in LightInject)
• PerPerLogicalCallContextScopeManagerProvider -> Scopes are stored per logical call context. (AsyncLocal<T>)

So what scope manager should be used for a given application type? Well, it depends :)

For "legacy" web apps that uses the old System.Web stack, we can't really use any of the built-in scope managers. Per thread is not going to work since web requests can start and finish on different threads. Per call context does not really work either since ASP.NET does not flow async locals throughout the web request. The only "safe" place to store ambient informations such as the current scope is in HttpContext.Current.Items collection. It is safe until the point where you do a ConfigureAwait(false). That could cause the HttpContext.Current to return null and the ambient information is lost.

Take a look here for more information
https://github.com/seesharper/LightInject/issues/386

LightInject.Web provides the PerWebRequestScopeManager that stores the scope in HttpContext.Current.Items.

For ASP.NET Core applications it is safe to use PerPerLogicalCallContextScopeManagerProvider since the entire web request from start to finish in within the same logical call context.

My advice is to try to moving the app ASP.NET Core where these are solved properly. You can still target the full framework. Or use LightInject.Web and be careful with ´ConfigureAwait(false)´.

[vegar]

I have no idea how many times I've read the docs, but I somehow missed this PerRequestLifetime -> new instance for every time it is requested.... I've always assumed Request==WebRequest I guess.

So what I take from your answer is:

• If I want an service instance to be shared within a single web request, I should use PerScopeLifetime, not PerRequestLifetime.
• As long as we are in the world of System.Web, there is no guarantee that this will work anyway.

[seesharper]

As long as we are in the world of System.Web, there is no guarantee that this will work anyway.

Stay away from ConfigureAwait(false) and you should be good.

By staying away I mean code that accesses the ambient context (eg HttpContent.Current )should not be asynchronously executed using ConfigureAwait(false)

.Net Framework 4.7.1 introduces the concept of Execution steps that seems to address this issue although I have never tested this myself. https://blogs.msdn.microsoft.com/dotnet/2017/09/13/net-framework-4-7-1-asp-net-and-configuration-features/

[seesharper]

If I want an service instance to be shared within a single web request, I should use PerScopeLifetime, not PerRequestLifetime.

Yup, that's correct

[vegar]

Stay away from ConfigureAwait(false) and you should be good.

And that's where this thread started: As long as MediatR uses ConfigureAwait(false), and MediatR is the one calling my code, There isn't much I can do. Except write my own MediatR pipeline :-)

Even though this thread has been very educating and interesting for me, I can't really see that I can blame MediatR for my problem. So I guess we should just close it? @jbogard ?

[jbogard]

Yep!

On Fri, May 25, 2018 at 12:18 PM Vegar Vikan [e-mail]
wrote:

Stay away from ConfigureAwait(false) and you should be good.

And that's where this thread started: As long as MediatR uses
ConfigureAwait(false), and MediatR is the one calling my code, There
isn't much I can do. Except write my own MediatR pipeline :-)

Even though this thread has been very educating and interesting for me, I
can't really see that I can blame MediatR for my problem. So I guess we
should just close it? @jbogard https://github.com/jbogard ?

—
You are receiving this because you were mentioned.

Reply to this email directly, view it on GitHub
https://github.com/jbogard/MediatR/issues/261#issuecomment-392022069,
or mute the thread
https://github.com/notifications/unsubscribe-auth/AAGYMt1q6Xm_VX7HJtoDf2TVwnOpwfGHks5t1-hqgaJpZM4T2yl3
.

[seesharper]

@jbogard

So if accessing for instance HttpContext.Current inside a Pre/Post processor, that's going to blow up?

https://github.com/jbogard/MediatR/blob/a8338117ccce8817d4a2bf0a64bf78bbc93a581a/src/MediatR/Pipeline/RequestPreProcessorBehavior.cs#L24

[jbogard]

Yep! You’ll need to capture those values explicitly.

On Fri, May 25, 2018 at 12:29 PM Bernhard Richter [e-mail]
wrote:

@jbogard https://github.com/jbogard

So if accessing for instance HttpContext.Current inside a Pre/Post
processor, that's going to blow up?

https://github.com/jbogard/MediatR/blob/a8338117ccce8817d4a2bf0a64bf78bbc93a581a/src/MediatR/Pipeline/RequestPreProcessorBehavior.cs#L24

—
You are receiving this because you were mentioned.

Reply to this email directly, view it on GitHub
https://github.com/jbogard/MediatR/issues/261#issuecomment-392024299,
or mute the thread
https://github.com/notifications/unsubscribe-auth/AAGYMjNkW5N_Nr924wvbDK8wu8olbbeGks5t1-sXgaJpZM4T2yl3
.

[seesharper]

Excuse my ignorance here, but would it not be better to let the outermost caller decide if the context should be captured?

var response = await mediator.Send(new Ping()).ConfigureAwait(true);

When executing this I would expect all code to run within the sync context.
The fact that it breaks out of the sync context during pre/post processing seems a little bit odd to me.

If we don't care about sync context

var response = await mediator.Send(new Ping()).ConfigureAwait(false);

[joaomatossilva]

@seesharper This goes beyond MediatR. This is how the async await works.

What could be as any solution, would be to have a static configuration, where you do specify the type of await you want.

for example a

MediatorConfiguration.ContinueOnCapturedContext = true;

then you could call

await mediator.Send(new Ping())

without calling any ConfigureAwait and still have the context.
This would change all the places on MediatR where the ConfigureAwait is called to be

.ConfigureAwait(MediatorConfiguration.ContinueOnCapturedContext)

And it shoudl default to false

[seesharper]

This goes beyond MediatR.

Actually it is sort of an MediatR issue since processors are ripped out of context. IMHO it should be up to the processor to decide if the code within the processor is to be executed within the captured context. Since the RequestPostProcessorBehavior and the RequestPreProcessorBehavior executes the processors using ConfigureAwait(false), that decision has already been made by MediatR and there is nothing the processors can do about it .

There should at least be a way to work around this somehow

Just my two cents :)

[joaomatossilva]

I'm saying it's beyond MediatR, because its just common and best practice to have all calls with ConfigureAwait(false).

Only on cases you do want to have the context after you go async, then it makes more sense to configure the libraries you use to either resume or not on with same context. After all we're just discussing performance vs context persitense.

That's why I proposed to have a global setting to either you choose to have context or to run on best possible performance.

Anyway, this sort of design is being pushed away over time. This means I don't know if making a change to support something that will go legacy in a few years will make sense.

[vegar]

@seesharper Does that mean that using PerScopeLifetime will fail as well? The PerPerLogicalCallContextScopeManagerProvider want be able to manage a common scope as long as the .ConfigureAwait(false)-call comes in the middle?

[seesharper]

Nope, AsyncLocal (logical call context ) does not flow throughout the web request when on IIS (System.Web)
HttpContext.Current is your only friend here :)

[seesharper]

You can use PerScopeLifetime, but you need the PerWebRequestScopeManager from LightInject.Web to manage the scopes.

Long story short. If you have the option to move to ASP.NET Core, do it now :)

[vegar]

I'm not talking about HttpContext.Current-stuff. I'm talking about any type of (non-core) project utilizing PerScopeLifetime to share a single instance of a resource within a defined scope. Will that fail as soon as any 3rd party code in the middle throws away the context?

[seesharper]

Anyway, this sort of design is being pushed away over time. This means I don't know if making a change to support something that will go legacy in a few years will make sense.

@joaomatossilva

Yeah, I can agree to that. This is only a real issue on legacy ASP.NET apps

[seesharper]

I'm talking about any type of (non-core) project utilizing PerScopeLifetime to share a single instance of a resource within a defined scope. Will that fail as soon as any 3rd party code in the middle throws away the context?

You can use PerScopeLifetime in all kind of apps. IIS apps represents a special case because of the begin/end request pipeline. Hence the need for LightInject.Web

Will that fail as soon as any 3rd party code in the middle throws away the context?

If you captured the context (ConfigureAwait(true)), you should be good. Problem in this case is that MediatR throws it away before calling into your code

[danielmarbach]

Throw the http context on the request poco and you are done

[vegar]

yea, except that that doesn't work very well with anything but the simplest pipeline, since it requires all handlers of both requests and notifications to always pass it along just in case someone somewhere might need it.... We touch that topic further up in the thread.

But in the current project, that might turn out a lot easier than pressing on to .net core.....