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

container/list should state that it is not thread-safe #25105

Closed
RobMeades opened this issue Apr 26, 2018 · 4 comments
Closed

container/list should state that it is not thread-safe #25105

RobMeades opened this issue Apr 26, 2018 · 4 comments

Comments

@RobMeades
Copy link

Please answer these questions before submitting your issue. Thanks!

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

go1.6.2

Does this issue reproduce with the latest release?

Yes

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

linux/amd64

What did you do?

Accessed a container/list from two different go func() routines.

What were you tripped-up by

A panic occurred in list.PushBack() due to a nil pointer reference. This was because the other (timed) go func() routine was iterating through the list, processing entries, and then calling list.Remove() on the processed items. It seems as though the threading in Golang is so good that the timed go func() is acting upon an item while it is in the middle of being added to the list with list.PushBack().

What would you like to happen as a result?

It would be a good idea to add to the top of the container/list documentation page that it is NOT thread-safe, just so that no-one else falls over this. It took a few million attempts for me to hit it; an hour or two with loops iterating every 20 ms: it might be a very intermittent fatal bug for others, so worth a warning.

@randall77
Copy link
Contributor

This is true of most of the standard library. Even built-in maps are not thread safe. I'm not sure we want to pepper every API with such a warning.
Did you run with the race detector? It should have caught this bug immediately.

@bcmills
Copy link
Contributor

bcmills commented Apr 26, 2018

I agree with @randall77. Per Effective Go:

Go encourages a different approach in which shared values are passed around on channels and, in fact, never actively shared by separate threads of execution. Only one goroutine has access to the value at any given time.

Package-level functions are generally safe for concurrent use, but methods on individual values are not. We document the exceptions to that rule, not the cases that comply with it.

@bcmills bcmills closed this as completed Apr 26, 2018
@bcmills
Copy link
Contributor

bcmills commented Apr 26, 2018

OTOH, we certainly should mention the race detector in Effective Go. It seems we currently do not (#25107).

@RobMeades
Copy link
Author

Fair enough: I do make use of channels etc. but in this case I'm synchronising a semi-synchronous stream with a synchronous one, filling in any gaps by re-inserting nearby data, and so a pair of lists with a timed function in the middle seems like the obvious answer. Just need to remember that, despite its modernity, linked lists in Golang are as "exciting" as ever.

@golang golang locked and limited conversation to collaborators Apr 26, 2019
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Projects
None yet
Development

No branches or pull requests

4 participants