Skip to content

[SharovBot] node/eth: fix data race between MergeLoop and Aggregator.Close - #22245

Closed
erigon-copilot[bot] wants to merge 1 commit into
mainfrom
agent-fix/aggregator-close-race-fix
Closed

erigon-copilot[bot] wants to merge 1 commit into
mainfrom
agent-fix/aggregator-close-race-fix

Conversation

@erigon-copilot

@erigon-copilot erigon-copilot Bot commented Jul 5, 2026

Copy link
Copy Markdown
Contributor

[SharovBot]

Problem

TestImportClosesChaindataOnInitError was triggering a DATA RACE detected by -race:

WARNING: DATA RACE
Write at 0x00c0653acb90 by goroutine 321:
  github.com/erigontech/erigon/db/state.(*Aggregator).Close()
      db/state/aggregator.go:630

Previous read at 0x00c0653acb90 by goroutine 373:
  github.com/erigontech/erigon/db/state.(*Aggregator).MergeLoop()
      db/state/aggregator.go:1228

  github.com/erigontech/erigon/node/eth.New.func16()
      node/eth/backend.go:1133

Root Cause

eth.New creates its own sentryCtx/sentryCancel pair (derived from context.Background(), not from the ctx parameter). Stop() cancels via sentryCancel(), then waits on bgComponentsEg.

However, MergeLoop was launched with the outer ctx parameter — not backend.sentryCtx. So sentryCancel() did not terminate the MergeLoop goroutine. bgComponentsEg.Wait() returned immediately (the goroutine was still running), and then chainDB.Close() → Aggregator.Close() would write Aggregator fields while MergeLoop concurrently read them.

This race surfaces in the test because importChain calls defer ethereum.Stop() and runErigonCommand calls defer cancel() — the outer context is only cancelled after Stop() has already returned.

Fix

Capture backend.sentryCtx into a local variable and pass it to MergeLoop, so Stop() → sentryCancel() reliably cancels the goroutine before bgComponentsEg.Wait() returns.

Verification

go test -race -count=3 -run TestImportClosesChaindataOnInitError ./cmd/utils/app/...
ok  github.com/erigontech/erigon/cmd/utils/app  5.669s

3 consecutive passing runs with -race, no DATA RACE warnings.

CI job: https://github.com/erigontech/erigon/actions/runs/28674562917/job/85045122311

When eth.New is called, it creates its own sentryCtx/sentryCancel pair
(derived from context.Background, not from the ctx parameter).
Stop() cancels sentryCtx via sentryCancel(), then waits on bgComponentsEg.

Previously, MergeLoop was launched with the outer ctx parameter, so
sentryCancel() did NOT cancel the MergeLoop goroutine.  bgComponentsEg.Wait()
would return immediately because MergeLoop was still running (its context
was the outer ctx, not yet cancelled), and then chainDB.Close() →
Aggregator.Close() would write Aggregator fields while MergeLoop still
read them, triggering a DATA RACE in TestImportClosesChaindataOnInitError.

Fix: capture backend.sentryCtx into a local variable and pass it to
MergeLoop, so Stop() → sentryCancel() reliably terminates the goroutine
before bgComponentsEg.Wait() returns.

Co-authored-by: Giulio Rebuffo <giulio.rebuffo@gmail.com>
@erigon-copilot

erigon-copilot Bot commented Jul 5, 2026

Copy link
Copy Markdown
Contributor Author

[SharovBot]

Closing this PR as it's superseded by the already-merged PR #22244 which fixes the same DATA RACE via a complementary approach (tracking MergeLoop in bgComponentsEg). The root cause analysis was correct — MergeLoop was using the outer ctx instead of sentryCtx — but #22244 + #22203 together fully resolve the race with a cleaner approach.

@erigon-copilot erigon-copilot Bot closed this Jul 5, 2026
@erigon-copilot
erigon-copilot Bot deleted the agent-fix/aggregator-close-race-fix branch July 5, 2026 09:58
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.

0 participants