search

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.45k stars 10.03k forks source link

Add bit to DefaultHttpContext to indicate whether request is live #29709

Closed shirhatti closed 3 years ago

shirhatti commented 3 years ago

Since we pool HttpContext, instances of HttpContext no longer in use remain rooted. When scavenging the heap as part of analyzing a memory dump, this makes it extremely time consuming to ascertain whether an HttpContext represents an in-flight request. Adding a bit would greatly simplify this.

davidfowl commented 3 years ago

cc @Tratcher @halter73 @jkotalik

shirhatti commented 3 years ago

I spent some time in clrmd playing around with this. This didn't seem too complex to me and as such my current recommendation is not to add anything.

// typeof(obj) == Microsoft.AspNetCore.Http.DefaultHttpContext
if (obj.ReadValueTypeField("_features").ReadObjectField("<Collection>k__BackingField").IsNull)
{
  // Request is not live
}

cc @n777ty who asked us to file this issue

halter73 commented 3 years ago

This works, but what about people just using plain old SOS? Even in clrmd, it's harder than it needs to be. I think we should add the bit anyway.

n777ty commented 3 years ago

It would definitely help us a lot and speed up analysis of memory dumps (along with writing any analysis rules)... so my vote is for adding the bit.

BrennanConroy commented 3 years ago

Fixed via https://github.com/dotnet/aspnetcore/pull/29748