x/net/http2: ConfigureServer stops Shutdown from sending GOAWAY #18471
Comments
|
My guess here is that I shouldn't be using ConfigureServer at all but the behavior seems odd anyhow. |
|
In particular if you do something like this (https://github.com/golang/tools/blob/master/cmd/godoc/autocert.go#L38) then calling srv.Shutdown will not send GOAWAYs to clients. |
|
I suspect the problem is that when you use // h1ServerShutdownChan returns a channel that will be closed when the
// provided *http.Server wants to shut down.
//
// This is a somewhat hacky way to get at http1 innards. It works
// when the http2 code is bundled into the net/http package in the
// standard library. The alternatives ended up making the cmd/go tool
// depend on http Servers. This is the lightest option for now.
// This is tested via the TestServeShutdown* tests in net/http.
func h1ServerShutdownChan(hs *http.Server) <-chan struct{} {
if fn := testh1ServerShutdownChan; fn != nil {
return fn(hs)
}
var x interface{} = hs
type I interface {
getDoneChan() <-chan struct{}
}
if hs, ok := x.(I); ok {
return hs.getDoneChan()
}
return nil
} |
|
That's correct, Kale. I propose that calling the http2.ConfigureServer to configure max streams and such should not stop the bundled http.http2ConfigureServer from working. The latter is called in the callgraph of http.ListenAndServeTLS and should take precedence over the former for the purposes of GOAWAY. I'm yet to figure out why http.http2ConfigureServer fails if http2.ConfigureServer is called before it. |
|
/cc @bradfitz |
|
CL https://golang.org/cl/44003 mentions this issue. |
|
CL https://golang.org/cl/43230 mentions this issue. |
This will be used to allow http2 servers to register a shutdown function so that net/http.Server.Shutdown will work when the http2 server is configured via a manual call to http2.ConfigureServer. Currently, Shutdown only works when the http2 server is configured automatically by the net/http package. Updates #20302 Updates #18471 Change-Id: Ifc2b5f3126126a106b49ea4a7e999279852b9cc9 Reviewed-on: https://go-review.googlesource.com/44003 Run-TryBot: Tom Bergan <tombergan@google.com> TryBot-Result: Gobot Gobot <gobot@golang.org> Reviewed-by: Brad Fitzpatrick <bradfitz@golang.org>
This is a better fix that https://golang.org/cl/43455. Instead of creating a separate goroutine to wait for the global shutdown channel, we reuse the new serverMsgCh, which was added in a prior CL. We also use the new net/http.Server.RegisterOnShutdown method to register a shutdown callback for each http2.Server. Updates golang/go#20302 Updates golang/go#18471 Change-Id: Icf29d5e4f65b3779d1fb4ea92924e4fb6bdadb2a Reviewed-on: https://go-review.googlesource.com/43230 Run-TryBot: Tom Bergan <tombergan@google.com> Reviewed-by: Brad Fitzpatrick <bradfitz@golang.org>
|
CL https://golang.org/cl/44006 mentions this issue. |
This is a better fix that https://golang.org/cl/43455. Instead of creating a separate goroutine to wait for the global shutdown channel, we reuse the new serverMsgCh, which was added in a prior CL. We also use the new net/http.Server.RegisterOnShutdown method to register a shutdown callback for each http2.Server. Updates golang/go#20302 Updates golang/go#18471 Change-Id: Icf29d5e4f65b3779d1fb4ea92924e4fb6bdadb2a Reviewed-on: https://go-review.googlesource.com/43230 Run-TryBot: Tom Bergan <tombergan@google.com> Reviewed-by: Brad Fitzpatrick <bradfitz@golang.org>
What version of Go are you using (
go version)?What operating system and processor architecture are you using (
go env)?What did you do?
If possible, provide a recipe for reproducing the error.
A complete runnable program is good.
A link on play.golang.org is best.
Used this program and a self signed cert and key (generated by crypto/tls/generate_cert.go)
Run this without
-configureand it sends GOAWAY frames (seen in nghttp)But with -configure, nghttp does not see a GOAWAY, only DATA frames.
What did you expect to see?
I expected http2.ConfigureServer to not stop Shutdown from sending GOAWAY frames on server connections.
What did you see instead?
http2.ConfigureServer stops Shutdown from sending GOAWAY frames.
[Note: Verified with latest golang.org/x/net/http2 SHA: 8fd7f25955530b92e73e9e1932a41b522b22ccd9]
The text was updated successfully, but these errors were encountered: