-
Notifications
You must be signed in to change notification settings - Fork 4.7k
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
HttpClient has many TIME_WAIT or CLOSE_WAIT connections #28842
Comments
This usually means that your code using HttpClient is not disposing of the HttpResponseMessage objects etc. In order to provide further guidance, you'll need to submit a code sample of how you are using HttpClient. |
It's very standard actually. I created a named http client. I have injected IHttpClientFactory a transient service. I created a httpClient with the IHttpClientFactory.Create("namedClient") in the service constructor. Then I just started posting requests via service methods. I just followed https://docs.microsoft.com/en-us/aspnet/core/fundamentals/http-requests?view=aspnetcore-2.2#named-clients The only difference is that, I don't use baseUrl. I just have full url for posts. |
That example should be disposing of the HttpResponseMessage that results from awaiting the SendAsync call. Can you try doing the same in your code and see if that makes a difference? |
Example of a better recommended pattern: public async Task<IEnumerable<GitHubIssue>> GetAspNetDocsIssues()
{
HttpResponseMessage response = await Client.GetAsync(
"/repos/aspnet/docs/issues?state=open&sort=created&direction=desc");
try
{
response.EnsureSuccessStatusCode();
var result = await response.Content
.ReadAsAsync<IEnumerable<GitHubIssue>>();
}
finally
{
// Dispose of HttpResponseMessage to free up system resources and close network connections.
response.Dispose();
}
return result;
} |
It looks like it worked. Thank you. Perhaps you should update the code on Microsoft site as well. |
Nope. I spoke too early. Again I have over 4000 CLOSE_WAITS right now with |
Can you please share a repro? |
It will take some time to prepare the repro. But meanwhile if it will help. Below is the
|
@stephentoub I have created a test repo for this. https://github.com/ahmettahasakar/HttpClientTest In order for project to work, you need a Firebase authentication file. You can generate one easily from https://firebase.google.com/docs/admin/setup |
Thanks. I'll take a look.
It's expected that the connections stay open. You don't want them to stay open at all? By default connections are kept alive in a connection pool for several minutes in order to avoid the expense of creating and tearing them down for every request. |
@stephentoub I don't mind having the connections open. As you said, it's good to reuse them for performance. However, I think there is something else going on here. Although I post requests to the same URL, I believe because of Google's load balancers and DNS settings, it connects to different server most of the time. So it rarely reuses the same opened connection. And when I do 1000 posts, instead of reusing the connections, it just opens new connections to a different server each time. This is my theory by the way. I may be totally off, and all of this may be normal. But I don't think to have thousands of connections open with the state |
Are you sure you are using the same HttpClient instance each time?
If you are using the same URL and the same HttpClient instance, then connections should be reused, regardless of stuff like DNS round robin. |
@geoffkizer I initiliaze clients with the line below So my understanding from https://docs.microsoft.com/en-us/aspnet/core/fundamentals/http-requests?view=aspnetcore-2.2#named-clients is that the transient services are supposed to use the same client. Am I wrong? Isn't the whole point of IHttpClientFactory to manage HttpClients to be reused |
I'm not really familiar with the IHttpClientFactory stuff; this is not part of HttpClient proper. As a quick test, try something like |
@geoffkizer I asked the same question in dotnet/extensions#1196 . @muratg said that it's a HttpClient issue |
It could well be an HttpClient issue, but it would be good to verify that you are actually using the same HttpClient instance here. |
@geoffkizer You are right. The |
Great, thanks for the info. |
HttpClientFactory.CreateClient is guaranteed to return a new HttpClient instance each time, but it may wrap a shared HttpMessageHandler, which is actually what would be pooling connections. |
So what's the problem then? 😕 |
None of us has yet investigated. It could be in HttpClient, it could be in HttpClientFactory, it could be in the sample, etc. |
I did a quick debug. Perhaps it will give you a better picture. If I make HttpClient static (HashCode remains the same), then only a few connections open. The connection seems reused. If I create a named HttpClient, HashCode changes each time as it's supposed to. However, as you mentioned HttpMessageHandler is supposed to be reused. I have verified that in fact this is happening. IHttpClientFactory is feeding the same HttpMessageHandler to HttpClient constructor. But after that point, I'm not sure what's happening. It just opens a new connection every time. |
I'm a bit late to the party! @davidsh and @stephentoub - One of the areas I've been trying to establish some consistent common advice for is the disposal requirements of HttpRequestMessage and HttpResponseMessage. I did a small amount of digging myself, spoke to a few people and also a bit of GitHub searching. One reference I found was...
For request content at least, I believe the disposal statement might be superseeded by a change to the behaviour documented in the comments of HttpClient - https://github.com/dotnet/corefx/blob/e0ba7aa8026280ee3571179cc06431baf1dfaaac/src/System.Net.Http/src/System/Net/Http/HttpClient.cs#L540-L565 Other conversations have led me to conclude that disposal is only really required if the Content is a stream. Typically this leads to a discussion on whether your code "owns" the stream being used i.e. has grabbed it from the response content using ReadAsStreamAsync(). In that case, disposal seems to be required. In the docs sample, it uses the ReadAsAsync method from "Microsoft.AspNet.WebApi.Client" and underneath that uses the stream to deserialise via Newtonsoft.Json. I had a very quick look and couldn't see any obvious signs of their code disposing of the stream though. I planned to go back in and look at that some more. From your comments, it seems to me that the safest general advice is to wrap responses in a try/finally as suggest and be done with it. I'm not sure what the link is from disposing of the response and the number of connections in TIME_WAIT though? I wouldn't expect response.Dispose() to have any effect on connections as those are closed based on disposal of the handler. Or have I missed something here? |
@ahmettahasakar did your issue get resolved? I'm facing the same issue. |
IHttpClientFactory 并不不是很方便,很多时候需要将http请求封闭到独立的类库中去,但是IHttpClientFactory似乎更多的是通过controller 构造函数注入来创建httpclient,不方便类库中相互调用 [edit by @davidsh .... google translate] |
@snehashankar I ended up creating a singleton class that holds a named HttpClient. And i inject that class wherever i needed. This is the only way i get it to reuse connections |
@ahmettahasakar 1.If you do not need support for persistent connections, you can try Connection: close //pool
public static readonly TimeSpan DefaultPooledConnectionLifetime = Timeout.InfiniteTimeSpan;
public static readonly TimeSpan DefaultPooledConnectionIdleTimeout = TimeSpan.FromMinutes(2);
All connections live for 2 minutes by default(idle timeout) (the number is limited to SocketsHttpHandler.MaxConnectionsPerServer (default = InfinitY) What are your expectations from working with a factory? Not quite understand what singleton helped. They were created by hand or something without a factory, then yes, it became possible to kill them? |
I was expecting that if i used a named HttpClient, i would reuse the same sockets to post subsequent requests to the same url. However, it seemed that altough named HttpClient is supposed to use the same HttpMessageHandler, it opened a new connection eveytime for which i had to wait 2 minutes to close. In summary, instead of closing connections, i just wanted to reuse them , hence "named HttpClient" |
@ahmettahasakar
public bool Equals(HttpConnectionKey other) =>
Kind == other.Kind &&
Host == other.Host &&
Port == other.Port &&
ProxyUri == other.ProxyUri &&
SslHostName == other.SslHostName; [Test]
public async Task Test1()
{
var url = "https://www.instagram.com/?hl=ru";
var services = new ServiceCollection();
services.AddHttpClient();
services.AddHttpClient("name");
IHttpClientFactory factory = services.BuildServiceProvider().GetRequiredService<IHttpClientFactory>();
for (var i = 0; i < 10; ++i)
{
var http = factory.CreateClient("name");
await http.GetStringAsync(url);
}
await Task.Delay(10000);
}
1 Connection [Test]
public async Task Test1()
{
var url = "https://www.instagram.com/?hl=ru";
var services = new ServiceCollection();
services.AddHttpClient();
services.AddHttpClient("name");
services.AddHttpClient("name2");
IHttpClientFactory factory = services.BuildServiceProvider().GetRequiredService<IHttpClientFactory>();
for (var i = 0; i < 10; ++i)
{
var http = factory.CreateClient("name");
var http2 = factory.CreateClient("name2");
await http.GetStringAsync(url);
await http2.GetStringAsync(url);
}
await Task.Delay(10000);
}
2 connections /// <summary>
/// <para>
/// Gets or sets a value that determines whether the <see cref="IHttpClientFactory"/> will
/// create a dependency injection scope when building an <see cref="HttpMessageHandler"/>.
/// If <c>false</c> (default), a scope will be created, otherwise a scope will not be created.
/// </para>
/// <para>
/// This option is provided for compatibility with existing applications. It is recommended
/// to use the default setting for new applications.
/// </para>
/// </summary>
/// <remarks>
/// <para>
/// The <see cref="IHttpClientFactory"/> will (by default) create a dependency injection scope
/// each time it creates an <see cref="HttpMessageHandler"/>. The created scope has the same
/// lifetime as the message handler, and will be disposed when the message handler is disposed.
/// </para>
/// <para>
/// When operations that are part of <see cref="HttpMessageHandlerBuilderActions"/> are executed
/// they will be provided with the scoped <see cref="IServiceProvider"/> via
/// <see cref="HttpMessageHandlerBuilder.Services"/>. This includes retrieving a message handler
/// from dependency injection, such as one registered using
/// <see cref="HttpClientBuilderExtensions.AddHttpMessageHandler{THandler}(IHttpClientBuilder)"/>.
/// </para>
/// </remarks>
public bool SuppressHandlerScope { get; set; } |
I think I missed the point. What do you mean @lobster2012-user |
@ahmettahasakar |
@ahmettahasakar for (int i = 0; i < 100; i++)
{
FirebaseService firebaseService = (FirebaseService)_serviceProvider.GetService(typeof(FirebaseService));
var task = firebaseService.PostToFirebase(payload);
tasks.Add(task);
Console.WriteLine(i);
}
await Task.WhenAll(tasks.ToArray()); These are parallel calls. |
You can put await there to make them in sync. Same problem still exists. I tried every variation I could think of. And if you use the a static HttpClient, then even if they are parallel, it gets handled correctly. |
@ahmettahasakar |
Thanks @stephentoub. @Tratcher, can you please take a look when you're back on Monday? |
@lobster2012-user thank you for your effort. It would be great if you can try with the url i have been trying with. As you have read from my earlier messages, i also suspected something unique about dns and load balancers. But @geoffkizer said it shouldn't be an issue. @lobster2012-user I have added a little change to prove that even when in sync mode, the problem persists. https://github.com/ahmettahasakar/HttpClientTest/commit/65bcf465c7d6acccd5571f96182b1b1f0cd1c526 |
I just went over @lobster2012-user and tried to reproduce with the Instagram url. I realized that the connections that I had listed weren't originating from |
@ahmettahasakar |
I have the same problem or at least similar in .net core 2.2 and microsoft/dotnet:2.2-aspnetcore-runtime-alpine docker image, was this inssue solved in .net core 3.0? The problem is several open socket connections, even using the factory Could someone tell me what is wrong with my implementation? I can't currently upgrade to .net core 3.0 |
I use AddHttpClient() dependency injection to add a client to a service. At times, when I execute netstat -a on the server, I see many connections open with TIME_WAIT or CLOSE_WAIT status. I believe that these connections take up so much resource that, other TCP connections are unable to operate. Is this possible? Is there a way to stop these, and is it safe?
The text was updated successfully, but these errors were encountered: