sync: fix deadlock when a Map method is called from Map.Range

[fxamacker]

Fixes #5528

Currently, calling any sync.Map method from sync.Map.Range callback f deadlocks. Moreover, Go's sync.Map explicitly permits the Range callback to call other methods on the map ("Range does not block other methods on the receiver; even f itself may call any method on m").

This PR prevents the deadlock by changing sync.Map.Range to:

• take a snapshot of the map keys under the lock
• release the lock
• iterate over the snapshot

A snapshot satisfies Go's sync.Map.Range contract, which only requires that no key is visited more than once and may reflect any mapping from any point during the call.

Using a snapshot keeps the implementation simple with minimal changes to existing code, in line with this file's stated scope ("no more efficient than a map with a lock").

Deadlock Details

src/sync/map.go is a plain map guarded by a single non-reentrant task.PMutex, and every method holds that lock for its entire duration.

Range likewise holds the lock across the user callback, so any sync.Map method called from inside the callback deadlocks.

Added Tests

Added TestMapRangeAndDelete regression test, which deletes map entries from inside the map's Range callback.

Tradeoffs

The snapshot approach prioritized simplicity, predictability, and minimal changes to existing code over performance.

See comment about copying entire map vs copying just the keys.

PR Updates

• Updated to manually copy entries instead of using maps.Clone. Initially, this PR used maps.Clone, but importing maps from sync triggers an import cycle not allowed error on baremetal targets (e.g. -target=microbit) and breaks make smoketest.

• Updated to copy only the keys as suggested by @dgryski. More details in my comment about copying entire map vs copying just the keys.

[dgryski]

Do we need to clone the entire map or can we get away with just getting a copy of all the keys and ranging over those and looking up values in the original map?

[fxamacker]

Do we need to clone the entire map or can we get away with just getting a copy of all the keys and ranging over those and looking up values in the original map?

@dgryski yes, we can snapshot just the keys under the lock and then Load each value from the m. However, it has different tradeoffs.

Pros:

• Less memory (snapshot only keys, instead of keys + values)

Cons:

• Instead of 1 lock acquisition, N+1 lock acquisitions (1 for the keys, then one per Load). Many more lock operations, but shorter individual holds.
• Not a consistent point-in-time view of the map: values can be changed and keys can be deleted between snapshot of the key and loading the value.

Both approaches satisfy Go's sync.Map.Range contract. As noted in the Tradeoffs section, I went with maps.Clone to prioritize simplicity, predictability, and minimal changes to existing code.

Happy to switch to the key-snapshot approach if you prefer. Please let me know.

[dgryski]

I still think the lower memory usage from just copying the keys is worth it, on the assumption that uncontested locks are still pretty fast but memory pressure is expensive.

[dgryski]

LGTM. Thanks for iterating on this!