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.

Sign up for GitHub

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/atomic: can Value still not be copied after the first use? #32318

Closed
hyper0x opened this issue May 30, 2019 · 4 comments
Closed

sync/atomic: can Value still not be copied after the first use? #32318

hyper0x opened this issue May 30, 2019 · 4 comments
Labels
Documentation Issues describing a change to documentation. FrozenDueToAge NeedsInvestigation Someone must examine and confirm this is a valid issue and not a duplicate of an existing one.

Comments

@hyper0x
Copy link

hyper0x commented May 30, 2019

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

$ go version
go version go1.12.5 darwin/amd64

Does this issue reproduce with the latest release?

Yes.

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

go env Output
$ go env
GOARCH="amd64"
GOBIN="/Users/haolin/Gobin"
GOCACHE="/Users/haolin/Library/Caches/go-build"
GOEXE=""
GOFLAGS=""
GOHOSTARCH="amd64"
GOHOSTOS="darwin"
GOOS="darwin"
GOPATH="/Users/haolin/Golib:/Users/haolin/Golib2:/Users/haolin/Projects/golang:/Users/haolin/Projects/gopcp/gopcp/example.v2:/Users/haolin/GeekTime/Golang_Puzzlers_FULL"
GOPROXY=""
GORACE=""
GOROOT="/usr/local/opt/go/libexec"
GOTMPDIR=""
GOTOOLDIR="/usr/local/opt/go/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/ts/7lg_tl_x2gd_k1lm5g_48c7w0000gn/T/go-build623934671=/tmp/go-build -gno-record-gcc-switches -fno-common"

What did you do?

According to the document of type sync/atomic.Value, A Value must not be copied after first use. But I see in the source code of this type that it is no longer contains the noCopy type and go vet also no longer checks for copy operations of such values.

So, is the values of sync/atomic.Value still not allowed to be copied after the first use??

By the way, I've already looked at this issue: #21504 .

What did you expect to see?

I want consistency about how to type sync/atomic.Value is used.

What did you see instead?

The documentation and source code should be consistent, but they are not.
@mdempsky mdempsky changed the title Is the values of sync/atomic.Value still not allowed to be copied after the first use? sync/atomic: can Value still not be copied after the first use? May 30, 2019
@mdempsky
Copy link
Contributor

mdempsky commented May 30, 2019 •
edited
Loading

Looking at the code, I don't see any reason why Value can't be copied, as long as there aren't any concurrent calls to Store. It seems to me like the comment could be toned down to just warn that copying a Value isn't atomic.

It looks like the warning was added in c81a353.

/cc @valyala @ianlancetaylor

@mdempsky mdempsky added Documentation Issues describing a change to documentation. NeedsInvestigation Someone must examine and confirm this is a valid issue and not a duplicate of an existing one. labels May 30, 2019
@ianlancetaylor
Copy link
Member

The whole point of a Value is to be atomic. I think I would rather keep the comment. It's true that go vet no longer warns about copying, but that's OK. I don't see anything to do here.

@hyper0x
Copy link
Author

hyper0x commented May 31, 2019

I still reserve my opinion. Since the statement in the documentation is useful, a detection (like the previous go vet) or error reporting mechanism (like sync.Cond) should be provided.

@ianlancetaylor
Copy link
Member

I'm fine with a vet check if someone can figure out how to write one without breaking #21504.

@golang golang locked and limited conversation to collaborators May 30, 2020
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Assignees
No one assigned
Labels
Documentation Issues describing a change to documentation. FrozenDueToAge NeedsInvestigation Someone must examine and confirm this is a valid issue and not a duplicate of an existing one.
Projects
None yet
Milestone
No milestone
Development

No branches or pull requests
4 participants
@mdempsky @hyper0x @ianlancetaylor @gopherbot