Skip to content
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

Closed
ahmettahasakar opened this issue Mar 1, 2019 · 40 comments
Closed

HttpClient has many TIME_WAIT or CLOSE_WAIT connections #28842

ahmettahasakar opened this issue Mar 1, 2019 · 40 comments
Labels
area-System.Net.Http question Answer questions and provide assistance, not an issue with source code or documentation.
Milestone

Comments

@ahmettahasakar
Copy link

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?

@davidsh
Copy link
Contributor

davidsh commented Mar 1, 2019

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.

@ahmettahasakar
Copy link
Author

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.

@stephentoub
Copy link
Member

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?

@davidsh
Copy link
Contributor

davidsh commented Mar 2, 2019

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;
}

@ahmettahasakar
Copy link
Author

ahmettahasakar commented Mar 2, 2019

It looks like it worked. Thank you. Perhaps you should update the code on Microsoft site as well.
@glennc @rynowak @stevejgordon

@ahmettahasakar
Copy link
Author

Nope. I spoke too early. Again I have over 4000 CLOSE_WAITS right now with response.Dispose() added. Do you think setting a Connection: close would help and is it necessary? @davidsh @stephentoub

@ahmettahasakar ahmettahasakar reopened this Mar 2, 2019
@stephentoub
Copy link
Member

Nope. I spoke too early. Again I have over 4000 CLOSE_WAITS right now with response.Dispose() added. Do you think setting a Connection: close would help and is it necessary?

Can you please share a repro?

@ahmettahasakar
Copy link
Author

It will take some time to prepare the repro. But meanwhile if it will help. Below is the netstat -a shows. Over 4000 rows of it. I'm using it to send push notifications to Firebase Cloud Messaging. And as I stated, i don't set base url. I just do http post with full urls, if it makes a different. So maybe Google is doing some weird stuff with their load balancers and servers perhaps to keep the connections open?

tcp        1      0 master-memberserv:44653 ams16s32-in-f10.1e1:443 CLOSE_WAIT
tcp        1      0 master-memberserv:32769 ams15s29-in-f106.1e:443 CLOSE_WAIT
tcp        1      0 master-memberserv:34231 ams15s32-in-f10.1e1:443 CLOSE_WAIT
tcp        1      0 master-memberserv:45417 fra02s28-in-f10.1e1:443 CLOSE_WAIT
tcp        1      0 master-memberserv:36765 ams16s31-in-f10.1e1:443 CLOSE_WAIT
tcp        1      0 master-memberserv:34433 ams15s21-in-f10.1e1:443 CLOSE_WAIT
tcp        1      0 master-memberserv:39045 fra02s28-in-f10.1e1:443 CLOSE_WAIT
tcp        1      0 master-memberserv:33929 ams16s31-in-f10.1e1:443 CLOSE_WAIT
tcp        1      0 master-memberserv:38447 ams15s30-in-f10.1e1:443 CLOSE_WAIT
tcp        1      0 master-memberserv:36419 ams15s21-in-f10.1e1:443 CLOSE_WAIT
tcp        1      0 master-memberserv:44855 ams15s40-in-f10.1e1:443 CLOSE_WAIT
tcp        1      0 master-memberserv:42707 ams15s40-in-f10.1e1:443 CLOSE_WAIT
tcp        1      0 master-memberserv:34921 ams15s30-in-f10.1e1:443 CLOSE_WAIT
tcp        1      0 master-memberserv:42767 ams15s29-in-f106.1e:443 CLOSE_WAIT
tcp        1      0 master-memberserv:35237 ams16s29-in-f10.1e1:443 CLOSE_WAIT
tcp        1      0 master-memberserv:45307 ams16s29-in-f10.1e1:443 CLOSE_WAIT
tcp        1      0 master-memberserv:40095 ams16s30-in-f10.1e1:443 CLOSE_WAIT
tcp        1      0 master-memberserv:46173 ams15s32-in-f10.1e1:443 CLOSE_WAIT
tcp        1      0 master-memberserv:44443 ams16s29-in-f10.1e1:443 CLOSE_WAIT
tcp        1      0 master-memberserv:38657 ams16s31-in-f10.1e1:443 CLOSE_WAIT
tcp        1      0 master-memberserv:39739 ams15s33-in-f10.1e1:443 CLOSE_WAIT
tcp        1      0 master-memberserv:34855 ams16s31-in-f10.1e1:443 CLOSE_WAIT
tcp        1      0 master-memberserv:36435 ams15s32-in-f10.1e1:443 CLOSE_WAIT
tcp        1      0 master-memberserv:46519 ams15s40-in-f10.1e1:443 CLOSE_WAIT
tcp        1      0 master-memberserv:42839 ams16s30-in-f10.1e1:443 CLOSE_WAIT
tcp        1      0 master-memberserv:38919 ams15s30-in-f10.1e1:443 CLOSE_WAIT
tcp        1      0 master-memberserv:46211 ams15s29-in-f106.1e:443 CLOSE_WAIT
tcp        1      0 master-memberserv:33343 ams15s40-in-f10.1e1:443 CLOSE_WAIT
tcp        1      0 master-memberserv:40581 ams16s32-in-f10.1e1:443 CLOSE_WAIT
tcp        1      0 master-memberserv:33939 ams16s30-in-f10.1e1:443 CLOSE_WAIT
tcp        1      0 master-memberserv:38275 fra02s28-in-f10.1e1:443 CLOSE_WAIT
tcp        1      0 master-memberserv:46397 ams15s32-in-f10.1e1:443 CLOSE_WAIT
tcp        1      0 master-memberserv:36333 ams15s30-in-f10.1e1:443 CLOSE_WAIT
tcp        1      0 master-memberserv:42675 ams16s31-in-f10.1e1:443 CLOSE_WAIT
tcp        1      0 master-memberserv:37631 ams16s29-in-f10.1e1:443 CLOSE_WAIT
tcp        1      0 master-memberserv:35741 ams15s33-in-f10.1e1:443 CLOSE_WAIT
tcp        1      0 master-memberserv:45223 ams16s31-in-f10.1e1:443 CLOSE_WAIT
tcp        1      0 master-memberserv:33375 fra02s28-in-f10.1e1:443 CLOSE_WAIT
tcp        1      0 master-memberserv:33375 fra02s28-in-f10.1e1:443 CLOSE_WAIT

@ahmettahasakar
Copy link
Author

@stephentoub I have created a test repo for this. https://github.com/ahmettahasakar/HttpClientTest
I made it so that there are 100 concurrent posts to Firebase Cloud Messaging. Even though I post blank payloads, the connections still seem to stay open.

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

@stephentoub
Copy link
Member

Thanks. I'll take a look.

Even though I post blank payloads, the connections still seem to stay open.

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.

@ahmettahasakar
Copy link
Author

@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 CLOSE_WAIT is normal. At times, I get errors about not being able to open any new TCP connections from other services. So these thousands of open connections that are just waiting to be reused, are clogging the sockets for other services.

@geoffkizer
Copy link
Contributor

Are you sure you are using the same HttpClient instance each time?

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.

If you are using the same URL and the same HttpClient instance, then connections should be reused, regardless of stuff like DNS round robin.

@ahmettahasakar
Copy link
Author

ahmettahasakar commented Mar 3, 2019

@geoffkizer I initiliaze clients with the line below
_client = clientFactory.CreateClient(FirebaseServiceClient);

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

@geoffkizer
Copy link
Contributor

I'm not really familiar with the IHttpClientFactory stuff; this is not part of HttpClient proper.

As a quick test, try something like Console.WriteLine($"{_client.GetHashCode()}") each time before you use it.

@ahmettahasakar
Copy link
Author

@geoffkizer I asked the same question in dotnet/extensions#1196 . @muratg said that it's a HttpClient issue

@geoffkizer
Copy link
Contributor

It could well be an HttpClient issue, but it would be good to verify that you are actually using the same HttpClient instance here.

@ahmettahasakar
Copy link
Author

@geoffkizer You are right. The GetHashCode() is different for each client

@geoffkizer
Copy link
Contributor

Great, thanks for the info.

@stephentoub
Copy link
Member

Are you sure you are using the same HttpClient instance each time?

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.
https://docs.microsoft.com/en-us/dotnet/api/system.net.http.ihttpclientfactory.createclient?view=aspnetcore-2.2
so the hash code of the HttpClient instance being different unfortunately doesn't necessarily mean that's the problem.

@ahmettahasakar
Copy link
Author

So what's the problem then? 😕

@stephentoub
Copy link
Member

stephentoub commented Mar 3, 2019

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.

@ahmettahasakar
Copy link
Author

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.

@stevejgordon
Copy link
Contributor

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...

It's not important in these scenarios. Disposing a request or response only calls Dispose on their Content field. Of the various HttpContent implementations, only StreamContent needs to dispose anything. HttpClient's default SendAsync fully buffers the response content and disposes the stream, so there's nothing the caller needs to do.

Originally posted by @Tratcher in aspnet/Security#886 (comment)

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?

@snehashankar
Copy link

@ahmettahasakar did your issue get resolved? I'm facing the same issue.
(I do not have my HttpClient disposable, however, my HttpRequestMessage and HttpResponseMessage are disposable)
How did you fix your issue?

@BoyFaceGirl
Copy link

BoyFaceGirl commented Mar 9, 2019

IHttpClientFactory 并不不是很方便,很多时候需要将http请求封闭到独立的类库中去,但是IHttpClientFactory似乎更多的是通过controller 构造函数注入来创建httpclient,不方便类库中相互调用

[edit by @davidsh .... google translate]
Ihttpclientfactory is not very convenient, and many times you need to close the HTTP request to a separate class library, but Ihttpclientfactory seems to be creating httpclient more through controller constructor injection. Inconvenient to call each other in class libraries

@ahmettahasakar
Copy link
Author

@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

@lobster2012-user
Copy link

lobster2012-user commented Mar 13, 2019

@ahmettahasakar
(Rewrote the text.)

1.If you do not need support for persistent connections, you can try Connection: close
2. It does not make sense to call dispose on httpclient, which is created through the factory,
look
LifetimeTrackingHttpMessageHandler
SetHandlerLifetime
The idle timeout for the factory defaults to 2 minutes. (idletime)
The idle timeout inside the pool is 2 minutes by default.
3
constant (ExpiryTime) = Infinity for factory
default (ExpiryTimeout) = infinity for pool

//pool
public static readonly TimeSpan DefaultPooledConnectionLifetime = Timeout.InfiniteTimeSpan;
public static readonly TimeSpan DefaultPooledConnectionIdleTimeout = TimeSpan.FromMinutes(2);
//factory
private TimeSpan _handlerLifetime = 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?

@ahmettahasakar
Copy link
Author

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"

@lobster2012-user
Copy link

lobster2012-user commented Mar 13, 2019

@ahmettahasakar

subsequent
Yes.

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);
        }
PS C:\Users\User> netstat -a | select-string insta
TCP    192.168.1.*:51319     instagram-p42-shv-01-arn2:https  ESTABLISHED
PS C:\Users\User>

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);
        }
PS C:\Users\User> netstat -a | select-string insta

  TCP    192.168.1.*:51407     instagram-p42-shv-01-arn2:https  ESTABLISHED
  TCP    192.168.1.*:51408     instagram-p42-shv-01-arn2:https  ESTABLISHED

2 connections
It is interesting ...

 /// <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; }

@ahmettahasakar
Copy link
Author

I think I missed the point. What do you mean @lobster2012-user

@lobster2012-user
Copy link

lobster2012-user commented Mar 14, 2019

@ahmettahasakar
I thought there was a problem in scope, but no.
Could not reproduce the problem, I would like to see the your repo. (Link above - I'll see.)
I tested everything on netcore2.2

@lobster2012-user
Copy link

@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.

@ahmettahasakar
Copy link
Author

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.

@lobster2012-user
Copy link

lobster2012-user commented Mar 14, 2019

@ahmettahasakar
I added await task; in cycle, and got only one openned connection. I tested with instagram host.
There are no redirects and one subdomain.
I suggest to test with the same address.
???????Possible problem due to redirects?????????

@stephentoub
Copy link
Member

And if you use the a static HttpClient, then even if they are parallel, it gets handled correctly.

@muratg, @glennc, I've not yet been able to investigate, but the description thus far really makes it sound like it's an HttpClientFactory issue.

@muratg
Copy link

muratg commented Mar 14, 2019

Thanks @stephentoub. @Tratcher, can you please take a look when you're back on Monday?

@ahmettahasakar
Copy link
Author

ahmettahasakar commented Mar 14, 2019

@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

@ahmettahasakar
Copy link
Author

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 client.PostAsync(). Infact they were from the firebase token authentication requests in Client Configuration action of IHttpClientFactory. So that's why when I switched to a singleton, or a static property, CreateClient(clientName) wasn't called more than once, and the problem was gone. Altough this was written in the documentation clearly, I had missed it. Each time CreateClient is called, a new instance of HttpClient is created and the configuration action is called. Thank you everyone for all the help.

@lobster2012-user
Copy link

@ahmettahasakar
(offtop)
The funny thing is that the library for Google can work with the ihttpclientfactory(own definition) factory, but FromStream (json) has no such argument, you can create an issue. If it makes sense to you

@msftgits msftgits transferred this issue from dotnet/corefx Feb 1, 2020
@msftgits msftgits added this to the 3.0 milestone Feb 1, 2020
@gsGabriel
Copy link

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
I replicated the problem in an application for testing in the following repository: https://github.com/gsGabriel/httpclient-factory/

the connections listed
image

Could someone tell me what is wrong with my implementation? I can't currently upgrade to .net core 3.0

@ghost ghost locked as resolved and limited conversation to collaborators Dec 14, 2020
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
area-System.Net.Http question Answer questions and provide assistance, not an issue with source code or documentation.
Projects
None yet
Development

No branches or pull requests