From 9d929fc51a8b0b57705d3f72c21a26c1608f884f Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Ralph=20K=C3=BCpper?= Date: Thu, 11 Jun 2026 20:48:47 +0200 Subject: [PATCH] fix(http): drain perry-ext-net queue from http-server pump so destroyed raw-upgrade socket stops pinning the event loop (#5010) MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit 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. --- crates/perry-ext-http-server/src/server.rs | 14 ++++++++++++++ crates/perry-ext-net/src/dispatch.rs | 14 ++++++++++++-- crates/perry-ext-net/src/lib.rs | 21 +++++++++++++++++++++ crates/perry-ext-net/src/lifecycle.rs | 17 +++++++++++++++++ 4 files changed, 64 insertions(+), 2 deletions(-) diff --git a/crates/perry-ext-http-server/src/server.rs b/crates/perry-ext-http-server/src/server.rs index 2a34aa0012..1140ce2c5e 100644 --- a/crates/perry-ext-http-server/src/server.rs +++ b/crates/perry-ext-http-server/src/server.rs @@ -1521,6 +1521,20 @@ pub extern "C" fn js_node_http_server_process_pending() -> i32 { } count += crate::http2_server::process_pending_h2_events(); + // #5010 — drain perry-ext-net's own pending-event queue. A raw + // `'upgrade'` (#4973) hands the listener a real `net.Socket` adopted into + // perry-ext-net (`adopt_upgraded_tcp_stream`); when user code destroys it, + // the socket task queues a `Close` event in perry-ext-net's queue. For an + // http-only program perry-stdlib runs with its OWN bundled net (so its + // `external-net-pump` arm is OFF and never touches ext-net's queue), and + // the perry-ext-net aux pump proved unreliable across workspace link + // layouts. The http-server pump, by contrast, runs every tick + // (external-http-server-pump) and directly depends on perry-ext-net, so + // draining here — through the UNIQUE `js_ext_net_drain_pending` symbol + // (no stdlib twin) — reliably empties that queue so the destroyed upgrade + // socket stops pinning the event loop. Cheap (one mutex peek) when empty. + count += unsafe { perry_ext_net::js_ext_net_drain_pending() }; + count } diff --git a/crates/perry-ext-net/src/dispatch.rs b/crates/perry-ext-net/src/dispatch.rs index 67b3e2b74b..967f9a3aa5 100644 --- a/crates/perry-ext-net/src/dispatch.rs +++ b/crates/perry-ext-net/src/dispatch.rs @@ -38,7 +38,12 @@ extern "C" { } extern "C" fn process_pending_aux() -> i32 { - unsafe { crate::js_net_process_pending() } + // Drain via the DISTINCT `js_ext_net_drain_pending` symbol — NOT the + // `js_net_process_pending` extern, whose symbol the bundled stdlib net + // twin shadows in a workspace build (that twin drains stdlib's queue, + // leaving ext-net's own adopted-socket events — e.g. the raw-`'upgrade'` + // `Close` — stuck and the loop pinned). #5010. + unsafe { crate::js_ext_net_drain_pending() } } pub(crate) fn ensure_runtime_dispatch_registered() { @@ -186,7 +191,12 @@ unsafe fn socket_method(handle: i64, method: &str, args: &[f64]) -> Option undefined() } "destroy" | "destroySoon" => { - crate::js_net_socket_destroy(handle); + // Drive teardown through the DISTINCT `js_ext_net_destroy_socket` + // symbol — NOT the `js_net_socket_destroy` extern, whose symbol the + // bundled stdlib net twin shadows in a workspace build (it would + // mark the socket destroyed in stdlib's registry, leaving the + // adopted raw-`'upgrade'` socket alive in ext-net's). #5010. + crate::js_ext_net_destroy_socket(handle); undefined() } "on" | "addListener" if args.len() >= 2 => { diff --git a/crates/perry-ext-net/src/lib.rs b/crates/perry-ext-net/src/lib.rs index a833ee2ddf..31a2663e6a 100644 --- a/crates/perry-ext-net/src/lib.rs +++ b/crates/perry-ext-net/src/lib.rs @@ -1499,6 +1499,27 @@ pub unsafe extern "C" fn js_net_socket_upgrade_tls( /// capacity retained → zero steady-state allocation). #[no_mangle] pub unsafe extern "C" fn js_net_process_pending() -> i32 { + js_ext_net_drain_pending() +} + +/// Drain ext-net's own pending-event queue. +/// +/// This carries a DISTINCT `#[no_mangle]` symbol (`js_ext_net_drain_pending`), +/// deliberately NOT the `js_net_process_pending` name that the bundled stdlib +/// net ALSO exports. In a workspace/auto-optimize build both crates are +/// linked, so `js_net_process_pending` is a duplicate symbol; the link binds +/// every reference to whichever twin wins (stdlib's). The aux pump +/// (`process_pending_aux`) and the extern wrapper above therefore call THIS +/// uniquely-named entry point instead — a symbol with no twin and nothing to +/// fold against — so the adopted raw-`'upgrade'` socket's `Close` event in +/// ext-net's own queue is actually drained rather than left to pin the event +/// loop forever. Without this the loop hung, and the behavior flipped with +/// unrelated code-size changes (link-order roulette). (#5010) +/// +/// # Safety +/// Fires user JS closures (listeners); callers must hold a valid runtime. +#[no_mangle] +pub unsafe extern "C" fn js_ext_net_drain_pending() -> i32 { thread_local! { static SCRATCH: std::cell::RefCell> = const { std::cell::RefCell::new(Vec::new()) }; diff --git a/crates/perry-ext-net/src/lifecycle.rs b/crates/perry-ext-net/src/lifecycle.rs index 5af1923202..aca6b39789 100644 --- a/crates/perry-ext-net/src/lifecycle.rs +++ b/crates/perry-ext-net/src/lifecycle.rs @@ -317,6 +317,23 @@ pub unsafe extern "C" fn js_net_socket_end(handle: i64, chunk_bits: i64) { /// `handle` must be a registered socket id (raw, NOT NaN-boxed). #[no_mangle] pub unsafe extern "C" fn js_net_socket_destroy(handle: i64) { + js_ext_net_destroy_socket(handle); +} + +/// Destroy an `ext-net` socket by id, operating directly on this crate's +/// socket registry. +/// +/// This carries a DISTINCT `#[no_mangle]` symbol (`js_ext_net_destroy_socket`), +/// deliberately NOT the `js_net_socket_destroy` name that the bundled stdlib +/// net ALSO exports. In a workspace/auto-optimize build both are linked, so +/// `js_net_socket_destroy` is a duplicate symbol bound to whichever twin +/// wins (stdlib's). The handle-dispatch `socket_method` "destroy" arm and the +/// extern wrapper above call THIS uniquely-named entry point instead — a +/// symbol with no twin — so an adopted raw-`'upgrade'` socket is actually +/// marked destroyed in ext-net's own registry rather than in stdlib's empty +/// one, which is what let the event loop drain. (#5010) +#[no_mangle] +pub extern "C" fn js_ext_net_destroy_socket(handle: i64) { let mut sockets = statics::sockets().lock().unwrap(); if let Some(s) = sockets.get_mut(&handle) { s.destroyed = true;