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

runtime/race: doesn't support sync.Pool #17306

Closed
pascaldekloe opened this issue Oct 1, 2016 · 6 comments
Closed

runtime/race: doesn't support sync.Pool #17306

pascaldekloe opened this issue Oct 1, 2016 · 6 comments
Labels
FrozenDueToAge NeedsFix The path to resolution is known, but the work has not been done. RaceDetector
Milestone

Comments

@pascaldekloe
Copy link
Contributor

The sync.Pool is disabled when the race detector is active with issue #7203. Accidental reuse of data after pool submissions is not unheard of.

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

go version devel +03a1dc3 Wed Sep 28 23:13:53 2016 +0000 darwin/amd64

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

GOARCH="amd64"
GOBIN=""
GOEXE=""
GOHOSTARCH="amd64"
GOHOSTOS="darwin"
GOOS="darwin"
GOPATH="/Users/pdekloe/repo/colfer"
GORACE=""
GOROOT="/Users/pdekloe/repo/go"
GOTOOLDIR="/Users/pdekloe/repo/go/pkg/tool/darwin_amd64"
CC="clang"
GOGCCFLAGS="-fPIC -m64 -pthread -fno-caret-diagnostics -Qunused-arguments -fmessage-length=0 -fdebug-prefix-map=/var/folders/sz/9wzvxsb57p73r4099d6_5zgr39574r/T/go-build242286039=/tmp/go-build -gno-record-gcc-switches -fno-common"
CXX="clang++"
CGO_ENABLED="1"

What did you do?

Cause a race condition by leaking data that was submitted into sync.Pool for reuse.

What did you expect to see?

A race detection.

What did you see instead?

My bug.

@ianlancetaylor
Copy link
Contributor

CC @dvyukov

@quentinmit quentinmit added the NeedsFix The path to resolution is known, but the work has not been done. label Oct 4, 2016
@quentinmit quentinmit added this to the Go1.8Maybe milestone Oct 4, 2016
@quentinmit
Copy link
Contributor

This may be hard to fix; this isn't a race in the traditional sense. (It's only a race if the object is then pulled out of the pool again before the next GC.) I don't know if the race detector has enough options to handle this.

@dvyukov
Copy link
Member

dvyukov commented Oct 5, 2016

@pascaldekloe do you have any real bug? If it's in open-source code, please point to it.

@dvyukov
Copy link
Member

dvyukov commented Oct 5, 2016

@quentinmit race detector has acquire, release annotations and ignores. It should be enough.

The problem with straightforward enabling of Pool under race detector is that it will introduce lots of parasitic synchronization (esp the Mutex) that can hide lots of unrelated races.

We could annotate Pool to synchronize on the objects cached (i.e. if goroutine A takes object X put by goroutine B, then A synchronizes only with B). But values are not necessary pointers.
We could also enable cache probablistically -- randomly decide to Put or to drop.

@pascaldekloe
Copy link
Contributor Author

pascaldekloe commented Oct 6, 2016

The uncommited code was ment for an opensource project. I've chosen to use a buffered channel as a pool instead.

Here's a playground example: https://play.golang.org/p/vwn35eBL8-

@gopherbot
Copy link

CL https://golang.org/cl/31589 mentions this issue.

@golang golang locked and limited conversation to collaborators Oct 30, 2017
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
FrozenDueToAge NeedsFix The path to resolution is known, but the work has not been done. RaceDetector
Projects
None yet
Development

No branches or pull requests

5 participants