Skip to content

[SharovBot] fix data race in FilesItem.closeFilesAndRemove#21384

Open
erigon-copilot[bot] wants to merge 1 commit into
mainfrom
erigon-copilot/fix-filesitem-closerace
Open

[SharovBot] fix data race in FilesItem.closeFilesAndRemove#21384
erigon-copilot[bot] wants to merge 1 commit into
mainfrom
erigon-copilot/fix-filesitem-closerace

Conversation

@erigon-copilot
Copy link
Copy Markdown
Contributor

Summary

  • Fix data race in (*FilesItem).closeFilesAndRemove() detected by -race in TestHistoryVerification_SimpleBlocks
  • Root cause: TOCTOU race between deleteMergeFile (sets canDelete=true, checks refcount.Load()==0) and refcntDecrement (does refcount.Add(-1)==0 && canDelete.Load()), allowing both paths to concurrently enter closeFilesAndRemove() on the same FilesItem
  • Fix: add sync.Once field to FilesItem and wrap closeFilesAndRemove() body in Once.Do(), ensuring at-most-once execution regardless of concurrent callers

Test plan

  • go test -race -run TestHistoryVerification_SimpleBlocks -count=5 ./execution/verify/... — all 5 runs pass, no DATA RACE warnings
  • go build ./... passes
  • No test files modified

🤖 Generated with Claude Code

Use sync.Once to ensure closeFilesAndRemove executes at most once per
FilesItem, preventing concurrent close from deleteMergeFile and
refcntDecrement paths racing on the same instance.

Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com>

Co-authored-by: Giulio Rebuffo <giulio.rebuffo@gmail.com>
Copy link
Copy Markdown
Collaborator

@AskAlexSharov AskAlexSharov left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

seems we need better solution for this problem, or revert:
#20462
#20490

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