ASP.NET Core Middleware Components are Singletons

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.