fix(net): prevent onConnectEnd listener accumulation in TLS sockets

[faytekin]

What does this PR do?

Fixes #12117

When a TLS connection fails before the handshake completes, the onConnectEnd listener never gets removed. So if you're using something like MongoDB with connection pooling, every
reconnect attempt adds another listener and they just pile up forever - that's why people were seeing their heap grow without bound.

The fix is pretty simple: use prependOnceListener instead of prependListener so it cleans itself up. Also added some defensive cleanup in the error/close handlers and _destroy()
just to be safe.

How did you verify your code works?

Added a regression test that hammers a server with failed connections and checks that listeners don't accumulate. The test fails on main (shows 3 listeners when there should be ≤1) and
passes with this fix.

[coderabbitai]

Walkthrough

Replaced multiple TLS and abort event registrations with one-time listeners and added conditional removals of the TLS connect-end handler to ensure it runs at most once and is removed on TLS negotiation, errors, or socket close. Added tests validating listener cleanup and abort behavior.

Changes

Cohort / File(s)	Summary
TLS socket listener changes src/js/node/net.ts	Replaced addListener/prependListener end handlers with prependOnceListener and updated abort signal registrations to { once: true }. Added conditional removals of the onConnectEnd listener on TLS negotiation, error, and close paths to prevent stale callbacks and potential leaks.
Regression tests for listener cleanup test/regression/issue/12117.test.ts	Added three tests: repeated TLS client connections to verify no end-listener leak, a handshake success test ensuring onConnectEnd is removed, and an AbortSignal cleanup test ensuring aborting after socket destruction does not throw.

Suggested reviewers

• Jarred-Sumner
• dylan-conway

🚥 Pre-merge checks | ✅ 4

✅ Passed checks (4 passed)

Check name	Status	Explanation
Title check	✅ Passed	The title accurately summarizes the main change: preventing onConnectEnd listener accumulation in TLS sockets, which is the primary focus of the code changes.
Description check	✅ Passed	The description covers both required sections from the template: it explains what the PR does (fixes listener accumulation issue #12117) and how it was verified (regression test that fails on main and passes with the fix).
Linked Issues check	✅ Passed	The PR directly addresses issue #12117 by fixing the root cause of the memory leak: preventing onConnectEnd listener accumulation on failed TLS handshakes, verified by the regression test.
Out of Scope Changes check	✅ Passed	All changes are within scope: modifications to TLS socket listener handling in net.ts and addition of a regression test directly targeting the reported memory leak issue.

_{✏️ Tip: You can configure your own custom pre-merge checks in the settings.}

---

Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out.

❤️ Share

• X
• Mastodon
• Reddit
• LinkedIn

_{Comment @coderabbitai help to get the list of available commands and usage tips.}

[coderabbitai]

Actionable comments posted: 2

🤖 Fix all issues with AI agents

In `@test/regression/issue/12117.test.ts`:
- Around line 8-35: The test title mentions "TLS socket" but the code uses new
net.Socket(), so it doesn't exercise the TLS-specific onConnectEnd listener
(bunTlsSymbol path) — either change the title to reflect TCP or convert the test
to use TLS: replace the net.Socket usage with tls.connect(...) (import tls and
call tls.connect with host/port) and ensure the server is a TLS-capable server
(or create a proper TLS server fixture) so the bunTlsSymbol TLS listener path is
executed; update references in the test (`#12117` TLS socket reconnection should
not leak onConnectEnd listeners, net.Socket, tls.connect, bunTlsSymbol,
onConnectEnd) accordingly.
- Around line 68-77: The test "test('#12117 AbortSignal listener cleanup')"
currently has no assertions; update it to assert the expected behavior by either
wrapping the test body in a try/catch and failing the test on thrown errors or
by adding explicit assertions around controller.abort() (e.g., expect(() =>
controller.abort()).not.toThrow()) and verifying no memory-leaks/listener
buildup (for example, check that creating/destroying sockets with new
net.Socket({ signal: controller.signal }) does not increase listener count on
controller.signal or that socket.destroy() clears listeners). Ensure references
to AbortController, controller.abort, and the net.Socket creation/destruction
are used to locate and modify the test.

[nrzky]

Someone finally had to step up and take responsibility on this. Congratulations @faytekin :).

[byldrma3]

Bun is fast.
So fast that sockets leave… but their memory doesn’t 😄

Thanks for fixing this — much appreciated @faytekin

[mertasan]

It works for me! Thanks!

[epicentre]

We are experiencing this issue and hope it will be resolved as soon as possible. Thanks.

[Jubstaaa]

Absolute legend! 👑 killed the leak and saved the day. Drinks on me abi! 🍻 @faytekin

[ilhanaydinli]

We actually switched from Bun to Node.js because of this issue. This fix is critical for anyone using connection pooling with MongoDB, as it prevents the memory leak caused by accumulating TLS listeners. Great job, @faytekin!

[cirospaciari]

LGTM running CI thanks!

[cirospaciari]

Looks like we have some failing tests like test/js/node/test/parallel/test-tls-wrap-econnreset-socket.js and at test/js/node/test/parallel/test-tls-wrap-econnreset-socket.js