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

time: is documented as safe for concurrent use, but unmarshal methods do not synchronize #19935

Closed
nicholasmaniscalco opened this issue Apr 11, 2017 · 3 comments

Comments

Projects
None yet
4 participants
@nicholasmaniscalco
Copy link
Contributor

commented Apr 11, 2017

time.Time is documented as being safe for concurrent use, however, some methods modify it without synchronization: GobDecode, UnmarshalBinary, UnmarshalJSON, and UnmarshalText.

It seems like we should update the documentation. What's the right way to document the goroutine-safety guarantee? Something like:

"A Time value can be used by multiple goroutines simultaneously so long as no goroutine calls an unmarshaler method (t.GobDecode, t.UnmarshalBinary, t.UnmarshalJSON, t.UnmarshalText)." ?

@gopherbot

This comment has been minimized.

Copy link

commented Apr 11, 2017

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

@ianlancetaylor

This comment has been minimized.

Copy link
Contributor

commented Apr 11, 2017

Just commented on the CL: The unmarshaler methods by definition modify the receiver. Is this something that confuses people in practice?

@nicholasmaniscalco

This comment has been minimized.

Copy link
Contributor Author

commented Apr 20, 2017

(moving conversation from the CL to the issue)

Probably not. I just came across this and it seemed like the documentation wasn't completely accurate.

Since the docs say "A Time value can be used by multiple goroutines simultaneously" I was expecting the unmarshaler methods to modify the receiver atomically.

As it stands now, there's a potential data race if a Time is shared by two goroutines and one of them calls an unmarshaler method.

It seems unlikely to cause a real problem, but I figured it's worth making the docs more accurate.

@bradfitz bradfitz added this to the Go1.9Maybe milestone May 24, 2017

@gopherbot gopherbot closed this in 4aa5d2e May 24, 2017

@golang golang locked and limited conversation to collaborators May 24, 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.