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

sync: clarify if it's valid to call Map methods inside the Map.Range callback #46399

Closed
mvdan opened this issue May 26, 2021 · 4 comments
Closed
Labels
Documentation FrozenDueToAge help wanted NeedsFix The path to resolution is known, but the work has not been done.
Milestone

Comments

@mvdan
Copy link
Member

mvdan commented May 26, 2021

Map is documented as safe for concurrent use, so I understand it is safe to call a method like LoadAndDelete while Range is running in another goroutine, for example.

However, what isn't clear is whether I can call a method like LoadAndDelete in Range's callback. Technically it's not a concurrent call, as it's happening on the same goroutine - it's just a nested call, as it happens within Range.

Is that supported? From my reading of the code it seems like it shouldn't cause a data race. I wonder if it could lead to undesirable effects however, such as if I delete a key that I haven't ranged over yet, or if I keep adding more keys and potentially make the range run for a long time.

I don't know what the answer is or what it should be, but I think the docs could be a bit clearer.

cc @rsc @ianlancetaylor @dvyukov @aclements as per https://dev.golang.org/owners

@mvdan mvdan added Documentation NeedsDecision Feedback is required from experts, contributors, and/or the community before a change can be made. labels May 26, 2021
@ianlancetaylor
Copy link
Contributor

CC @bcmills

@bcmills
Copy link
Contributor

bcmills commented May 26, 2021

Yes, nested calls are supported as of CL 37342, which was back when sync.Map was still golang.org/x/syncmap.Map.

It would be fine to update the documentation to be more explicit about the fact that calls during Range (including reentrant calls to Range itself) are allowed.

@bcmills bcmills added help wanted NeedsFix The path to resolution is known, but the work has not been done. labels May 26, 2021
@bcmills bcmills added this to the Backlog milestone May 26, 2021
@gopherbot gopherbot removed the NeedsDecision Feedback is required from experts, contributors, and/or the community before a change can be made. label May 26, 2021
@bcmills
Copy link
Contributor

bcmills commented May 26, 2021

(The guarantees of Range are intended to parallel the guarantees for ordinary maps, but with the additional property that concurrent calls from other goroutines are allowed; see also #35239 and #46097.)

@gopherbot
Copy link

Change https://golang.org/cl/337389 mentions this issue: sync: clarify the validity to call Map methods inside Range

@bcmills bcmills modified the milestones: Backlog, Go1.18 Nov 10, 2021
@golang golang locked and limited conversation to collaborators Nov 11, 2022
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
Documentation FrozenDueToAge help wanted NeedsFix The path to resolution is known, but the work has not been done.
Projects
None yet
Development

No branches or pull requests

4 participants