fix(pxe): stop block synchronizer on PXE shutdown#22604
Open
benesjan wants to merge 1 commit intomerge-train/fairiesfrom
Open
fix(pxe): stop block synchronizer on PXE shutdown#22604benesjan wants to merge 1 commit intomerge-train/fairiesfrom
benesjan wants to merge 1 commit intomerge-train/fairiesfrom
Conversation
`PXE.stop()` did not stop the `BlockSynchronizer`, which could lead to a race where an in-progress sync accesses the DB after it's been closed. Add a `stop()` method to `BlockSynchronizer` that waits for any ongoing sync and stops the block stream, and call it from `PXE.stop()` before closing the DB. Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com>
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Summary
Fixes https://linear.app/aztec-labs/issue/F-339
The issue reported three resource leaks in
PXE.stop():L2BlockStream— activeRunningPromisewith timers: The issue assumed the block stream runs in polling mode (start()), but in the PXE contextblockStream.start()is never called — the stream is only used in manual sync mode viablockStream.sync(). So there are no active timers orInterruptibleSleephandles keeping the event loop alive. That said, callingblockStream.stop()is still good defensive practice in case the usage mode changes.LMDB store — open file descriptors:
PXE.stop()already callsthis.db.close(), so this part was already fixed at some point after the issue was filed.HTTP keep-alive sockets from
createAztecNodeClient: Node's built-infetch(undici) uses HTTP keep-alive by default — after a request completes, the underlying TCP socket stays open in case another request comes to the same host. These idle sockets register as active handles on the event loop, preventingprocess.exit()from happening naturally. Fixing this would require either using a custom undiciAgent/Poolwith aclose()method, or settingkeepalive: false(which hurts performance). In practice this doesn't matter: the sockets time out on their own after a few seconds, and for long-running services (the main use case) the process stays up anyway. Only relevant if you need the process to exit immediately afterstop()without callingprocess.exit(0).What this PR actually fixes:
BlockSynchronizerhad nostop()method at all. IfPXE.stop()were called while a sync was in progress, the ongoing sync could race againstdb.close(). While the job queue draining makes this unlikely in practice, adding an explicitstop()is the right defensive pattern and matches whatServerWorldStateSynchronizeralready does.Changes
stop()method toBlockSynchronizerthat awaits any in-progress sync, then stops the block streamblockStateSynchronizer.stop()inPXE.stop()between draining the job queue and closing the DB