net/http: Client errors caused by Server.Shutdown should be always retryable

[tocrafty]

Go version

go version go1.21.3 linux/amd64

Output of go env in your module/workspace:

GO111MODULE=''
GOARCH='amd64'
GOBIN=''
GOCACHE='/root/.cache/go-build'
GOENV='/root/.config/go/env'
GOEXE=''
GOEXPERIMENT=''
GOFLAGS=''
GOHOSTARCH='amd64'
GOHOSTOS='linux'
GOINSECURE=''
GOMODCACHE='/root/go/pkg/mod'
GONOPROXY=''
GONOSUMDB=''
GOOS='linux'
GOPATH='/root/go'
GOPRIVATE=''
GOPROXY='xxxxxx,direct'
GOROOT='/root/sdk/go1.21.3'
GOSUMDB='xxxx'
GOTMPDIR=''
GOTOOLCHAIN='auto'
GOTOOLDIR='/root/sdk/go1.21.3/pkg/tool/linux_amd64'
GOVCS=''
GOVERSION='go1.21.3'
GCCGO='gccgo'
GOAMD64='v1'
AR='ar'
CC='gcc'
CXX='g++'
CGO_ENABLED='1'
GOMOD='/dev/null'
GOWORK=''
CGO_CFLAGS='-O2 -g'
CGO_CPPFLAGS=''
CGO_CXXFLAGS='-O2 -g'
CGO_FFLAGS='-O2 -g'
CGO_LDFLAGS='-O2 -g'
PKG_CONFIG='pkg-config'
GOGCCFLAGS='-fPIC -m64 -pthread -Wl,--no-gc-sections -fmessage-length=0 -ffile-prefix-map=/tmp/go-build82557609=/tmp/go-build -gno-record-gcc-switches'

What did you do?

I write a demo to simulate http shutdown.
The demo start a http server and N goroutine to send http POST requests. Server and client each has a cache data, aka serverData and clientData. N goroutines are indexed by id, and eagerly send Post request to server with id carried by body. Once HTTP server receives a request, it increases the corresponding serverData and sends httpOK response. Client goroutine quits for error or increases clientData by one.
At the end of program, Server.Shutdown is called. Wait all client goroutine exit. Check whether serverData and clientData is equal.
If net/http.Server.Shutdown is perfect graceful, the following situation should not happen.

Also note that closing a connection does not terminate any associated goroutine running ServeHTTP: those goroutines will continue to run until they decide to return (perhaps as a result of noticing a failed write to the response).

Any client error caused by shutdown should be retryable. This is more friendly for client other than undefined server behavior.

An apparently fix would be checking Server.shuttingDown after readRequest but before ServeHTTP.

What did you see happen?

build and run the demo, I get the following output:

http.Server.Serve http: Server closed
Post Post "http://127.0.0.1:47681/": EOF
Post Post "http://127.0.0.1:47681/": EOF
Post Post "http://127.0.0.1:47681/": EOF
Post Post "http://127.0.0.1:47681/": EOF
Post Post "http://127.0.0.1:47681/": EOF
Post Post "http://127.0.0.1:47681/": EOF
Post Post "http://127.0.0.1:47681/": EOF
Post Post "http://127.0.0.1:47681/": EOF
Post Post "http://127.0.0.1:47681/": http: server closed idle connection
Post Post "http://127.0.0.1:47681/": http: server closed idle connection
Post Post "http://127.0.0.1:47681/": http: server closed idle connection
Post Post "http://127.0.0.1:47681/": http: server closed idle connection
Post Post "http://127.0.0.1:47681/": http: server closed idle connection
Post Post "http://127.0.0.1:47681/": http: server closed idle connection
Post Post "http://127.0.0.1:47681/": http: server closed idle connection
Post Post "http://127.0.0.1:47681/": http: server closed idle connection
Post Post "http://127.0.0.1:47681/": http: server closed idle connection
Post Post "http://127.0.0.1:47681/": http: server closed idle connection
Post Post "http://127.0.0.1:47681/": http: server closed idle connection
Post Post "http://127.0.0.1:47681/": http: server closed idle connection
Post Post "http://127.0.0.1:47681/": http: server closed idle connection
Post Post "http://127.0.0.1:47681/": http: server closed idle connection
Post Post "http://127.0.0.1:47681/": http: server closed idle connection
Post Post "http://127.0.0.1:47681/": http: server closed idle connection
Post Post "http://127.0.0.1:47681/": http: server closed idle connection
Post Post "http://127.0.0.1:47681/": http: server closed idle connection
Post Post "http://127.0.0.1:47681/": http: server closed idle connection
Post Post "http://127.0.0.1:47681/": http: server closed idle connection
Post Post "http://127.0.0.1:47681/": http: server closed idle connection
Post Post "http://127.0.0.1:47681/": http: server closed idle connection
Post Post "http://127.0.0.1:47681/": http: server closed idle connection
Post Post "http://127.0.0.1:47681/": http: server closed idle connection
Post Post "http://127.0.0.1:47681/": http: server closed idle connection
Post Post "http://127.0.0.1:47681/": http: server closed idle connection
Post Post "http://127.0.0.1:47681/": http: server closed idle connection
Post Post "http://127.0.0.1:47681/": http: server closed idle connection
Post Post "http://127.0.0.1:47681/": http: server closed idle connection
http.Server.Shutdown <nil>
data at index 17 mismatch, client 6091, server 6092
data at index 36 mismatch, client 6081, server 6082
data at index 59 mismatch, client 6038, server 6039
data at index 61 mismatch, client 5971, server 5972
data at index 75 mismatch, client 5979, server 5980
data at index 85 mismatch, client 6075, server 6076
data at index 99 mismatch, client 6044, server 6045

What did you expect to see?

If http.Server.Shutdown is perfect graceful, there should be no logs like data at index 99 mismatch, client 6044, server 6045.

[gopherbot]

Change https://go.dev/cl/565277 mentions this issue: net/http: check server shutting down before processing the request

[thanm]

@neild per owners