ASP.NET Core Middleware Components are Singletons

Wednesday, April 19, 2017

This is the first post in a series of posts based on code reviews of systems where ASP.NET Core is involved.

I recently came across code like the following:

public class FaultyMiddleware
{
    public FaultyMiddleware(RequestDelegate next)
    {
        _next = next;
    }

    public async Task Invoke(HttpContext context)
    {
        // saving the context so we don't need to pass around as a parameter
        this._context = context;

        DoSomeWork();

        await _next(context);            
    }

    private void DoSomeWork()
    {
        // code that calls other private methods
    }

    // ...

    HttpContext _context;
    RequestDelegate _next;
}

The problem here is a misunderstanding of how middleware components work in the ASP.NET Core pipeline. ASP.NET Core uses a single instance of a middleware component to process multiple requests, so it is best to think of the component as a singleton. Saving state into an instance field is going to create problems when there are concurrent requests working through the pipeline.

If there is so much work to do inside a component that you need multiple private methods, a possible solution is to delegate the work to another class and instantiate the class once per request. Something like the following:

public class RequestProcessor
{
    private readonly HttpContext _context;

    public RequestProcessor(HttpContext context)
    {
        _context = context;
    }

    public void DoSomeWork()
    {
        // ... 
    }
}

Now the middleware component has the single responsibility of following the implicit middleware contract so it fits into the ASP.NET Core processing pipeline. Meanwhile, the RequestProcessor, once given a more suitable name, is a class the system can use anytime there is work to do with an HttpContext.


Comments
gravatar Mikhail Shilkov Wednesday, April 19, 2017
Saving context into the field for private method is horrible regardless. They should just pass context to DoSomeWork as explicit argument.
gravatar paul h Wednesday, April 19, 2017
If RequestProcessor is injected into FaultyMiddleware, you do not have control over the scope. It would be a singleton - the same as the parent. Incidentally, what you probably want to do is use HttpContextAccessor instead of HttpContext directly.
gravatar Scott Wednesday, April 19, 2017
For constructor injection yes, it would be a singleton. However, the Invoke method is also injectable, so you can take transient or scoped services. Also, the RequestDelegate takes a context, not a context accessor.
paul h Wednesday, April 19, 2017
Interesting about the method injection for Invoke. Did not know about that. Regarding the HttpContext constructor argument in RequestProcessor, as far as I can tell, this will not get resolved by the framework. Invoke seems special in that respect. Elsewhere you need to register IHttpContextAccessor and use that instead. Or have I missed something?
gravatar Scott Wednesday, April 19, 2017
Oh, for that piece, yes. Assuming it is registered as a service would be better take an accessor. Sorry for the confusion.
gravatar Deven Thursday, April 20, 2017
Thanks for this article. To add to the above debate, can use a factory (func) injection in the constructor, that will allow us to create a new instance for every request.
tom Thursday, April 20, 2017
Could you please explain why "Saving context into the field..." ? Uncle Bob has different opinion...
gravatar Scott Thursday, April 20, 2017
@Deven: Can also let the container initialize the handler, too. @tom: Because the instance field will be used across multiple requests. Unless we execute requests in serial there will be race conditions.
Comments are closed.

My Pluralsight Courses

K.Scott Allen OdeToCode by K. Scott Allen
What JavaScript Developers Should Know About ECMAScript 2015
The Podcast!