From c09c15c716354862cff98f8b52f9bf63cb5922bf Mon Sep 17 00:00:00 2001
From: David Reiss <david@temporal.io>
Date: Wed, 25 Sep 2024 17:25:41 +0000
Subject: [PATCH] Fix SyncMap.PopAll if SyncMap is copied (#6557)

## 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
---
 common/collection/sync_map.go | 11 +++++++----
 1 file changed, 7 insertions(+), 4 deletions(-)

diff --git a/common/collection/sync_map.go b/common/collection/sync_map.go
index 34f38ada811..0fa1b8aac2e 100644
--- a/common/collection/sync_map.go
+++ b/common/collection/sync_map.go
@@ -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
 }
 
@@ -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
 }