Commit c80e0d37 authored by Brad Fitzpatrick's avatar Brad Fitzpatrick

net/http: fix data race with concurrent use of Server.Serve

Fixes #16505

Change-Id: I0afabcc8b1be3a5dbee59946b0c44d4c00a28d71
Reviewed-on: https://go-review.googlesource.com/25280
Run-TryBot: Brad Fitzpatrick <bradfitz@golang.org>
TryBot-Result: Gobot Gobot <gobot@golang.org>
Reviewed-by: default avatarChris Broadfoot <cbro@golang.org>
parent 4a15508c
......@@ -4716,3 +4716,14 @@ func BenchmarkCloseNotifier(b *testing.B) {
}
b.StopTimer()
}
// Verify this doesn't race (Issue 16505)
func TestConcurrentServerServe(t *testing.T) {
for i := 0; i < 100; i++ {
ln1 := &oneConnListener{conn: nil}
ln2 := &oneConnListener{conn: nil}
srv := Server{}
go func() { srv.Serve(ln1) }()
go func() { srv.Serve(ln2) }()
}
}
......@@ -2129,8 +2129,8 @@ type Server struct {
ErrorLog *log.Logger
disableKeepAlives int32 // accessed atomically.
nextProtoOnce sync.Once // guards initialization of TLSNextProto in Serve
nextProtoErr error
nextProtoOnce sync.Once // guards setupHTTP2_* init
nextProtoErr error // result of http2.ConfigureServer if used
}
// A ConnState represents the state of a client connection to a server.
......@@ -2260,10 +2260,8 @@ func (srv *Server) Serve(l net.Listener) error {
}
var tempDelay time.Duration // how long to sleep on accept failure
if srv.shouldConfigureHTTP2ForServe() {
if err := srv.setupHTTP2(); err != nil {
return err
}
if err := srv.setupHTTP2_Serve(); err != nil {
return err
}
// TODO: allow changing base context? can't imagine concrete
......@@ -2408,7 +2406,7 @@ func (srv *Server) ListenAndServeTLS(certFile, keyFile string) error {
// Setup HTTP/2 before srv.Serve, to initialize srv.TLSConfig
// before we clone it and create the TLS Listener.
if err := srv.setupHTTP2(); err != nil {
if err := srv.setupHTTP2_ListenAndServeTLS(); err != nil {
return err
}
......@@ -2436,14 +2434,36 @@ func (srv *Server) ListenAndServeTLS(certFile, keyFile string) error {
return srv.Serve(tlsListener)
}
func (srv *Server) setupHTTP2() error {
// setupHTTP2_ListenAndServeTLS conditionally configures HTTP/2 on
// srv and returns whether there was an error setting it up. If it is
// not configured for policy reasons, nil is returned.
func (srv *Server) setupHTTP2_ListenAndServeTLS() error {
srv.nextProtoOnce.Do(srv.onceSetNextProtoDefaults)
return srv.nextProtoErr
}
// setupHTTP2_Serve is called from (*Server).Serve and conditionally
// configures HTTP/2 on srv using a more conservative policy than
// setupHTTP2_ListenAndServeTLS because Serve may be called
// concurrently.
//
// The tests named TestTransportAutomaticHTTP2* and
// TestConcurrentServerServe in server_test.go demonstrate some
// of the supported use cases and motivations.
func (srv *Server) setupHTTP2_Serve() error {
srv.nextProtoOnce.Do(srv.onceSetNextProtoDefaults_Serve)
return srv.nextProtoErr
}
func (srv *Server) onceSetNextProtoDefaults_Serve() {
if srv.shouldConfigureHTTP2ForServe() {
srv.onceSetNextProtoDefaults()
}
}
// onceSetNextProtoDefaults configures HTTP/2, if the user hasn't
// configured otherwise. (by setting srv.TLSNextProto non-nil)
// It must only be called via srv.nextProtoOnce (use srv.setupHTTP2).
// It must only be called via srv.nextProtoOnce (use srv.setupHTTP2_*).
func (srv *Server) onceSetNextProtoDefaults() {
if strings.Contains(os.Getenv("GODEBUG"), "http2server=0") {
return
......
Markdown is supported
0%
or
You are about to add 0 people to the discussion. Proceed with caution.
Finish editing this message first!
Please register or to comment