-
Notifications
You must be signed in to change notification settings - Fork 18k
x/net/http2: SetKeepAlivesEnabled(false) closes all HTTP/2 connections older than 5 seconds #36946
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
Comments
@bradfitz It seems the connection should be transitioned to StateHijacked. I haven't checked but I believe we are just collecting http/2 connections. |
@fraenkel @bradfitz It looks like https://play.golang.org/p/UtgaaHEJjYw Here is example output running with HTTP/2, you can see that request 4 comes back with an
and with
|
@fraenkel Agreed, I too am very curious to learn more. |
/cc @bradfitz @tombergan |
@fraenkel Is there anything I can do to help make progress? I am very interested in addressing this issue as it is causing us upgrade issues in production. Right now my only fix is to disable HTTP/2. |
Not at this time. |
This should be fixed by #39776 |
In that case, should the CL reference this issue? |
@fraenkel Thank you so much for looking into this issue and taking a stab at fixing it! I really appreciate it. I'm not sure the changes in https://golang.org/cl/240278 completely address the issue, however. I just reran my two reproduction programs from above using the CL. The first one (https://play.golang.org/p/7mPbWhl7QeA) is fixed but I think the second one (https://play.golang.org/p/UtgaaHEJjYw) still has issues:
The client never gets a response for request 4 despite the fact that |
Please use GODEBUG=http2debug=2. Depending on what LastStreamId is sent in the GoAway will decide whether its correct or not. |
New issue please. |
Change https://golang.org/cl/240278 mentions this issue: |
On Server.Shutdown, all idle connections are closed. A caveat for new connections is that they are marked idle after 5 seconds. Previously new HTTP/2 connections were marked New, and after 5 seconds, they would then become idle. With this change, we now mark HTTP/2 connections as Active to allow the proper shutdown sequence to occur. Fixes #36946 Fixes #39776 Change-Id: I31efbf64b9a2850ca544da797f86d7e1b3378e8b Reviewed-on: https://go-review.googlesource.com/c/go/+/240278 Reviewed-by: Emmanuel Odeke <emm.odeke@gmail.com> Run-TryBot: Emmanuel Odeke <emm.odeke@gmail.com> TryBot-Result: Gobot Gobot <gobot@golang.org>
What version of Go are you using (
go version
)?Does this issue reproduce with the latest release?
Yes.
What operating system and processor architecture are you using (
go env
)?go env
OutputWhat did you do?
Here is a play.golang.org link for a program which reproduces the issue:
https://play.golang.org/p/7mPbWhl7QeA
it may be necessary to save it to a
main.go
file and run it manually withHere is an example output from running the program:
The program starts up an HTTPS server and has a client send it requests in a loop. Importantly, both the client and server are configured to use HTTP/2 over TLS and the client reuses a single TCP connection for each request. The handler for the HTTPS server blocks for 3 seconds and then sends back its response. On the fourth request, once we can be sure that the client connection has been open for at least 5 seconds,
SetKeepAlivesEnabled(false)
is called on the server within the handler of the request, before the response has been written out.What did you expect to see?
After the call to
SetKeepAlivesEnabled
, I expected the requests sent by the client to continue working as normal. The documentation forSetKeepAlivesEnabled
claims only to disable HTTP keep-alives, however by looking at the source code one can see it also apparently calls a method calledcloseIdleConns
. However, because we are callingSetKeepAlivesEnabled
within the handler of a request that has not yet been responded to yet, clearly this connection is not idle. The documentation forStateIdle
is defined to be:What did you see instead?
The connection is closed by
SetKeepAlivesEnabled
viacloseIdleConns
and the client request fails, coming back with anunexpected EOF
error. I believe this is ultimately due to the fact that the HTTP/2 code keeps its own connection state bookkeeping which is not shared with the HTTP/1 server. Using thehttp.Server.ConnState
hook and some sneaky use of theruntime
package, the logs in the example output above show that, according to the HTTP/1 server, the connection never transitions out of theStateNew
state while the HTTP/2 code transitions it betweenStateActive
andStateIdle
with each new request on the connection.Add to that this heuristic within
closeIdleConns
:and you get the outcome that any HTTP/2 connection that has been open for longer than 5 seconds will be closed by a call to
SetKeepAlivesEnabled(false)
. This heuristic was apparently introduced by #22682.You can run the example program with both the client and the server configured to use HTTP/1 by passing
-http=false
:which shows that the client requests continue working after the call to
SetKeepAlivesEnabled(false)
. Here is an example output running with HTTP/1:Please let me know if I can provide any additional information about the issue.
The text was updated successfully, but these errors were encountered: