豆豆友情提示:这是一个非官方 GitHub 代理镜像,主要用于网络测试或访问加速。请勿在此进行登录、注册或处理任何敏感信息。进行这些操作请务必访问官方网站 github.com。 Raw 内容也通过此代理提供。
Skip to content

fix(node/http): stop leaking TCP wrappers on HTTPS upgrade with createConnection TLSSocket#32961

Merged
bartlomieju merged 4 commits intomainfrom
node-tls-mem-leak
Mar 24, 2026
Merged

fix(node/http): stop leaking TCP wrappers on HTTPS upgrade with createConnection TLSSocket#32961
bartlomieju merged 4 commits intomainfrom
node-tls-mem-leak

Conversation

@magurotuna
Copy link
Copy Markdown
Member

Fixes a Node-compat HTTP/TLS bug where https.request() could leak native TCP wrappers when createConnection returns a pre-created TLSSocket and the request upgrades the connection.

This is the same pattern used by npm:ws - create the TLS socket first with tls.connect(), then pass it to https.request({ createConnection }).

This patch:

  • reuses the original encrypted TLSSocket during the upgrade path instead of creating a replacement socket
  • skips the sock-init workaround for sockets that are already encrypted
  • waits for secureConnect before flushing the request on pre-created TLSSockets
  • preserves the TLS-upgrade state needed when the handle is reinitialized

User-visible improvement

With the attached repro, RSS no longer grows linearly with each upgraded connection.

Before this patch:

  • RSS grows from 102 MB to 349 MB over about 4,053 connections in a comparable ~5 minute run

After this patch:

  • RSS grows from 96 MB to 118 MB over about 3,920 connections in a comparable ~5 minute run
  • heap usage stays roughly flat
  • the process reaches a steady state instead of continuing to climb
deno_tls_upgrade_rss_comparison

Script used can be found at https://github.com/magurotuna/tls_then_upgrade/blob/main/client.ts

magurotuna and others added 4 commits March 25, 2026 02:27
The upgrade callback's socket parameter is typed as Duplex, not Socket.
Also flatten the nested promise to avoid a dangling unresolved promise
on test cleanup.

Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com>
The kReinitializeHandle path should only be used when the socket was
provided via createConnection as an encrypted TLS socket. For plain
HTTP upgrades, create a fresh Socket to avoid dangling state that
prevents the close event from firing.

Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com>
@bartlomieju bartlomieju merged commit d736de8 into main Mar 24, 2026
112 checks passed
@bartlomieju bartlomieju deleted the node-tls-mem-leak branch March 24, 2026 21:14
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.

2 participants