[API Proposal]: HttpClient.Shared

[stephentoub]

Background and motivation

Many uses of HttpClient require configuring either properties on HttpClient itself or on a customized underlying handler passed to HttpClient (e.g. new HttpClient(new SocketsHttpHandler() { ... })). But many uses are also really simple, with tests, utilities, and even simple product source wanting to make simple HTTP requests. Today each such use needs to either create and destroy a new HttpClient instance, or it needs to manage its own cached instance, or it needs to use something like HttpClientFactory, which both involves DI and is rarely a simple scenario. For these simple scenarios, where no settings need to be configured, it'd be nice to have a one-liner ability to make HTTP requests and to use some of the simple helpers exposed on HttpClient.

Related: #65261

API Proposal

namespace System.Net.Http
{
    public class HttpClient
    {
+        public static HttpClient Shared { get; }
    }
}

• Dispose on the instance returned from Shared would be a nop.
• HttpClient already tracks whether any request has been performed on the instance and immediately stops allowing configuration of its properties once one has been. Shared would ensure that bit is set from the get-go so that no one could mutate the instance.
• We could choose to either use the defaults that HttpClient instances get in general, or we could choose different ones based on knowing the instance will be around for a long time and used by potentially many unrelated consumers, e.g. we could decrease connection idle timeout to more aggressively retire unused connections.

API Usage

string text = await HttpClient.Shared.GetStringAsync("https://dot.net/somefile.txt");

Alternative Designs

No response

Risks

• This is yet another way to get an HttpClient instance.
• Configuration isn't possible, so there's a cliff if anyone starts using this and then needs to tweak something (in that they'd then need to switch to writing the code they'd have had to write previously before this member existed).
• Potential for more resource consumption if folks that were creating/destroying their own instance around a single request switched to the shared instance and ended up with an unused connection sticking around for longer than before.

[ghost]

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

Issue Details

---

Background and motivation

Many uses of HttpClient require configuring either properties on HttpClient itself or on a customized underlying handler passed to HttpClient (e.g. new HttpClient(new SocketsHttpHandler() { ... })). But many uses are also really simple, with tests, utilities, and even simple product source wanting to make simple HTTP requests. Today each such use needs to either create and destroy a new HttpClient instance, or it needs to manage its own cached instance, or it needs to use something like HttpClientFactory, which both involves DI and is rarely a simple scenario. For these simple scenarios, where no settings need to be configured, it'd be nice to have a one-liner ability to make HTTP requests and to use some of the simple helpers exposed on HttpClient.

Related: #65261

API Proposal

namespace System.Net.Http
{
    public class HttpClient
    {
+        public static HttpClient Shared { get; }
    }
}

• Dispose on the instance returned from Shared would be a nop.
• HttpClient already tracks whether any request has been performed on the instance and immediately stops allowing configuration of its properties once one has been. Shared would ensure that bit is set from the get-go so that no one could mutate the instance.
• We could choose to either use the defaults that HttpClient instances get in general, or we could choose different ones based on knowing the instance will be around for a long time and used by potentially many unrelated consumers, e.g. we could decrease connection idle timeout to more aggressively retire unused connections.

API Usage

string text = await HttpClient.Shared.GetStringAsync("https://dot.net/somefile.txt");

Alternative Designs

No response

Risks

• This is yet another way to get an HttpClient instance.
• Configuration isn't possible, so there's a cliff if anyone starts using this and then needs to tweak something (in that they'd then need to switch to writing the code they'd have had to write previously before this member existed).
• Potential for more resource consumption if folks that were creating/destroying their own instance around a single request switched to the shared instance and ended up with an unused connection sticking around for longer than before.

Author:	stephentoub
Assignees:	-
Labels:	api-suggestion, area-System.Net.Http
Milestone:	-

[geoffkizer]

We could choose to either use the defaults that HttpClient instances get in general, or we could choose different ones based on knowing the instance will be around for a long time and used by potentially many unrelated consumers, e.g. we could decrease connection idle timeout to more aggressively retire unused connections.

It seems weird to me to have different defaults for the shared instance than a brand new instance. I'd expect that a new instance would in general have the same behavior as the shared instance, only limited to exactly the requests I use it for.

[CarnaViire]

It seems weird to me to have different defaults for the shared instance than a brand new instance.

Then it would still have the same problem people are solving with HttpClientFactory (DNS changes being ignored). And you will not be able to configure SocketsHttpHandler to set PooledConnectionLifetime.

[campersau]

Configuration isn't possible, ...

Currently the HttpClient.DefaultProxy gets used when the first call to Send it made, so this is one thing which could be configurable right? Or wouldn't this use a proxy at all?

Cookies would then also be shared right? Isn't that a potential security risk?

[karelz]

Triage: Looks like interesting idea, it may simplify lots of things - the easy usage. As mentioned above, we would need to decide on reasonable defaults (PooledConnectionLifetime, MaxConnectionPerServer, timeouts, etc.)
Protecting any changes prior to first usage is overkill.

We should sit on it for a while to think about it more.

We need to be careful about interaction / overlap with HttpClientFactory. @davidfowl @Tratcher any thoughts?
cc @scalablecory

Cookies might be problematic - technically a memory leak in waiting.

[Tratcher]

Currently the HttpClient.DefaultProxy gets used when the first call to Send it made, so this is one thing which could be configurable right? Or wouldn't this use a proxy at all?

DefaultProxy is static, nothing would change there as part of this proposal for a shared instance.

Cookies would then also be shared right? Isn't that a potential security risk?

Cookies should already be segregated by domain so there shouldn't be much additional risk.

Cookies might be problematic - technically a memory leak in waiting.

Might have to revisit cookie retention policies. E.g. Capacity limits and LRU disposal.

This proposal seems orthogonal to HttpClientFactory.

We may also want to consider adding HttpClientHandler.Shared that would come similarly pre-configured, but still allow you to add DelegatingHandlers, DefaultRequestHeaders, etc.. The handler has always been the real resource/lifetime issue, not the client.

[scalablecory]

This Shared property doesn't seem ideal for usage everywhere as in your sample. I would prefer simpler static methods like Http.GetAsync for that. We might potentially optimize that better, and wouldn't have to worry about e.g. DefaultRequestHeaders.

In addition to this API, consider: HttpClient.SharedHandler that gives you the underlying handler. This will allow use with libraries that take the abstract handler.

Note we rejected this idea once before in a team meeting. I think it was due to pooled auth, but iirc we don't have that behavior anymore.

[AraHaan]

I would like to combine this with the option of optionally changing the settings on the member of the cached instance here.

If this can be done then I can nuke places in my code where I also do my own caching of HttpClient instances that live for the entire process lifetime (it would help me with remembering not to dispose of it).

[wfurt]

In addition to this API, consider: HttpClient.SharedHandler that gives you the underlying handler. This will allow use with libraries that take the abstract handler.

I assume that could be platform handler as well on mobile devices, right? To me the biggest challenges are around connection management as that impact OS resources and network behavior.

[scalablecory]

In addition to this API, consider: HttpClient.SharedHandler that gives you the underlying handler. This will allow use with libraries that take the abstract handler.

I assume that could be platform handler as well on mobile devices, right? To me the biggest challenges are around connection management as that impact OS resources and network behavior.

Yes it would need to be a platform handler.

[scalablecory]

This Shared property doesn't seem ideal for usage everywhere as in your sample. I would prefer simpler static methods like Http.GetAsync for that. We might potentially optimize that better, and wouldn't have to worry about e.g. DefaultRequestHeaders.

A using alias that worked for static properties would be a nice way to have similar shortness but keep all the extension methods still naturally working. Hmm.

[MihaZupan]

Triage: Marking as ready to review since a decision about the exact defaults we would want such an instance to use is unlikely to affect the API shape.

[terrajobst]

Video

• We should ensure that all properties that allow setting should always be disabled, not just until a request was made.
• We would like to have a behavior like HttpClient.Shared functionally behaves like new HttpClient() but some settings might be tuned to reflect that it's long lived
• Dispose on the shared instance will be an no-op
• We might want to have a shared property for handlers too, but not instead of this

namespace System.Net.Http;

public class HttpClient
{
    public static HttpClient Shared { get; }
}

[stephentoub]

@dotnet/ncl, I implemented this and started writing tests for it, and realized there are a variety of members that don't lend themselves well to this. DefaultRequestHeaders, for example, as it is not thread-safe, or CancelPendingRequests, as it'll end up canceling everything anyone scheduled to the instance. Do we still think it's a good idea to add, or should we close this?

[Tratcher]

there are a variety of members that don't lend themselves well to this. DefaultRequestHeaders, for example

We should ensure that all properties that allow setting should always be disabled

Yeah, most of the properties (7) don't make sense, but most of the methods and extensions do (dozens). Since the properties are only designed to be set by the person creating it anyways, and the user isn't creating this instance, would they expect to set these? It might be an FAQ, but one that could be answered by throwing a good exception?

[dersia]

I think this makes a lot of sense. 99% if uses that I see in code reviews are just using new HttpClient() and then just using the get/post/etc methods and most don't even use other properties. And often users don't understand how HttpClient is behaving when used with the using pattern, but they have learned to always use the using pattern whenever they see a disposable object. So having a shared instance makes a lot of sense.

After watching the stream on YouTube about this issue, I agree with @bartonjs that the defaults should be changed, although I understand that this would work for everyone and might be confusing, so I'd like to suggest another static member SslEnforcedShared or TlsEnforcedShared or so, that would have ramification checking turned on.

So I think it's important to keep this feature and document the issues well. (or even have analyzers for that?)

[stephentoub]

but one that could be answered by throwing a good exception?

In the PR i put up they end up throwing either NotSupportedException (getting the default headers instance) or InvalidOperationException (setting any of the properties).

[AraHaan]

I sure hope that in the impementation that the shared instance is not created until the property is first used (so that way we do not end up in a situation where the application would end up with 2 HttpClient instances where one is an advanced instance, and the other from HttpClient.Shared).

[Tratcher]

There will be a need to configure things like the protocol version and policy, but not in code, maybe via AppContext switches? E.g. when library code is written using the shared instance, how do you tell it to use HTTP/2 instead?

[MihaZupan]

You can still create a custom HttpRequestMessage instance and set the version/policy/custom headers there.
Those always take priority over anything set on HttpClient.

I sure hope that in the impementation that the shared instance is not created until the property is first used (so that way we do not end up in a situation where the application would end up with 2 HttpClient instances where one is an advanced instance, and the other from HttpClient.Shared).

It will be lazy to save a little of bit of startup time when not used, but even if it weren't, creating an HttpClient instance is almost free if you never actually use it.

[AraHaan]

You can still create a custom HttpRequestMessage instance and set the version/policy/custom headers there. Those always take priority over anything set on HttpClient.

I sure hope that in the impementation that the shared instance is not created until the property is first used (so that way we do not end up in a situation where the application would end up with 2 HttpClient instances where one is an advanced instance, and the other from HttpClient.Shared).

It will be lazy to save a little of bit of startup time when not used, but even if it weren't, creating an HttpClient instance is almost free if you never actually use it.

That is the problem, Microsoft themselves recommend that only a single instance of HttpClient be used globally due to the implementation of Windows (and unix websockets) having a hard coded limit until all sockets are exhausted. Also, some people like me do custom things like changing the base domain for the entire HttpClient as well, I guess a custom HttpRequestMessage implementation might handle that as well? The thing I can think of currently is when people use HttpClient for things like connecting to discord's apis endpoints to make rest calls to it.

[stephentoub]

That is the problem

What is the problem?

Nothing here changes any of that. If you want to completely ignore HttpClient.Shared, you can, and it won't take up any resources.

I guess a custom HttpRequestMessage implementation might handle that as well?

Yes, you can set an absolute url in the request message.

[MichalPetryka]

Microsoft themselves recommend that only a single instance of HttpClient be used globally due to the implementation of Windows (and unix websockets) having a hard coded limit until all sockets are exhausted.

IIRC HttpClient pools all the sockets internally, which means that the pool itself will be created lazily when used.

[stephentoub]

I still think this is valuable, but there's enough consternation over this that I'm going to close it.

[AraHaan]

I am not against this; my main point of view is that I think the shared instance should be created after it is first accessed so that way it will not cause problems for applications using advanced HttpClient logic. I also think it is valuable, however I also think that all possible ways that applications could be using HttpClient should be in mind as well at the same time (Basically only pay for what we use).

Also another thing to consider:

• is the shared instance thread safe?

[stephentoub]

my main point of view is that I think the shared instance should be created after it is first accessed so that way it will not cause problems for applications using advanced HttpClient logic

That's what the PR did.

is the shared instance thread safe?

Yes. HttpClient is in general.

[AraHaan]

Ah I see, well then I see no point in not doing it now that it is now cleared up on the confusion.