dotnet / aspnetcore

ASP.NET Core is a cross-platform .NET framework for building modern cloud-based web applications on Windows, Mac, or Linux.
https://asp.net
MIT License
35.35k stars 9.99k forks source link

IFeatureCollection has been disposed when trying to access HttpContext.Request.Headers #20551

Closed Jasdo closed 4 years ago

Jasdo commented 4 years ago

Steps to reprodce

  1. Create a simple API ASP.NET Core 3.1 Web Application using Visual Studio 2019 template.
  2. In the handler method, add access to HttpContext.Request.Headers.
  3. Create a REST client that sends many requests simultaneously.

The result is that eventually, a System.ObjectDisposedException will be thrown upon accessing HttpContext.Request.Headers.

Startup code

public class Startup
{
    public Startup(IConfiguration configuration)
    {
        Configuration = configuration;
    }

    public IConfiguration Configuration { get; }

    // This method gets called by the runtime. Use this method to add services to the container.
    public void ConfigureServices(IServiceCollection services)
    {
        services.AddSingleton<WeatherForecastController>();
        services.AddMvc().AddControllersAsServices();
    }

    // This method gets called by the runtime. Use this method to configure the HTTP request pipeline.
    public void Configure(IApplicationBuilder app, IWebHostEnvironment env)
    {
        if (env.IsDevelopment())
        {
            app.UseDeveloperExceptionPage();
        }

        app.UseHttpsRedirection();
        app.UseRouting();
        app.UseAuthorization();
        app.UseEndpoints(endpoints =>
        {
            endpoints.MapControllers();
        });
    }
}

Controller code

[ApiController]
[Route("[controller]")]
public class WeatherForecastController : ControllerBase
{
    private static readonly string[] Summaries = new[]
    {
        "Freezing", "Bracing", "Chilly", "Cool", "Mild", "Warm", "Balmy", "Hot", "Sweltering", "Scorching"
    };

    private readonly ILogger<WeatherForecastController> _logger;
    public WeatherForecastController(ILogger<WeatherForecastController> logger)
    {
        _logger = logger;
    }

    [HttpGet]
    public IEnumerable<WeatherForecast> Get()
    {
        var headers = HttpContext.Request.Headers;
        var rng = new Random();
        return Enumerable.Range(1, 5).Select(index => new WeatherForecast
        {
            Date = DateTime.Now.AddDays(index),
            TemperatureC = rng.Next(-20, 55),
            Summary = Summaries[rng.Next(Summaries.Length)]
        })
        .ToArray();
    }
}

Client code

static void Main(string[] args)
{
    Parallel.For(0, 3000, i =>
    {
        var getWeatherUri = new Uri("https://localhost:44341/weatherforecast");

        MediaTypeWithQualityHeaderValue requestHeader = new MediaTypeWithQualityHeaderValue("application/json");
        var client = new HttpClient();
        client.DefaultRequestHeaders.Accept.Clear();
        client.DefaultRequestHeaders.Accept.Add(requestHeader);
        var response = client.GetAsync(getWeatherUri.AbsoluteUri).Result;
        if (!response.IsSuccessStatusCode)
        {
            throw new Exception($"{response.StatusCode}: {response.RequestMessage} | {response.ReasonPhrase}");
        }
    });
}

Further technical details

Tratcher commented 4 years ago

Do you have the full stacktrace for the exception?

Is this on IIS or Kestrel? Seems like IIS.

If you observe and categorize the failures as shown by HttpClient, what's the breakdown that timeout, receive 500s, etc.? This may be related to the client timing out and disconnecting. Are there any more interesting failures reported by the client?

Also, how does it behave if you switch to HTTP/2?

analogrelay commented 4 years ago
        services.AddSingleton<WeatherForecastController>();

Does it work if you remove this? I'm not sure Controllers are supposed to be singletons. (cc @rynowak)

Jasdo commented 4 years ago
        services.AddSingleton<WeatherForecastController>();

Does it work if you remove this? I'm not sure Controllers are supposed to be singletons. (cc @rynowak)

Yes, it works. The problem is that it creates a new instance of the controller for each REST call, and I'm trying to avoid that. Of course I can move the part that I want to be a "singleton" to another class, but everything worked fine before we migrated to 3.1 from 2.1. I assume it is related to this change in FeatureReferences.

analogrelay commented 4 years ago

The problem is that it creates a new instance of the controller for each REST call, and I'm trying to avoid that.

Why are you trying to avoid that? If you need a singleton, it should be a service consumed by the controller, not the controller itself.

Tratcher commented 4 years ago

I checked with MVC and controllers are not supported as singletons. They have request state, so even if you didn't hit issues like this, you'd still have concurrency problems.

Jasdo commented 4 years ago

The problem is that it creates a new instance of the controller for each REST call, and I'm trying to avoid that.

Why are you trying to avoid that? If you need a singleton, it should be a service consumed by the controller, not the controller itself.

Yes, this is how it is implemented now in our code, the thing is that using the controller as a singleton worked until we migrated from ASP .NET Core 2.1 to 3.1.

Jasdo commented 4 years ago

@Tratcher, @anurse Thank you for your help!