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

Jump to bottom

sync: clarify Cond "created as part of other structures" documentation #20471

Closed
joneskoo opened this issue May 23, 2017 · 3 comments
Closed

sync: clarify Cond "created as part of other structures" documentation #20471

joneskoo opened this issue May 23, 2017 · 3 comments
Labels
Documentation FrozenDueToAge
Milestone
Go1.9Maybe

Comments

@joneskoo
Copy link
Contributor

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

go version go1.8.1 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/joneskoo"
GORACE=""
GOROOT="/usr/local/Cellar/go/1.8.1/libexec"
GOTOOLDIR="/usr/local/Cellar/go/1.8.1/libexec/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/p9/8mb6mhcx7p3br8f18crtczzw0000gn/T/go-build980179018=/tmp/go-build -gno-record-gcc-switches -fno-common"
CXX="clang++"
CGO_ENABLED="1"
PKG_CONFIG="pkg-config"
CGO_CFLAGS="-g -O2"
CGO_CPPFLAGS=""
CGO_CXXFLAGS="-g -O2"
CGO_FFLAGS="-g -O2"
CGO_LDFLAGS="-g -O2"

What did you do?

I'm creating a sync.Cond as part of a struct. Documentation explicitly says this is ok.

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

What did you expect to see?

Wait for Cond until Broadcast is called one second later.

What did you see instead?

panic: runtime error: invalid memory address or nil pointer dereference
[signal SIGSEGV: segmentation violation code=0xffffffff addr=0x0 pc=0x82ffb]

goroutine 1 [running]:
sync.(*Cond).Wait(0x104401a0, 0x0)
	/usr/local/go/src/sync/cond.go:56 +0x7b
main.main()
	/tmp/sandbox578695461/main.go:17 +0x120

There isn't an example for sync.Cond in the documentation and it's hardly used in the standard library.

Should the documentation explain that the locker must be set if the Cond is created as part of a structure? If yes, then what does this really mean:

A Cond can be created as part of other structures. A Cond must not be copied after first use.

Please update title if this turns out to be a documentation issue. A simple example and where the Cond might be useful would be nice.
@bradfitz
Copy link
Contributor

You can embed a Cond value in another struct (what the docs are saying?), but the zero value is not valid. You must initialize the Locker.

@bradfitz bradfitz changed the title sync: nil pointer with Cond created as part of struct sync: clarify Cond "created as part of other structures" documentation May 23, 2017
@bradfitz bradfitz added this to the Go1.9Maybe milestone May 23, 2017
@joneskoo
Copy link
Contributor Author

@bradfitz https://go-review.googlesource.com/c/44071/ I've added a one line doc change.

Before we close this, is it possible to add a panic if L is nil with more descriptive error? Nil pointer dereference is panic anyway already. I guess more a philosophical thing if things should check for things if used against doc. Looking at the existing copy checker, I think yes.

Further, I guess an example would be good to include, never too many examples?

@gopherbot
Copy link

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

@joneskoo joneskoo mentioned this issue May 25, 2017
Open
@golang golang locked and limited conversation to collaborators Jun 7, 2018
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Assignees
No one assigned
Labels
Documentation FrozenDueToAge
Projects
None yet
Milestone
Go1.9Maybe
Development

No branches or pull requests
3 participants
@bradfitz @joneskoo @gopherbot