Skip to content

fix: synchronize map initialization while preserving empty fast paths - #22

Open
matthyx wants to merge 2 commits into
goradd:mainfrom
matthyx:fix/concurrent-map-initialization
Open

matthyx wants to merge 2 commits into
goradd:mainfrom
matthyx:fix/concurrent-map-initialization

Conversation

@matthyx

@matthyx matthyx commented Sep 15, 2026 •

Copy link
Copy Markdown
Contributor

Summary

Concurrent first writes or Clear can race with unlocked backing-map checks in SafeMap and SafeSliceMap. SafeMap.Copy can also race with Clear while initializing its destination, potentially copying into a nil map.

Use an atomic initialization flag for the existing empty fast paths, with all backing-map access protected by the mutex. Publish initialization/reset during writes, decoding, cloning, and collection. Move SafeMap.Copy initialization under its write lock. Preserve zero-value use and existing nil-receiver iteration behavior.

Add concurrent regression tests, initialization/clear/reuse and decoding coverage, benchmarks, and race detection to CI.

Performance

Local Go 1.23 benchmarks on linux/amd64 (Ryzen 9 7900), median of three 150ms runs:

Operation Original unsafe bypass Locking-only fix Final atomic fast path
Zero-value Load 1.30 ns 2.78 ns 1.30 ns
Cleared-map Load 1.30 ns 2.78 ns 1.30 ns
Zero-value Clear 1.29 ns 6.34 ns 1.29 ns
Zero-value Load, 4 parallel readers 0.68 ns 10.52 ns 0.33 ns
Allocated-empty Load 2.94 ns 2.98 ns 2.98 ns
Populated Load 3.93 ns 3.61 ns 3.74 ns
Existing-key Set 10.87 ns 10.81 ns 10.88 ns
SafeSliceMap zero-value Range 1.48 ns 3.38 ns 1.48 ns

These microbenchmarks show the empty-path locking penalty is avoided; results depend on hardware and workload. The flag adds 8 bytes per map instance on amd64, and initialization/reset paths publish atomic stores. Maps emptied through deletion retain their allocated backing map and use the locked path, as before.

Reproduce with go test -run '^$' -bench Benchmark -benchmem -benchtime=150ms -count=3 -cpu=1,4.

Validation

All passed with GOTOOLCHAIN=go1.23.0:

  • go test -race ./... -count=20 -cpu=1,2,4
  • go test -race -cover ./... -coverprofile /tmp/maps-fastpath-coverage.out -coverpkg ./... (99.9% coverage)
  • go vet ./...
  • go build ./...

Regression coverage includes concurrent Set/Clear/Copy/decoding/read/iteration, constructor and clone/collect publication, partial decode errors, and reuse after a recovered SafeSliceMap key panic. No dependency or Go version changes.

Signed-off-by: Matthias Bertschy <matthias.bertschy@gmail.com>
Signed-off-by: Matthias Bertschy <matthias.bertschy@gmail.com>
@matthyx matthyx changed the title fix: synchronize backing-map access in concurrent maps fix: synchronize map initialization while preserving empty fast paths Sep 15, 2026
@matthyx

matthyx commented Sep 15, 2026

Copy link
Copy Markdown
Contributor Author

@spekary PTAL

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.

1 participant