Skip to content

[SharovBot] Fix data race between buildFiles and recalcVisibleFiles#19590

Merged
Giulio2002 merged 2 commits intomainfrom
fix/data-race-buildfiles-visiblefiles
Mar 4, 2026
Merged

[SharovBot] Fix data race between buildFiles and recalcVisibleFiles#19590
Giulio2002 merged 2 commits intomainfrom
fix/data-race-buildfiles-visiblefiles

Conversation

@Giulio2002
Copy link
Collaborator

[SharovBot] Fix DATA RACE in db/state/aggregator.go

Summary

  • buildFiles() calls BeginFilesRo() on Domain and InvertedIndex without holding visibleFilesLock, racing with recalcVisibleFiles() which writes _visible/_visibleFiles fields under the same lock
  • Wraps the BeginFilesRo() calls in buildFiles() with a.visibleFilesLock.RLock()/RUnlock() to synchronize with the writer, matching the pattern already used in Aggregator.BeginFilesRo()

Test plan

  • go build ./... passes
  • go test -race ./execution/verify/... -run TestHistoryVerification_WithUserTransactions passes 3 consecutive times with no DATA RACE
  • go test -race ./db/state/... passes with no DATA RACE

🤖 Generated with Claude Code

buildFiles() calls BeginFilesRo() on Domain and InvertedIndex without
holding visibleFilesLock, racing with recalcVisibleFiles() which writes
_visible/_visibleFiles under the same lock. Wrap the BeginFilesRo()
calls in buildFiles() with visibleFilesLock.RLock() to synchronize
with the writer, matching the pattern already used in
Aggregator.BeginFilesRo().

Fixes TestHistoryVerification_WithUserTransactions DATA RACE.

Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com>
@Giulio2002 Giulio2002 enabled auto-merge (squash) March 4, 2026 00:58
@Giulio2002 Giulio2002 merged commit 2db7148 into main Mar 4, 2026
24 checks passed
@Giulio2002 Giulio2002 deleted the fix/data-race-buildfiles-visiblefiles branch March 4, 2026 03:16
sudeepdino008 pushed a commit that referenced this pull request Mar 4, 2026
…19590)

**[SharovBot]** Fix DATA RACE in `db/state/aggregator.go`

## Summary
- `buildFiles()` calls `BeginFilesRo()` on `Domain` and `InvertedIndex`
without holding `visibleFilesLock`, racing with `recalcVisibleFiles()`
which writes `_visible`/`_visibleFiles` fields under the same lock
- Wraps the `BeginFilesRo()` calls in `buildFiles()` with
`a.visibleFilesLock.RLock()`/`RUnlock()` to synchronize with the writer,
matching the pattern already used in `Aggregator.BeginFilesRo()`

## Test plan
- [x] `go build ./...` passes
- [x] `go test -race ./execution/verify/... -run
TestHistoryVerification_WithUserTransactions` passes 3 consecutive times
with no DATA RACE
- [x] `go test -race ./db/state/...` passes with no DATA RACE

🤖 Generated with [Claude Code](https://claude.com/claude-code)

---------

Co-authored-by: Claude Opus 4.6 <noreply@anthropic.com>
sudeepdino008 pushed a commit that referenced this pull request Mar 4, 2026
…19590)

**[SharovBot]** Fix DATA RACE in `db/state/aggregator.go`

## Summary
- `buildFiles()` calls `BeginFilesRo()` on `Domain` and `InvertedIndex`
without holding `visibleFilesLock`, racing with `recalcVisibleFiles()`
which writes `_visible`/`_visibleFiles` fields under the same lock
- Wraps the `BeginFilesRo()` calls in `buildFiles()` with
`a.visibleFilesLock.RLock()`/`RUnlock()` to synchronize with the writer,
matching the pattern already used in `Aggregator.BeginFilesRo()`

## Test plan
- [x] `go build ./...` passes
- [x] `go test -race ./execution/verify/... -run
TestHistoryVerification_WithUserTransactions` passes 3 consecutive times
with no DATA RACE
- [x] `go test -race ./db/state/...` passes with no DATA RACE

🤖 Generated with [Claude Code](https://claude.com/claude-code)

---------

Co-authored-by: Claude Opus 4.6 <noreply@anthropic.com>
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.

2 participants