net: handle undefined parent in _unrefTimer and _destroy#64644
Open
haggholm wants to merge 1 commit into
Open
Conversation
Collaborator
|
Review requested:
|
Codecov Report✅ All modified and coverable lines are covered by tests. Additional details and impacted files@@ Coverage Diff @@
## main #64644 +/- ##
==========================================
- Coverage 92.01% 90.14% -1.88%
==========================================
Files 381 741 +360
Lines 170167 242095 +71928
Branches 26075 45552 +19477
==========================================
+ Hits 156585 218239 +61654
- Misses 13291 15364 +2073
- Partials 291 8492 +8201
🚀 New features to boost your workflow:
|
The fix and approach are from nodejs#64491 by Shivay-98; this reopens it to get it landed, since the original stalled awaiting requested changes. `Socket.prototype._unrefTimer` and `Socket.prototype._destroy` both walk the `_parent` chain with a strict `!== null` check. During connection teardown a socket's `_parent` can be left `undefined` (for example a TLS socket layered over another stream), so the loop steps onto `undefined` and reads a property off it, throwing a TypeError: Cannot read properties of undefined (reading 'Symbol(timeout)') from an uncaught I/O callback and crashing the process. Using a nullish (`!= null`) check terminates the walk on both `null` and `undefined`. [petter@hightouch.io: apply the same fix to the identical loop in `_destroy`, which the original regression test already exercised via `destroy()`; add direct unit coverage for both paths.] Fixes: nodejs#64490 Refs: nodejs#64491 Signed-off-by: Petter Häggholm <petter@hightouch.io>
haggholm
force-pushed
the
fix/net-unref-timer-parent-chain
branch
from
July 21, 2026 16:44
6b91d30 to
ae8a326
Compare
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.
The fix and approach are from #64491 by @Shivay-98; opening from a fresh branch to land it, since that PR stalled awaiting the requested changes (unrelated commit dropped, failing test addressed,
Signed-off-byadded).Socket.prototype._unrefTimerandSocket.prototype._destroyboth walk the socket_parentchain with a strict!== nullcheck. When teardown leaves a socket's_parentasundefined(for example a TLS socket layered over another stream), the loop steps ontoundefinedand throwsfrom an uncaught I/O callback, crashing the process. Switching both loops to a nullish
!= nullcheck stops the walk onundefinedas well.The original PR fixed only
_unrefTimer; its regression test also callsdestroy(), which hits the same unpatched loop in_destroy, so that path is fixed and covered here too.Fixes: #64490
Refs: #64491
Checklist
make -j4 test(UNIX) passes