Should HttpClient instances created by HttpClientFactory be disposed?

Question

So, I've registered a named client with the services collection in my Startup.cs:

services.AddHttpClient(someServiceName, 
                       client => client.BaseAddress = baseAddress);

and now can inject an IHttpClientFactory from my service provider.

Using this IHttpClientFactory, I conjure up a client instance:

var client = httpClientFactory.CreateClient(someServiceName)

Once upon a time, it was necessary to be very careful about the disposing of HttpClient instances, as it was rarely the right thing to do.

However, now we have HttpClientFactory, does this matter any more? Should/Can this client be disposed without worry? e.g.

using (var httpClient = httpClientFactory.CreateClient(someServiceName))
using (var response = await httpClient.PostAsync(somePath, someData))
{
    var content = await response.Content.ReadAsAsync<SomeResponse>();
    //...
}

Check this article which explains how client is handled by factory https://www.stevejgordon.co.uk/introduction-to-httpclientfactory-aspnetcore — Nkosi, Jun 18 '18 at 14:54
@Nkosi Yes, I've read that. However, I've located some code that looks much like the last snippet in my question, and I'm wondering if I need to fix it. — spender, Jun 18 '18 at 14:56

Mark Shevchenko · Accepted Answer · 2023-07-18T14:09:56.840

Calling the Dispose method is not required but you can still call it if you need for some reasons.

Proof: HttpClient and lifetime management

Disposal of the client isn't required. Disposal cancels outgoing requests and guarantees the given HttpClient instance can't be used after calling Dispose. IHttpClientFactory tracks and disposes resources used by HttpClient instances. The HttpClient instances can generally be treated as .NET objects not requiring disposal.

Let's check the source of DefaultHttpClientFactory:

public HttpClient CreateClient(string name)
{
    if (name == null)
    {
        throw new ArgumentNullException(nameof(name));
    }

    var handler = CreateHandler(name);
    var client = new HttpClient(handler, disposeHandler: false);

    var options = _optionsMonitor.Get(name);
    for (var i = 0; i < options.HttpClientActions.Count; i++)
    {
        options.HttpClientActions[i](client);
    }

    return client;
}

The instance of HttpMessageHandler stores unmanaged resources of the HttpClient. In the classical scenario the HttpClient creates the instance of HttpMessageHandler and disposes it while itself disposing.

You can see in the above code that different instances of HttpClient shares single instance of HttpMessageHandler and doesn't dispose it (disposeHandler: false).

So, the call of the HttpClient.Dispose does nothing. But it's not dangerous.

score 41 · Answer 2 · answered Jun 18 '18 at 14:56

41

No. You should not dispose of your client. To be more general, you should not dispose of anything retrieved via a DI container, which in ASP.NET Core is by default the service collection. The lifetime is managed by the DI container, so if you dispose of the client, but it's later injected into something, you'll get an ObjectDisposedException. Let the container handle disposal.

This is actually a common confusion with IDisposable classes. You should personally only implement IDisposable if your class itself owns dependencies. If all its dependencies are injected, you should not implement IDisposable, since it doesn't own anything that needs disposal. Likewise, you should not dispose of anything injected into your class, as it doesn't own those dependencies. Only dispose of things you specifically new up. If you don't see the keyword new, you probably shouldn't be disposing.

answered Jun 18 '18 at 14:56

Chris Pratt

232,153
36
385
444

67

Well this is a little confusing, because the client *isn't* retrieved via a DI container. The *factory* is retrieved via a DI container. – Kieren Johnstone Apr 15 '19 at 08:28
And the factory owns the clients and disposes of them. When the factory is disposed, it will dispose of any clients it owns. The same rule still applies: if you don't explicitly new it up, don't dispose it. – Chris Pratt Apr 15 '19 at 11:44
14

but the naming in httpClientFactory.CreateClient(someServiceName) does seem to imply that you new it up, so that is the confusing part. I'm with @KierenJohnstone – Wiebe Tijsma Jul 16 '19 at 14:50
6

It's doesn't matter that a resource is newed up, only whether or not you control its lifetime. In the case of the factory, the factory does the instantiation and it controls the lifetime, not you. That's the point. Only dispose of resources that belong to you. – Chris Pratt Jul 16 '19 at 17:45
3

@ChrisPratt Would you agree this would be less of a problem if C# or .NET included some kind of semantics for object lifetime and ownership to prevent this kind of confusion? Even something simple like an interface (e.g. `IDisposableHahahNotReally`) or referring to objects by a smart-pointer-type object, e.g. `OwnedObject.Instance` and `BorrowedObject.Value`). – Dai Aug 23 '19 at 06:32
Not really. It's very straight forward actually. Unless you use `new` you don't dispose. Calling a factory's create method is not the same. There, the factory is newing stuff up, so it is responsible for disposing, not you. And, of course, if you're injecting, you obviously don't dispose. – Chris Pratt Aug 23 '19 at 08:08
1

@ChrisPratt I don't think it matters. I remember reading in a GitHub issue on the ASP.NET repo that it is the HttpClientHandler that must not be disposed. When you dispose of a HttpClient (created by a factory as above) the handler "tells" the clients not to dispose of it, when it gets disposed of. So, code like this is not problematic at all `using (var client = _httpClientFactory.CreateClient())`. (I think) :) – onefootswill Sep 25 '19 at 05:53
5

Perhaps not. I know that HttpMessageHandler is what the factory actually holds on to and the HttpClient instances are transient scoped. However, the simple fact remains that you shouldn't dispose of that which you don't create or own. If the implementation were to change, you might suddenly break something. – Chris Pratt Sep 25 '19 at 07:03
@ChrisPratt, this is such a leaky abstraction. HttpClient dispose cancels pending requests. So we should know that and cancel pending requests manually if needed because we're now responsible for knowing what Dispose does when we created HttpClients outside of the factory class? – MikeJ Jul 17 '23 at 16:00
@Dai Dispose pattern isn't about lifetime management. It's about resource management. This thread conflates the two. By it's definition Dispose should be idempotent. Calling it multiple times should never break anything. – MikeJ Jul 17 '23 at 16:08

Should HttpClient instances created by HttpClientFactory be disposed?

2 Answers2

Linked