Skip to content

fix(http): raw 'upgrade' + tls.TLSSocket + destroy no longer hangs the event loop (#5010)#5018

Merged
proggeramlug merged 1 commit into
mainfrom
fix/upgrade-tls-destroy-5010
Jun 11, 2026
Merged

fix(http): raw 'upgrade' + tls.TLSSocket + destroy no longer hangs the event loop (#5010)#5018
proggeramlug merged 1 commit into
mainfrom
fix/upgrade-tls-destroy-5010

Conversation

@proggeramlug

Copy link
Copy Markdown
Contributor

Fixes #5010.

Summary

test-http-upgrade-reconsume-stream regressed to a hang (exit 124 instead of 0) after #5001/#4973. The repro:

const tls = require('tls'); const http = require('http');
const server = http.createServer(common.mustNotCall());
server.on('upgrade', common.mustCall((request, socket, head) => {
  new tls.TLSSocket(socket);   // re-own the raw upgrade socket
  server.close();
  socket.destroy();
}));
server.listen(0, common.mustCall(() => {
  http.get({ port: server.address().port,
             headers: { Connection: 'Upgrade', Upgrade: 'websocket' } }).on('error', () => {});
}));

#4973 made a raw 'upgrade' hand the listener a real net.Socket adopted into perry-ext-net (adopt_upgraded_tcp_stream). Destroying it left a live handle pinning the loop. (The new tls.TLSSocket(socket) wrap named in the title is a red herring — the hang reproduces without it.)

Root cause — perry-stdlib/perry-ext-net duplicate-symbol ("twin") hazards

Both crates export the same #[no_mangle] net symbols; in a workspace/auto-optimize link the bundled-stdlib twin wins, and two things broke for the adopted socket:

  1. socket.destroy() dynamic-dispatched through js_net_socket_destroy → bound to stdlib's twin → marked the socket destroyed in stdlib's empty registry while it stayed destroyed=false (alive) in ext-net's.
  2. The socket's Close event sat in ext-net's own pending-event queue, which nothing reliably drained. For an http-only program perry-stdlib runs its own bundled net, so its external-net-pump arm never touches ext-net's queue, and the ext-net aux pump proved unreliable across link layouts — the behavior even flipped with unrelated code-size changes (link-order roulette).

Fix

  • Give the destroy + queue-drain entry points distinct, twin-free #[no_mangle] symbols: js_ext_net_destroy_socket / js_ext_net_drain_pending (the original externs now delegate to them).
  • Route the handle-dispatch destroy arm and the aux pump through those unique symbols.
  • Drain ext-net's queue from the http-server pump (js_node_http_server_process_pending) — it runs every tick via external-http-server-pump and directly depends on perry-ext-net, so the call is a plain crate-path call to a symbol with no twin.

Verification

  • Repro: exit 0, 10/10 runs and 8/8 on an independent rebuild (previously 0/8 — deterministically hung).
  • No regression: normal http request/response exits 0; normal net echo data flow is unchanged (actually more reliable than pristine in spot checks). The pre-existing net-program close-hang is unrelated and out of scope.
  • cargo test -p perry-ext-net: 8 passed.

Code-only — version bump + changelog left for the maintainer to fold at merge.

…ed raw-upgrade socket stops pinning the event loop (#5010)

test-http-upgrade-reconsume-stream regressed to a hang after #5001/#4973:
a raw HTTP 'upgrade' now hands the listener a real net.Socket adopted into
perry-ext-net, and `new tls.TLSSocket(socket); server.close(); socket.destroy()`
left a live handle pinning the loop (exit 124 instead of 0).

Two perry-stdlib/perry-ext-net duplicate-symbol ('twin') hazards were at play:
- socket.destroy() dynamic-dispatched through js_net_socket_destroy, whose
  symbol the bundled stdlib net shadows, so the adopted socket was marked
  destroyed in stdlib's empty registry and stayed alive in ext-net's.
- the adopted socket's Close event sat in ext-net's own pending-event queue,
  which the unreliable aux pump / shadowed js_net_process_pending never drained.
  For an http-only program perry-stdlib runs its OWN bundled net, so its
  external-net-pump arm never touches ext-net's queue. The behavior even
  flipped with unrelated code-size changes (link-order roulette).

Fix: give the destroy + queue-drain entry points DISTINCT, twin-free
#[no_mangle] symbols (js_ext_net_destroy_socket / js_ext_net_drain_pending),
route the handle-dispatch + aux pump through them, and drain ext-net's queue
from the http-server pump (js_node_http_server_process_pending) — which runs
every tick via external-http-server-pump and directly depends on perry-ext-net.

Deterministic now: repro exits 0 across repeated runs and independent rebuilds.
@proggeramlug proggeramlug merged commit 3304cb5 into main Jun 11, 2026
11 of 13 checks passed
@proggeramlug proggeramlug deleted the fix/upgrade-tls-destroy-5010 branch June 11, 2026 19:40
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.

regression: http 'upgrade' + new tls.TLSSocket(socket) + destroy hangs the event loop (#5001 fallout)

1 participant