dotnet / runtime

.NET is a cross-platform runtime for cloud, mobile, desktop, and IoT apps.
https://docs.microsoft.com/dotnet/core/
MIT License
14.91k stars 4.63k forks source link

[HTTP/3] Implement connection scavenging #54968

Closed geoffkizer closed 2 months ago

geoffkizer commented 3 years ago

We are not scavenging HTTP3 connections currently (i.e. based on idle timeout or lifetime). We need to implement this.

ghost commented 3 years ago

Tagging subscribers to this area: @dotnet/ncl See info in area-owners.md if you want to be subscribed.

Issue Details
We are not scavenging HTTP3 connections currently (i.e. based on idle timeout or lifetime). We need to implement this.
Author: geoffkizer
Assignees: -
Labels: `area-System.Net.Http`
Milestone: 6.0.0
karelz commented 3 years ago

Triage: Would manifest as memory leak

ManickaP commented 3 years ago

@geoffkizer What exactly is missing? I see we check the expiration and dispose and clean up the connection: https://github.com/dotnet/runtime/blob/f59a132bfe9ff797f9d192d67f07673a13b6b29e/src/libraries/System.Net.Http/src/System/Net/Http/SocketsHttpHandler/HttpConnectionPool.cs#L812-L818

geoffkizer commented 3 years ago

There's no HTTP3 logic here: https://github.com/dotnet/runtime/blob/f59a132bfe9ff797f9d192d67f07673a13b6b29e/src/libraries/System.Net.Http/src/System/Net/Http/SocketsHttpHandler/HttpConnectionPool.cs#L1971

geoffkizer commented 3 years ago

BTW, I think we should update the HTTP3 connection pooling logic in general to match the recent changes I made to HTTP11/2. As part of that, this should just fall out.

geoffkizer commented 2 years ago

I changed the milestone to 7.0. We can't deliver production quality HTTP3 without this.

geoffkizer commented 2 years ago

See also https://github.com/dotnet/runtime/issues/60729, which has some proposals for how to change how connection scavenging works.

We should decide if we want to make any of those changes before we implement scavenging for HTTP3, to avoid wasted work.