Skip to content

storage: filesystem, fix concurrent access to index map#2139

Open
AriehSchneier wants to merge 1 commit into
go-git:mainfrom
AriehSchneier:fix-index-map-races
Open

storage: filesystem, fix concurrent access to index map#2139
AriehSchneier wants to merge 1 commit into
go-git:mainfrom
AriehSchneier:fix-index-map-races

Conversation

@AriehSchneier

Copy link
Copy Markdown
Contributor

Fix race conditions in ObjectStorage where the s.index map was accessed without proper mutex protection in multiple locations.

This PR extracts and focuses on just the index map race fixes from PR #2094. The other fixes in that PR have been superseded by:

Changes

storage/filesystem/object.go:

  • Document muI and muP mutex fields to clarify what they protect
  • Reindex(): Add mutex protection around s.index = nil
  • PackfileWriter(): Add mutex protection in Notify callback when writing to s.index[h]
  • EncodedObject(): Add read lock when checking if s.index != nil
  • HashesWithPrefix(): Copy index values to slice while holding lock before iteration
  • buildPackfileIters(): Add read lock when accessing s.index[h] in closure

storage/filesystem/object_race_test.go (new):

  • Add TestConcurrentIndexAccess to reproduce the index map race conditions
  • Test simulates concurrent readers (HashesWithPrefix, EncodedObject) and writer (Reindex)
  • Run with go test -race -run TestConcurrentIndexAccess ./storage/filesystem/

Note on Test Output

The test may still report race conditions from external dependencies (e.g., memfs package). This is expected - those are separate issues outside the scope of this PR. The s.index races that this PR fixes are now resolved.

Testing

# Run the race test
go test -race -run TestConcurrentIndexAccess ./storage/filesystem/

# Run all filesystem tests with race detector
go test -race ./storage/filesystem/... -timeout 5m

Supersedes #2094

Fix race conditions in ObjectStorage where s.index map was accessed
without proper mutex protection in multiple locations, leading to
concurrent read/write races.

Changes:
- Document muI and muP mutex fields
- Reindex(): Add mutex protection around s.index = nil
- PackfileWriter(): Add mutex protection in Notify callback when
  writing to s.index[h]
- EncodedObject(): Add read lock when checking if s.index != nil
- HashesWithPrefix(): Copy index values to slice while holding lock
  before iteration to avoid races
- buildPackfileIters(): Add read lock when accessing s.index[h]
  in closure

Add TestConcurrentIndexAccess to reproduce the race condition. The test
fails without mutex fixes and passes with them. Run with -race to detect
data races.

Assisted-by: Claude Sonnet 4.5 <[email protected]>
Signed-off-by: Arieh Schneier <[email protected]>
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