deps: patch @libp2p/tcp to fix network worker shutdown hang - #9825
Merged
Conversation
`sendReset()` calls `socket.resetAndDestroy()` unconditionally, which does not tear down the handle if the writable side has already ended. The socket stays alive as an active `TCPSocketWrap` while libp2p considers it closed, and since nothing else holds a reference it is never closed. The stranded handle keeps the worker spinning in `Environment::CleanupHandles()`, so `Worker.terminate()` never resolves and the process blocks in `uv_thread_join` until the process manager kills it. Applies libp2p/js-libp2p#3597 until it is released. Validated on a mainnet node over 71 shutdowns with no hang, against a baseline rate of 4 in 36. Details in https://gist.github.com/nflaig/5f41cfc50f38baf5046a034162943dc3 Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
![chatgpt-codex-connector[bot]](https://avatars.githubusercontent.com/in/1144995?s=60&v=4){kind=small}
There was a problem hiding this comment.
馃挕 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: ccf007cfb0
鈩癸笍 About Codex in GitHub
Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you
- Open a pull request for review
- Mark a draft as ready
- Comment "@codex review".
If Codex has suggestions, it will comment; otherwise it will react with 馃憤.
Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".
| sigstore: "4.0.0" | ||
|
|
||
| patchedDependencies: | ||
| "@libp2p/tcp@11.0.13": patches/@libp2p__tcp@11.0.13.patch |
There was a problem hiding this comment.
When Lodestar is installed from npm, this workspace-level patchedDependencies entry is not included in the published tarball, so consumers still receive an unpatched @libp2p/tcp. The release workflow publishes with Lerna, while packages/beacon-node/package.json:128 retains "@libp2p/tcp": "^11.0.13" and excludes both this workspace configuration and the patch file. Consequently, npm installations of @chainsafe/lodestar or @lodestar/beacon-node remain vulnerable to the shutdown hang this commit is intended to fix; ensure the published dependency path also carries the fix.
Useful? React with 馃憤聽/ 馃憥.
Member
Author
There was a problem hiding this comment.
pretty sure this comment is wrong @matthewkeil ?
nflaig
commented
Aug 14, 2026
Comment on lines
+23
to
+27
| sendReset (): void { | ||
| + if (this.socket.writableEnded) { | ||
| + this.socket.destroy() | ||
| + return | ||
| + } |
Member
Author
There was a problem hiding this comment.
not sure it's required to patch src but why not 馃し
Contributor
Performance Report馃殌馃殌 Significant benchmark improvement detected
Full benchmark results
|
matthewkeil
approved these changes
Aug 14, 2026
github-project-automation
Bot
moved this from In Progress
to Done
in Lodestar Team Coordination
wemeetagain
added a commit
that referenced
this pull request
Aug 24, 2026
## Summary Bump libp2p and related dependencies to the latest compatible published versions, including the released TCP shutdown fix from libp2p/js-libp2p#3597. ### Updated - `libp2p`: `3.1.6` -> `3.3.9` - `@libp2p/tcp`: `^11.0.13` -> `^11.0.27` - `@libp2p/interface`: `^3.1.0` -> `^3.3.0` - align the remaining direct `@libp2p/*`, reqresp, and datastore dependencies with the compatible release set - remove the temporary `@libp2p/tcp@11.0.13` patch and exact override added in #9825 - refresh `pnpm-lock.yaml` Co-authored-by: Cayman <caymannava@gmail.com> Co-authored-by: bing <spiralladder@fastmail.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
2 participants
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.
Applies libp2p/js-libp2p#3597 as a local
pnpm patchuntil it is released. This fixes the network worker shutdown hang, the underlying handle that #9790 mitigated but did not identify.Root cause
TCPSocketMultiaddrConnection.sendReset()callssocket.resetAndDestroy()unconditionally. When the writable side has already ended that does not tear the handle down, so the socket stays alive as an activeTCPSocketWrapwhile libp2p considers it closed. Nothing else holds a reference, so nothing ever closes it.Node's worker teardown then spins forever, because
Environment::CleanupHandles()runs until every handle is closed:The thread never exits, so
Worker.terminate()never resolves, so the main thread blocks inuv_thread_joinfromprocess.exit()until the process manager kills it.The patch guards the case the reset cannot handle:
Why the exact-version pin
@libp2p/tcpis declared as^11.0.13andpatchedDependencieskeys are version exact, so an 11.0.14 release would resolve past the patch and silently drop the fix. Theoverridespin prevents that, same as the existingsigstorepatch. It does mean no@libp2p/tcpbump until the patch is dropped.Testing
Validated on a mainnet node against a baseline hang rate of 4 in 36 shutdowns (11.1%).
70 consecutive shutdowns, synced with >=200 peers and a 5 minute soak each:
TCPSocketWrappresent afterlibp2p.stop()Plus one shutdown at 9.07h uptime, since orphaned socket counts grew with uptime: terminate 0.180s, no
TCPSocketWrap, exit 0.Counted independently from the raw node logs as well as from the test harness. The last unpatched shutdown on the same machine, three minutes before deploying the patched build, hung at 3.001s with
activeResources=MessagePort=1,TCPSocketWrap=6,Timeout=1.Observing 70 consecutive clean shutdowns if the bug were still present has probability 0.00026. Fisher exact against the measured baseline gives p = 0.012.
The
activeResourceslogging added by #9790 is what made this diagnosable, the presence ofTCPSocketWrapseparated 5 hangs from 32 clean shutdowns perfectly (Fisher p = 0.0000023).Longest uptime tested was 9.07h. The worst case observed, 28 orphaned sockets, took ~38h to accumulate, so this validates the mechanism rather than proving an upper bound.
Full write-up: https://gist.github.com/nflaig/5f41cfc50f38baf5046a034162943dc3
When to remove
Drop the patch and the
overridespin together once js-libp2p#3597 is released and@libp2p/tcpis bumped to a version containing it.AI Assistance Disclosure
Investigation, patch and validation with Claude Code.
馃 Generated with Claude Code