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
Upgrade shutting down early for http2 connections but not http1.1 #12
Comments
For what its worth I tested using a golang client and could not reproduce the issue. So many its something to do with how curl negotiates the connection? |
What do you mean when you say "drops connections"? Command line output would be useful. A quick google points towards http/2 having support for a message which indicates that a connection should not be used anymore (GOAWAY frame). HTTP/1 doesn't have such a thing (sadly) so might take longer to close. |
I've got a simple example that reproduces something similar.
This is attempting to start a request that will take 20 seconds. While that request is running we run a sighup to trigger the upgrade. If this is done over http2/ssl using curl it interrupts the curl to localhost:8080/test. If you do this without TLS or without curl it works. I've only tried it using curl and Go as the clients. Go worked but curl did not. curl error: "curl: (56) Unexpected EOF" My curl version if it matters
`
` |
I see the same behaviour as well, on Go 1.11.
I think the server should send a GOAWAY frame in this case. Adding a sleep after Shutdown() has returned does not fix this, so it's not just a case of not waiting long enough. |
@lmb do you believe this might be a bug in tableflip? Or something that can be addressed in the example code the readme provides? |
I don't think this is a tableflip bug, mainly because there is nothing specific to http2 in the library. I've run the test case a few more times, most of the time I get a GOAWAY, but sometimes the connection just drops. This seems like a race condition in net/http2 of some sort. I need to find a reliable way to reproduce this to figure out what is going on. |
It may not be a tableflip bug but the example uses the http service. I think most would infer that exact code will work. Maybe the example can be updated so that it will work with HTTP 2 but I’m not sure if that’s possible. It does limit the usefulness if this can’t be used with go HTTP processes especially since you can’t force it to use HTTP 1 all the time. |
I've repeated the test using curl and snooped the traffic using wireshark. Curl says:
From wireshark it's clear that GOAWAY is sent:
Seems like this is a bug in curl, which was fixed in curl/curl#2510 / 7.60. Go net/http in combination with tableflip is doing the right thing here. Re-reading your first comment it seems like you expect Go to wait 20s until exiting, the same as with HTTP/1? |
My example code the 20 seconds is just to simulate an HTTP request doing some work and finishing even after the sighup signal. If this is just a curl bug that would be great. I was just testing tableflip using curl and ran into this issue. Similar to you I tried my test using the Go http client and could not reproduce the problem. My local curl version is "curl 7.54.0" so it would fall into the version affected by the bug you posted. I think we can close this unless someone comes up with another way to reproduce using a newer curl or go client. Agree? |
Well, I think your test points out an interesting difference between H1 and H2. H1 will block until the handler has exited, while H2 "aborts" the handler. This is potentially a bug in the go standard library, do you want to raise an issue with them? In the meantime I'll close this issue. |
I'm running a web process on FreeBSD. Using curl to connect over http2 when I call SIGHUP on the process it shuts down and drops any connections without waiting for them to complete.
If I do the same but use the curl flag '--http1.1' the running connections complete before shutdown is called.
Any idea why http2 would not wait for the connections to complete? While http1.1 connections would?
Thank you,
The text was updated successfully, but these errors were encountered: