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
HTTP/2 connections to Google GFE aren't reused! #750
Comments
Confirmed OS independent because I get "connection dead" on OS X too. So far I've seen dead connections only with Google Frontend (GFE), and e.g. http2.golang.org doesn't have the same message. Even with those servers without connection dead, I don't think connection reuse works always properly because there's many TLS handshakes. |
Here's example of http2.golang.org and connection reuse not working. https://gist.github.com/joneskoo/924586cd853e0b2063aa6d6bd51f3e0a I would expect the TLS connection to be reused in that case. |
Based on IRC discussion on #curl what I saw against http2.golang.org is fixed in 1fc7672. Issue with GFE |
The "seems to be dead" logic checks if the socket is readable when it is about to get reused, as it shouldn't be. A readable socket there implies that data has arrived (that we haven't taken care of) or that the socket is closed. Either way we can't reuse it since its state is unknown. As a debug effort I added a call to This case is quirky to check with wireshark too since it is all HTTPS. |
With curl+nss on Homebrew: Homebrew/homebrew-core#49
The PING frame from server probably confuses cURL. |
I think a first take on this, is to make sure we drain the socket and send back http/2 frames even in this case when the a single stream happens to be done. With this little patch my simplest ways of reproducing this problem are now working correctly.
|
This fix will only really make it work because the PING immediately follows the DATA. If the PING would instead just be delayed and come a few hundred milliseconds or so later, we will not drain the socket there and subsequently fail on the reuse check again. But to work on that angle of the problem, I think I need to produce a test case that behaves like that first. Baby steps. |
It turns out the google GFE HTTP/2 servers send a PING frame immediately after a stream ends and its last DATA has been received by curl. So if we don't drain that from the socket, it makes the socket readable in subsequent checks and libcurl then (wrongly) assumes the connection is dead when trying to reuse the connection. Reported-by: Joonas Kuorilehto Discussed in #750
The fix you applied c8ab613 removes what appears to have been for preventing streams from getting stuck when close is true and the stream has some DATA, see d261652. Did you test for a regression or is there already a test that covers this? |
also ping @tatsuhiro-t see if he has any ideas on this |
I just read the code and I ran a couple of tests on live sites. Oh , the lack of HTTP/2 tests .... :-( |
The removed part is required when response header does not contain content-length header field. This is probably a long time goal, but in my opinion, it would be good to have some kind of "idle" handle which takes care of the HTTP/2 connection when no active regular handles exist. It will handle all ping/settings over the connection while there is no handles using that connection. |
I completely agree @tatsuhiro-t, and I've added a mention about that need in the KNOWN_BUGS document now. As a first stop gap we can at least make sure that we consume the trailing ping if we read it as part of that last |
What about preventing
|
The above patch works with the response without content-length, but we don't read PING frame with that. Perhaps, they are in the different packet? |
Correct. In fact my test case with two google servers after another fails with this patch so clearly that case needs the extra recv() to work...! How about detecting the closed stream in case
|
It seems to work for me too. Lacking test case (especially for corner cases) really hurts us here.. |
Agreed. I need to get working on this... Thanks anyway, I'll push that patch for now. |
I did this
I expected the following
... that the single HTTPS connection would be kept alive after the first request and reused for the subsequent one, as it is if you replace
www.google.se
withdaniel.haxx.se
.The interesting snippet of the trace at the end of the first request and the beginning of the second says:
curl/libcurl version
operating system
Debian Linux, kernel 4.4.6 but this is most likely an OS independent problem.
The text was updated successfully, but these errors were encountered: