x/sync/singleflight: panic in Do fn results in deadlock

[xonstone]

What version of Go are you using (go version)?

go version go1.12.7 darwin/amd64

Does this issue reproduce with the latest release?

This is the newest stable version

What operating system and processor architecture are you using (go env)?

go env Output

$ go env
GOARCH="amd64"
GOBIN=""
GOCACHE="/Users/user/Library/Caches/go-build"
GOEXE=""
GOFLAGS=""
GOHOSTARCH="amd64"
GOHOSTOS="darwin"
GOOS="darwin"
GOPATH="/Users/user/go"
GOPROXY=""
GORACE=""
GOROOT="/usr/local/Cellar/go/1.12.7/libexec"
GOTMPDIR=""
GOTOOLDIR="/usr/local/Cellar/go/1.12.7/libexec/pkg/tool/darwin_amd64"
GCCGO="gccgo"
CC="clang"
CXX="clang++"
CGO_ENABLED="1"
GOMOD=""
CGO_CFLAGS="-g -O2"
CGO_CPPFLAGS=""
CGO_CXXFLAGS="-g -O2"
CGO_FFLAGS="-g -O2"
CGO_LDFLAGS="-g -O2"
PKG_CONFIG="pkg-config"
GOGCCFLAGS="-fPIC -m64 -pthread -fno-caret-diagnostics -Qunused-arguments -fmessage-length=0 -fdebug-prefix-map=/var/folders/lq/0sdfc0ps1j5921_9zyy2cmz80000gn/T/go-build771037982=/tmp/go-build -gno-record-gcc-switches -fno-common"

What did you do?

The fn given to the singleflight group.Do panicked and was recovered outside the group.Do.

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

What did you expect to see?

The call would be discarded and the lock released.(Maybe the panic would be passed to all waiting calls with the same key)

What did you see instead?

The call panicked but the lock was never released resulting in a deadlock of all following calls with the same key.

[av86743]

panic occurs upon invocation of fn() here . Unfortunately API of DoChan/Result does not provide means to return/propagate panic.

[beoran]

The simplest workaround is to defer a recover inside the callback function that assigns it's value to a variable in the outer scope, like this:

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

Hope this is of any huse to you.

[xonstone]

Yeah, that's what I did but I would or document that this is needed or let singleflight also handle panics hence this issue.

[tv42]

singleflight might have chosen to not defer recover because of defer performance has historically been pretty noticable and groupcache (where singleflight originates) definitely strived to be fast.

[xonstone]

At first sight the implementation in groupcache seems to have the same problem

[ayanamist]

Since go1.14 add inlined defer support, will this be fixed to avoid dead lock?

[dmitshur]

/cc @rsc @ianlancetaylor @dvyukov @aclements per owners.

[dmitshur]

There was another report with detail that might be relevant in issue #41133. /cc @kirk91

[gopherbot]

Change https://golang.org/cl/251677 mentions this issue: singleflight: fix hangs after first Do panic

[bcmills]

Fixed by CL 251677.

[gopherbot]

Change https://golang.org/cl/260717 mentions this issue: singleflight: skip tests involving exec on js/wasm