When or if to Dispose HttpResponseMessage when calling ReadAsStreamAsync?
Solution 1:
So it seems like the calling code needs to know about and take ownership of the response message as well as the stream, or I leave the response message undisposed and let the finalizer deal with it. Neither option feels right.
In this specific case, there are no finalizers. Neither HttpResponseMessage
or HttpRequestMessage
implement a finalizer (and that's a good thing!). If you don't dispose of either of them, they will get garbage collected once the GC kicks in, and the handle to their underlying streams will be collected once that happens.
As long as you're using these objects, don't dispose. Once done, dispose of them. Instead of wrapping them in a using
statement, you can always explicitly call Dispose
once you're done. Either way the consuming code doesn't need to have any knowledge underlying http requests.
Solution 2:
You can also take stream as input parameter, so the caller has complete control over type of the stream as well as its disposal. And now you can also dispose httpResponse before control leaves the method.
Below is the extension method for HttpClient
public static async Task HttpDownloadStreamAsync(this HttpClient httpClient, string url, Stream output)
{
using (var httpResponse = await httpClient.GetAsync(url).ConfigureAwait(false))
{
// Ensures OK status
response.EnsureSuccessStatusCode();
// Get response stream
var result = await httpResponse.Content.ReadAsStreamAsync().ConfigureAwait(false);
await result.CopyToAsync(output).ConfigureAwait(false);
output.Seek(0L, SeekOrigin.Begin);
}
}
Solution 3:
Dealing with Disposes in .NET is both easy, and hard. For sure.
Streams pull this same nonsense... Does disposing the Buffer also then automatically dispose of the Stream it wrapped? Should it? As a consumer, should I even know whether it does?
When I deal with this stuff, I go by some rules:
- That if I think there are non-native resources at play (like, a network connection!), I don't ever let the GC "get around to it". Resource exhaustion is real, and good code deals with it.
- If a Disposable takes a Disposable as a parameter, there is never harm in covering my butt and making sure that my code disposes of every object it makes. If my code didn't make it, I can ignore it.
- GCs call ~Finalize, but nothing ever guarantees that Finalize (i.e. your custom destructor) invokes Dispose. There is no magic, contrary to opinions above, so you have to be responsible for it.
So, you've got an HttpClient, an HttpRequestMessage, and an HttpResponseMessage. The lifetimes of each of them, and any Disposable they make, must be respected. Therefore, your Stream should never be expected to survive outside of the Dispoable lifetime of HttpResponseMessage, becuase you didn't instantiate the Stream.
In your above scenario, my pattern would be to pretend that getting that Stream was really just in a Static.DoGet(uri) method, and the Stream you return would HAVE to be one of our own making. That means a second Stream, with the HttpResponseMessage's stream .CopyTo'd my new Stream (routing through a FileStream or a MemoryStream or whatever best fits your situation)... or something similar. Because:
- You have no right to the lifetime of HttpResponseMessage's Stream. That's his, not yours. :)
- Holding up the lifetime of a disposable like HttpClient, while you crunch the contents of that returned stream, is a crazy blocker. That'd be like holding onto a SqlConnection while you parse a DataTable (imagine how quickly we'd starve a connection pool if DataTables got huge)
- Exposing the how of getting that response may work against SOLID... You had a Stream, which is disposable, but it came from an HttpResponseMessage, which is disposable, but that only happened because we used HttpClient and HttpRequestMessage, which are disposable... and all you wanted was a stream from a URI. How confusing do those responsibilities feel?
- Networks are still the slowest lanes in computer systems. Holding them up for "optimization" is still insane. There are always better ways to handle the slowest components.
So use disposables like catch-and-release... make them, snag the results for yourself, release them as quickly as possible. And don't confuse optimization for correctness, especially from classes that you did not yourself author.