Skip to content

Make a copy of map items during iteration so it's threadsafe - #3634

Merged
peterebden merged 3 commits into
thought-machine:masterfrom
peterebden:peter/defensive-copy
Oct 15, 2026
Merged

peterebden merged 3 commits into
thought-machine:masterfrom
peterebden:peter/defensive-copy

Conversation

@peterebden

Copy link
Copy Markdown
Collaborator

Save some worries about potential deadlocks if these functions don't have that limitation any more. We don't call these functions on a hot path so allocating one slice per shard shouldn't cost much.

Comment thread src/cmap/cmap.go
}

// Items returns a copy of all the items currently in the shard (not including those that are not complete yet).
func (s *shard[K, V]) Items() []item[K, V] {

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Suggested change
func (s *shard[K, V]) Items() []item[K, V] {
func (s *shard[K, V]) items() []item[K, V] {

nit, mostly based on the "exported methods/functions should not return unexported types". Sort of doesn't apply because the method is itself on an unexported type anyway, but...

Copy link
Copy Markdown
Collaborator Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Yeah fair - I kind of prefer it this way though as an (unenforced of course) indicator that other things in the package are ok to call this (vs. mucking with shard members directly).

Point taken on the unexported type, but of course receivers in this package can name it which is the usual objection.

@peterebden
peterebden merged commit b308a61 into thought-machine:master Oct 15, 2026
16 checks passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants