Skip to content
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

net/http: TimeoutHandler allows wrapped handlers crash server with panics #22084

Closed
artyom opened this issue Sep 28, 2017 · 2 comments

Comments

Projects
None yet
4 participants
@artyom
Copy link
Contributor

commented Sep 28, 2017

Please answer these questions before submitting your issue. Thanks!

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.

https://play.golang.org/p/Ycz0W8a_ff

What did you expect to see?

I expect panics from http.Handlers to be handled by http.Server as documented (recovered), i.e. with the sentinel panic value http.ErrAbortHandler this program outputs something like this:

2017/09/28 22:55:18 Get http://127.0.0.1:52111: EOF

What did you see instead?

Program crashes:

panic: net/http: abort Handler

goroutine 21 [running]:
main.do.func1(0x13eb0e0, 0xc42011a0a0, 0xc420134000)
	/tmp/repro.go:19 +0x42
net/http.HandlerFunc.ServeHTTP(0x12c3438, 0x13eb0e0, 0xc42011a0a0, 0xc420134000)
	/Users/artyom/Library/go/src/net/http/server.go:1918 +0x44
net/http.(*timeoutHandler).ServeHTTP.func1(0xc420076c00, 0xc42011a0a0, 0xc420134000, 0xc420138060)
	/Users/artyom/Library/go/src/net/http/server.go:3043 +0x53
created by net/http.(*timeoutHandler).ServeHTTP
	/Users/artyom/Library/go/src/net/http/server.go:3042 +0x158
exit status 2

System details

go version go1.9 darwin/amd64
GOARCH="amd64"
GOBIN=""
GOEXE=""
GOHOSTARCH="amd64"
GOHOSTOS="darwin"
GOOS="darwin"
GOPATH="/tmp/go:/Users/artyom/go"
GORACE=""
GOROOT="/Users/artyom/Library/go"
GOTOOLDIR="/Users/artyom/Library/go/pkg/tool/darwin_amd64"
GCCGO="gccgo"
CC="clang"
GOGCCFLAGS="-fPIC -m64 -pthread -fno-caret-diagnostics -Qunused-arguments -fmessage-length=0 -fdebug-prefix-map=/var/folders/lb/3rk8rqs53czgb4v35w_342xc0000gn/T/go-build484141897=/tmp/go-build -gno-record-gcc-switches -fno-common"
CXX="clang++"
CGO_ENABLED="1"
CGO_CFLAGS="-g -O2"
CGO_CPPFLAGS=""
CGO_CXXFLAGS="-g -O2"
CGO_FFLAGS="-g -O2"
CGO_LDFLAGS="-g -O2"
PKG_CONFIG="pkg-config"
GOROOT/bin/go version: go version go1.9 darwin/amd64
GOROOT/bin/go tool compile -V: compile version go1.9
uname -v: Darwin Kernel Version 17.0.0: Thu Aug 24 21:48:19 PDT 2017; root:xnu-4570.1.46~2/RELEASE_X86_64
ProductName:	Mac OS X
ProductVersion:	10.13
BuildVersion:	17A365
lldb --version: lldb-900.0.45
  Swift-4.0

This happens because TimeoutHandler starts original handler in a separate goroutine:

go/src/net/http/server.go

Lines 3042 to 3045 in 78a0a31

go func() {
h.handler.ServeHTTP(tw, r)
close(done)
}()

I can work on fixing this by handling panic in this goroutine and re-raising it from the timeoutHandler.ServeHTTP itself, so it's handled by the Server. Please assign this to me if this solution looks ok.

@mvdan

This comment has been minimized.

Copy link
Member

commented Sep 28, 2017

@tombergan tombergan self-assigned this Sep 29, 2017

@tombergan tombergan added this to the Go1.10 milestone Sep 29, 2017

@gopherbot

This comment has been minimized.

Copy link

commented Oct 1, 2017

Change https://golang.org/cl/67410 mentions this issue: net/http: make TimeoutHandler recover child handler panics

@gopherbot gopherbot closed this in d1bef43 Oct 2, 2017

@golang golang locked and limited conversation to collaborators Oct 2, 2018

Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
You can’t perform that action at this time.