Skip to content

execution/commitment: fix state reader leak across sequential batches - #22460

Merged
yperbasis merged 2 commits into
erigontech:mainfrom
Sahil-4555:fix/state-reader-leak
Jul 15, 2026
Merged

yperbasis merged 2 commits into
erigontech:mainfrom
Sahil-4555:fix/state-reader-leak

Conversation

@Sahil-4555

Copy link
Copy Markdown
Collaborator

Issue

as seen from issue #22118, there's an intermittent assertion failure in the execution pipeline when running block execution in a loop over sequential batches (e.g. in TestFromZero_BranchCacheCoherentAcrossBatches/parallel).

The failure happens when initialising the next batch (Batch N+1). On startup, ExecV3 calls SeekCommitment to restore the trie state. If we are in offline mode (inMemHistoryReads == false) at this stage, historical in-memory reads are not allowed. However, if the previous batch (Batch N) was running in parallel mode, the custom asOfStateReader of the commitment calculator (which uses GetAsOf reads) leaks on the shared commitment context.

This stale state reader leak causes the restore path in the new batch to attempt historical GetAsOf reads, hitting the check in the memory overlay: failed restore state: GetAsOf called on TemporalMemBatch with inMemHistoryReads off

It was timing dependent and flaky, because the leak only manifests when the shared context is reused across batch boundaries under specific goroutine scheduling conditions.

Fix

We fixed this by explicitly setting sdc.stateReader = nil in the ClearRam() method of the SharedDomainsCommitmentContext.

ClearRam() is the designated cleanup routine invoked at the end of each batch execution to flush the in-memory RAM overlays. By hooking the state reader cleanup into ClearRam() we ensure that once we are done with a batch, and close its transaction scope, any stale state reader reference is completely discarded.

When the next batch starts and calls SeekCommitment, the context now has stateReader set to nil. This way, it can properly fall back to the regular LatestStateReader (which does normal, direct database reads) rather than trying to read via the time-travel overlay.

Tests

Since the existing suite already tests this behaviour, no new tests were added:

  • TestExec_RestoresCommitmentStateReader checks that the execution stage restores the commitment state reader cleanly.
  • TestFromZero_BranchCacheCoherentAcrossBatches/parallel tests parallel mode sequential execution batches with a reused SharedDomains overlay, verifying that the state of the commitment is cleanly restored across batch boundaries;

Both tests now pass reliably when the race detector is enabled.

Closes #22118

@AskAlexSharov
AskAlexSharov added this pull request to the merge queue Jul 15, 2026
@github-merge-queue
github-merge-queue Bot removed this pull request from the merge queue due to failed status checks Jul 15, 2026
@AskAlexSharov
AskAlexSharov enabled auto-merge July 15, 2026 12:01
@AskAlexSharov
AskAlexSharov added this pull request to the merge queue Jul 15, 2026
@github-merge-queue
github-merge-queue Bot removed this pull request from the merge queue due to failed status checks Jul 15, 2026
@yperbasis
yperbasis added this pull request to the merge queue Jul 15, 2026
Merged via the queue into erigontech:main with commit ef605f4 Jul 15, 2026
91 checks passed
@Sahil-4555
Sahil-4555 deleted the fix/state-reader-leak branch July 16, 2026 12:00
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.

Flaky: TestFromZero_BranchCacheCoherentAcrossBatches/parallel — GetAsOf on TemporalMemBatch with inMemHistoryReads disabled

3 participants