Skip to content

Commit

Permalink
Fix SyncMap.PopAll if SyncMap is copied (#6557)
Browse files Browse the repository at this point in the history
## What changed?
Change SyncMap.PopAll to clone+clear the contained map instead of
creating a new one.

## Why?
Swapping the map makes SyncMap not copyable, and it's intended to be
copyable (like a regular map reference).

## How did you test it?
existing test
  • Loading branch information
dnr authored Sep 25, 2024
1 parent f2d730a commit c09c15c
Showing 1 changed file with 7 additions and 4 deletions.
11 changes: 7 additions & 4 deletions common/collection/sync_map.go
Original file line number Diff line number Diff line change
Expand Up @@ -25,13 +25,16 @@
package collection

import (
"maps"
"sync"
)

// SyncMap implements a simple mutex-wrapped map. We've had bugs where we took the wrong lock
// when reimplementing this pattern, so it's worth having a single canonical implementation.
// SyncMap implements a simple mutex-wrapped map. SyncMap is copyable like a normal map[K]V.
type SyncMap[K comparable, V any] struct {
// Use a pointer to RWMutex instead of embedding so that the contents of this struct itself
// are immutable and copyable, and copies refer to the same RWMutex and map.
*sync.RWMutex
// For the same reason, contents (the pointer) should not be changed.
contents map[K]V
}

Expand Down Expand Up @@ -92,7 +95,7 @@ func (m *SyncMap[K, V]) Pop(key K) (value V, ok bool) {
func (m *SyncMap[K, V]) PopAll() map[K]V {
m.Lock()
defer m.Unlock()
contents := m.contents
m.contents = make(map[K]V)
contents := maps.Clone(m.contents)
clear(m.contents)
return contents
}

0 comments on commit c09c15c

Please sign in to comment.