[API Proposal]: Adding DisposeAsync to HttpResponseMessage and HttpContent

[liveans]

Background and motivation

The motivation for proposing the addition of DisposeAsync to HttpResponseMessage and HttpContent stems from the need to address performance issues related to the disposal of HTTP connections, particularly in the context of HTTP/3. The current implementation of synchronous disposal via Dispose can lead to sync-over-async scenarios deep within the QuicStream, which is not ideal.

The problem is exemplified by the issue reported on GitHub (#77139), where the shutdown of HTTP/3 connections or streams was observed to be slow, significantly affecting performance metrics such as requests per second (RPS). This issue highlights the impact of the current disposal pattern on performance, especially in high-throughput scenarios.

By introducing DisposeAsync, we aim to provide a more efficient way to dispose of these objects, particularly when dealing with asynchronous streams. This change would allow for asynchronous cleanup operations, which are more aligned with the nature of asynchronous programming and can help avoid the aforementioned performance pitfalls.

The addition of DisposeAsync would enable a more performant and reliable disposal mechanism, ensuring that resources are freed in a timely and non-blocking manner.

In conclusion, the proposal to add DisposeAsync is driven by a clear need to improve the performance and reliability of HTTP connection disposal, as evidenced by real-world issues and internal analysis. This change would bring the disposal pattern in line with modern asynchronous programming practices, benefiting developers and end-users alike with more efficient resource management and better application performance.

/cc @ManickaP

API Proposal

namespace System.Net.Http;

-public class HttpResponseMessage : IDisposable
+public class HttpResponseMessage : IDisposable, IAsyncDisposable
{
    public void Dispose();
+   public ValueTask DisposeAsync();
+   protected virtual ValueTask DisposeAsyncCore();
}

-public abstract class HttpContent: IDisposable
+public abstract class HttpContent: IDisposable, IAsyncDisposable
{
    public void Dispose();
+   public ValueTask DisposeAsync();
+   protected virtual ValueTask DisposeAsyncCore();
}

API Usage

HttpClient client = new(...);
await using HttpResponseMessage message = await client.GetAsync(...);
await using Stream responseStream = await message.Content.ReadAsStreamAsync();

or

HttpClient client = new(...);
HttpResponseMessage message = await client.GetAsync(...);
// Do something
_ = message.DisposeAsync();

Alternative Designs

TODO

Risks

TODO

[dotnet-policy-service]

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

[stephentoub]

What is it that a response QuicStream needs to do as part of disposal that is asynchronous?

[liveans]

What is it that a response QuicStream needs to do as part of disposal that is asynchronous?

QuicStream.DisposeAsync() waits for SHUTDOWN_COMPLETE event

[stephentoub]

What is it that a response QuicStream needs to do as part of disposal that is asynchronous?

QuicStream.DisposeAsync() waits for SHUTDOWN_COMPLETE event

The resources it's referring to, that's the freeing of the handle? Can the freeing of that handle be done fire-and-forget from the perspective of the stream such that neither Dispose / DisposeAsync needs to wait for the ack? Connection cleanup could be responsible for ensuring any stream handles that are still being cleaned up are freed prior to the connection handle being cleaned up.

[ManickaP]

@stephentoub It's not just about freeing the handle. Writing data to the stream is just copying them to MsQuic internal buffer (same as with sockets). The problem is that MsQuic is user space, so after app closure, everything that hasn't been flushed to the network is lost (e.g. microsoft/msquic#3842). On top of that, many applications do not dispose HttpClient (neither the gRPC benchmarks used in the linked issue, neither would the proposal of HttpClient.Shared, ...). So no connection disposal to wait for the pending stream disposals. In extremis, this could lead to lost data.

Technical details: to avoid any lost data (we still may lose sending ACKs) we need to wait for SEND_SHUTDOWN_COMPLETE. We could do that with "the last write" (WriteAsync with completeWrites == true and make QuicStream.CompleteWrites async). This still leaves out the scenario where QuicStream is used like an ordinary stream and completing the write-side is part of QuicStream.Dispose...