From b8a23036613595031180201caf99f7c0482cf8e6 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Ralph=20K=C3=BCpper?= Date: Wed, 12 Aug 2026 23:53:42 +0200 Subject: [PATCH 1/7] fix(link): refuse a link whose ext wrappers bundle a different tokio (#7629) Six gap tests SIGABRTed with "there is no reactor running" because `libperry_ext_http.a` / `libperry_ext_net.a` bundled a different tokio compilation than `libperry_stdlib.a`. tokio's runtime context is a `thread_local!` mangled with the compiling crate instance's hash, so two compilations are two contexts: perry-stdlib's runtime enters one, the wrapper reads the other, and under panic=abort the process dies at its first socket. #507's auto-optimize rebuild already prevents this by folding every tokio-using wrapper into the same cargo invocation as perry-stdlib-static, but nothing checked the invariant, so every path that bypassed that rebuild produced a binary that linked cleanly and aborted. - new `compile/shared_tokio.rs` reads each archive's tokio compilation id out of its `ar` member names (parsed in-process, no llvm-ar dependency) and refuses a mismatched pair, naming both ids and the command that fixes it - `optimized_libs/no_auto.rs` stops building a tokio-using wrapper alone under PERRY_NO_AUTO_OPTIMIZE, which is what manufactured the split - run_parity_tests.sh folds `-p perry-ext-net` into the main build (it was a second invocation), verifies the ext archives under PERRY_SKIP_BUILD=1, and forces the well-known set so the all-pumps stdlib actually links Also rewrites the `[gc-pin-latch]` FATAL, which asserted a cause its own tool refutes (#7990): it now prints a header-coherence verdict and ranks candidates by that evidence, with the pin-site scan last. --- changelog.d/7629-shared-tokio-unification.md | 114 ++++ crates/perry-runtime/src/gc/copying.rs | 22 +- crates/perry-runtime/src/gc/pin.rs | 238 +++++++++ crates/perry/src/commands/compile.rs | 1 + .../compile/optimized_libs/no_auto.rs | 36 ++ .../src/commands/compile/run_pipeline.rs | 29 + .../src/commands/compile/shared_tokio.rs | 498 ++++++++++++++++++ gc-handoff/REACTOR-NOTES.md | 188 +++++++ run_parity_tests.sh | 79 ++- 9 files changed, 1187 insertions(+), 18 deletions(-) create mode 100644 changelog.d/7629-shared-tokio-unification.md create mode 100644 crates/perry/src/commands/compile/shared_tokio.rs create mode 100644 gc-handoff/REACTOR-NOTES.md diff --git a/changelog.d/7629-shared-tokio-unification.md b/changelog.d/7629-shared-tokio-unification.md new file mode 100644 index 0000000000..04aba968a5 --- /dev/null +++ b/changelog.d/7629-shared-tokio-unification.md @@ -0,0 +1,114 @@ +### Fixed + +**Six gap tests aborted with "there is no reactor running" because two tokio compilations reached one binary (#7629).** + +`test_gap_fetch_request_from_node_incoming_message`, +`test_gap_http_client_no_redirect_follow`, `test_gap_http_overloads_3226plus`, +`test_gap_http_req_async_iterator`, +`test_gap_http_res_socket_writable_onfinished` and +`test_gap_net_connect_bound_value` died with SIGABRT (exit 134) — classified +CRASH, not FAIL — after a Rust panic on a worker thread: + +``` +thread '' panicked at crates/perry-ext-http/src/server/server.rs:911:13: +there is no reactor running, must be called from the context of a Tokio 1.x runtime +``` + +Five panicked at perry-ext-http's `tokio::spawn`; `net_connect_bound_value` +panicked one frame lower, inside tokio's own `net/tcp/listener.rs` (from +perry-ext-net's `TcpListener::bind` → `PollEvented::new` → `Handle::current()`). +**Same cause, one fix** — the differing frame is only where each wrapper first +touched the reactor. + +**Root cause.** `perry-ext-*` wrappers are `staticlib`s, so each bundles its own +copy of tokio; `libperry_stdlib.a` bundles one too, and perry-stdlib owns the +process's only runtime. tokio's `runtime::context::CONTEXT` is a +`thread_local!`, so its symbol carries the compiling crate instance's metadata +hash: two tokio compilations in one binary are two independent contexts. +perry-stdlib's runtime enters one, the wrapper reads the other, finds it empty, +and (under `panic = "abort"`) the process dies. `optimized_libs/driver.rs` +already documented this exact failure as #507 and prevents it on the +auto-optimize path by rebuilding every tokio-using wrapper **in the same cargo +invocation** as perry-stdlib-static — but nothing checked that the invariant +held, so every path that bypassed that rebuild produced a binary that linked +cleanly and aborted at its first socket. + +Measured on `55fd197d5`, reading the compilation id straight out of the archive +member names (`ar t … | grep -o 'tokio-[0-9a-f]*'`): + +| build | `libperry_stdlib.a` | `libperry_ext_http.a` | `libperry_ext_net.a` | result | +|---|---|---|---|---| +| auto-optimize | `tokio-692c8788…` | `tokio-692c8788…` | — | PASS 3/3 | +| `PERRY_NO_AUTO_OPTIMIZE=1` | `tokio-5aeb6213…` | `tokio-01c4c58f…` | `tokio-59c9ffcf…` | exit 134, 3/3 | +| one invocation, all packages | `tokio-5aeb6213…` | `tokio-5aeb6213…` | `tokio-5aeb6213…` | PASS 3/3 | + +Three different tokios in the middle row, one per `cargo build -p `. + +**What changed.** + +* New `crates/perry/src/commands/compile/shared_tokio.rs` — reads each + archive's tokio compilation id and refuses a link whose wrapper archives + disagree with the stdlib archive, naming both ids and the single `cargo + build` that fixes it. The `ar` container is parsed in-process rather than + through `llvm-ar`, so the check cannot silently stop gating when a tool is + missing; only wrappers where `binding_needs_shared_tokio` holds are checked, + which is the same predicate the #507 rebuild uses, so check and fix cannot + drift. The report records what it compared, so "compared nothing" is + distinguishable from "found no mismatch". +* `optimized_libs/no_auto.rs` no longer builds a tokio-using wrapper on its own + under `PERRY_NO_AUTO_OPTIMIZE` (`cargo build -p perry-ext-http`, which is + what manufactured the split). It cannot repair the situation either — + building the wrapper *with* `perry-stdlib-static` would overwrite the + prebuilt stdlib with this invocation's feature set and drop the + `external-*-pump` features, trading an abort for a hang — so it says what to + run instead. +* `run_parity_tests.sh`: `-p perry-ext-net` moves into `BUILD_PACKAGES` + (it was a *second* `cargo build -p perry-ext-net -j1`, i.e. the same split); + `PERRY_SKIP_BUILD=1` now verifies the required ext archives are present in + `PERRY_RUNTIME_DIR` before running anything, with the exact command; and the + no-auto gap path exports `PERRY_FORCE_WELL_KNOWN=events,http,net,ws,zlib`. + +That last one is a second, independent defect the first fix uncovered: the +`external-*-pump` features are a property of the ONE prebuilt stdlib while +archive selection is per-import, so a stdlib built with `external-zlib-pump` +failed to link any test that did not import `node:zlib` (five undefined +`_js_ext_zlib_*`). The auto-optimize path never hits it because it enables a +pump only when it is also routing that module. + +**Why CI stayed green.** The 8 gap shards run on `ubuntu-latest` and build +every archive in one `cargo build`, so the invariant held there by accident. +The failure was reachable only from a build configuration CI does not use — +which is how a red gap suite and a green required check coexisted for weeks. + +### Changed + +**The `[gc-pin-latch]` FATAL no longer asserts a cause its own tool refutes (#7990).** + +#7645's abort text said "some site sets `GC_FLAG_PINNED` without going through +`gc::pin_object`" and told the reader to run `scripts/gc_pin_sites.py`. On the +tree where the abort was next observed that tool reports **OK**, and both of its +allowlisted exceptions are test-only, so the stated cause was refuted by the +stated remedy. + +The message now decodes the flags byte by name, prints a **header-coherence +verdict** computed at the instant of the abort, explains that `GC_FLAG_TENURED` +on a nursery-resident object is ordinary (the non-moving generational path +tenures in place), and lists five candidates in the order the evidence +separates them — with the pin-site scan last and a note saying why it used to +lead. + +The verdict is load-bearing rather than decorative. `GC_FLAG_INTERNED` is +written in exactly one file (`string/intern.rs`) and only on +`GC_TYPE_STRING`, so #7990's reported header (`obj_type=8` = Map, `flags=0x37` +including `INTERNED`) is **not a coherent Map**: it reads as memory that once +held an interned string, which points at the #7154 unrooted-slot class rather +than at pin bookkeeping, and explains the ~1-in-16 rate (an unrooted register +only goes bad when a collection lands in its window). The message now says so +and points at `PERRY_GC_ZEAL=1 PERRY_GC_PROTECT_FROMSPACE=1 +PERRY_GC_PROTECT_FROMSPACE_DEPTH=800`. + +Unit tests in `gc/pin.rs` cover the verdict in both directions, including a +case built from #7990's exact header bytes. The underlying fault is not fixed — +this makes the abort point at the right investigation. No CI gate was added: at +~6% of runs it would go red on a healthy tree often enough to be ignored, the +same reasoning that declined to gate #7803's 19%. diff --git a/crates/perry-runtime/src/gc/copying.rs b/crates/perry-runtime/src/gc/copying.rs index b9a708ac52..7d113be3cc 100644 --- a/crates/perry-runtime/src/gc/copying.rs +++ b/crates/perry-runtime/src/gc/copying.rs @@ -1043,17 +1043,19 @@ pub(super) fn scan_remembered_dirty_slots_copying( #[cold] #[inline(never)] unsafe fn pinned_young_move_under_skipped_preflight(header: *mut GcHeader) -> ! { + // #7990: the report is built in `gc/pin.rs` from the header's own flags, + // because those flags are the only evidence that distinguishes an + // incomplete pin latch from a dangling pointer into recycled memory — and + // this message used to assert the former as fact while `gc_pin_sites.py`, + // the tool it told the reader to run, answered OK. eprintln!( - "[gc-pin-latch] FATAL: copying minor is about to relocate a PINNED young \ - object on a preflight-skipped cycle. header={:#x} obj_type={} size={} \ - flags={:#04x}\n\ - The young-pin latch (gc/pin.rs) is incomplete: some site sets \ - GC_FLAG_PINNED without going through gc::pin_object. Find it with \ - `python3 scripts/gc_pin_sites.py` and route it through pin_object (#7645).", - header as usize, - (*header).obj_type, - (*header).size, - (*header).gc_flags, + "{}", + super::pin::pinned_young_move_report( + header as usize, + (*header).obj_type, + (*header).size, + (*header).gc_flags, + ) ); std::process::abort() } diff --git a/crates/perry-runtime/src/gc/pin.rs b/crates/perry-runtime/src/gc/pin.rs index 5f3f7146ee..84750199d2 100644 --- a/crates/perry-runtime/src/gc/pin.rs +++ b/crates/perry-runtime/src/gc/pin.rs @@ -251,6 +251,155 @@ pub(crate) fn test_reset_young_pin_latch() { YOUNG_PIN_EVER.store(false, Ordering::Release); } +/// Is this header self-consistent, given what each flag is allowed to mean? +/// +/// Returns `None` when nothing contradicts, or `Some(reason)` when the header +/// cannot describe a live object of the type it claims. Used by the +/// `move_young` pin-latch abort: the flags and type it already has in hand are +/// the *only* evidence that exists at the instant of the fault, and they +/// separate "the young-pin latch is incomplete" from "the copier followed a +/// dangling pointer into recycled memory" — two faults with completely +/// different investigations that the abort used to report identically. +pub(super) fn header_incoherence(obj_type: u8, size: u32, flags: u8) -> Option { + use super::types::{GC_FLAG_INTERNED, GC_HEADER_SIZE, GC_TYPE_MAX, GC_TYPE_STRING}; + if obj_type == 0 || obj_type > GC_TYPE_MAX { + return Some(format!( + "obj_type={obj_type} is outside the defined range 1..={GC_TYPE_MAX}" + )); + } + if flags & GC_FLAG_INTERNED != 0 && obj_type != GC_TYPE_STRING { + return Some(format!( + "GC_FLAG_INTERNED is set, but it is written in exactly one place \ + (string/intern.rs) and only on GC_TYPE_STRING — never on {}", + gc_type_label(obj_type) + )); + } + let total = size as usize; + if total < GC_HEADER_SIZE || total > super::copying::MAX_YOUNG_MOVE_BYTES { + return Some(format!( + "size={total} is outside the range a nursery-resident object can \ + have ({GC_HEADER_SIZE}..={})", + super::copying::MAX_YOUNG_MOVE_BYTES + )); + } + None +} + +/// The `move_young` pin-latch abort's body: what happened, what the header +/// says about which fault this is, and the candidates in the order the +/// evidence separates them. +/// +/// # Why this is not one sentence naming one cause +/// +/// It was, and the cause it named was wrong. #7645's original text asserted +/// "some site sets `GC_FLAG_PINNED` without going through `gc::pin_object`" +/// and told the reader to run `scripts/gc_pin_sites.py`. On the tree where the +/// abort was next observed (#7990, zod dep-corpus, ~1 run in 16) that tool +/// reports **OK** — every pin does originate in `pin_object`, and both of its +/// allowlisted exceptions are test-only and unreachable from a user program. +/// So the message sent every reader at a hypothesis its own tool refutes, +/// which costs more than saying nothing. +pub(super) fn pinned_young_move_report( + header_addr: usize, + obj_type: u8, + size: u32, + flags: u8, +) -> String { + let mut out = format!( + "[gc-pin-latch] FATAL: copying minor is about to relocate a PINNED young \ + object on a preflight-skipped cycle. header={header_addr:#x} \ + obj_type={obj_type} ({}) size={size} flags={flags:#04x} ({})\n", + gc_type_label(obj_type), + flag_names(flags), + ); + match header_incoherence(obj_type, size, flags) { + Some(reason) => { + out.push_str(&format!( + " header coherence: INCONSISTENT — {reason}.\n \ + So this is probably NOT a live pinned object and the young-pin latch \ + is probably innocent: the copier reached a header in memory that used \ + to hold something else, i.e. a slot that was not rooted across a \ + collection (#7154 class). Chase THAT first:\n \ + PERRY_GC_ZEAL=1 PERRY_GC_PROTECT_FROMSPACE=1 \ + PERRY_GC_PROTECT_FROMSPACE_DEPTH=800\n \ + (the default depth of 4 quarantines four retired page-sets; a value \ + can cross hundreds of collections between its last valid observation \ + and its stale use, and then the default misses it silently.)\n", + )); + } + None => { + out.push_str( + " header coherence: consistent — the flags and type do not contradict, \ + so this reads as a real pinned object in a space the copying minor \ + relocates.\n", + ); + } + } + if flags & super::types::GC_FLAG_TENURED != 0 { + out.push_str( + " note: GC_FLAG_TENURED next to a young space is NOT an anomaly. The \ + non-moving generational path tenures in place — a tenured object stays \ + physically in the nursery and the trace merely pretends it is old \ + (gc/types.rs, GC_FLAG_TENURED).\n", + ); + } + out.push_str( + " candidates, in the order the evidence above separates them:\n \ + 1. a slot that was not rooted across a collection point handed the copier a \ + stale header (see the coherence verdict; docs/src/internals/gc-rooting-invariant.md).\n \ + 2. `pin_object_non_young` was called on a young-arena object. Its debug_assert \ + is compiled out of release builds, and only the `pin_object_non_young_\ + call_sites_are_never_young` test checks the callers — a caller added \ + without a case there is invisible.\n \ + 3. `pin_object` classified the object Longlived/Old at pin time and it is young \ + now. `gc/pin.rs` rests on spaces never flowing backwards.\n \ + 4. the preflight-skip decision (`preflight_walks_decided`, gc/copying.rs) is \ + wrong even though the latch is complete.\n \ + 5. LAST: a pin site outside `gc::pin_object`. `python3 scripts/gc_pin_sites.py` \ + decides this one, and on a clean tree it answers OK — which is why #7645's \ + original text naming it as THE cause was misleading (#7990).", + ); + out +} + +/// Human-readable name for a `GcHeader::obj_type`. +/// +/// `types::gc_type_name` is `#[cfg(feature = "diagnostics")]`, and this abort +/// has to print the same text in every build — a fault report that degrades +/// with the feature set is a fault report nobody can compare against. +fn gc_type_label(obj_type: u8) -> &'static str { + super::types::gc_type_info(obj_type).map_or("unknown", |info| info.name) +} + +/// Render `gc_flags` as the constant names it is made of, so a reader does not +/// have to decode a hex byte against `gc/types.rs` by hand. +fn flag_names(flags: u8) -> String { + use super::types::{ + GC_FLAG_ARENA, GC_FLAG_FORWARDED, GC_FLAG_HAS_SURVIVED, GC_FLAG_INTERNED, GC_FLAG_MARKED, + GC_FLAG_PINNED, GC_FLAG_SHAPE_SHARED, GC_FLAG_TENURED, + }; + let mut parts: Vec<&str> = Vec::new(); + for (bit, name) in [ + (GC_FLAG_MARKED, "MARKED"), + (GC_FLAG_ARENA, "ARENA"), + (GC_FLAG_PINNED, "PINNED"), + (GC_FLAG_SHAPE_SHARED, "SHAPE_SHARED"), + (GC_FLAG_INTERNED, "INTERNED"), + (GC_FLAG_TENURED, "TENURED"), + (GC_FLAG_HAS_SURVIVED, "HAS_SURVIVED"), + (GC_FLAG_FORWARDED, "FORWARDED"), + ] { + if flags & bit != 0 { + parts.push(name); + } + } + if parts.is_empty() { + "no flags".to_string() + } else { + parts.join("|") + } +} + /// `extern "C"` form of [`pin_object`] taking the **user** pointer, for crates /// that reach the runtime through FFI declarations rather than a Rust /// dependency edge (`perry-ui-macos`, which used to open-code @@ -266,3 +415,92 @@ pub unsafe extern "C" fn js_gc_pin_user_ptr(user_ptr: *mut u8) { } pin_object(user_ptr.sub(super::types::GC_HEADER_SIZE) as *mut GcHeader); } + +#[cfg(test)] +mod report_tests { + use super::*; + use crate::gc::types::{ + GC_FLAG_ARENA, GC_FLAG_INTERNED, GC_FLAG_MARKED, GC_FLAG_PINNED, GC_FLAG_TENURED, + GC_TYPE_MAP, GC_TYPE_STRING, + }; + + /// The exact header #7990 reported, byte for byte: + /// `obj_type=8 size=731 flags=0x37`. The old message called this "a pinned + /// young Map" and sent the reader to `gc_pin_sites.py`, which answers OK. + /// The header itself says it is not a coherent Map at all. + #[test] + fn the_7990_header_is_reported_as_incoherent() { + let flags = + GC_FLAG_MARKED | GC_FLAG_ARENA | GC_FLAG_PINNED | GC_FLAG_INTERNED | GC_FLAG_TENURED; + assert_eq!(flags, 0x37, "the issue's flags byte"); + let reason = header_incoherence(GC_TYPE_MAP, 731, flags) + .expect("INTERNED on a Map contradicts string/intern.rs"); + assert!(reason.contains("GC_FLAG_INTERNED"), "{reason}"); + + let report = pinned_young_move_report(0x2db2f681350, GC_TYPE_MAP, 731, flags); + assert!(report.contains("INCONSISTENT"), "{report}"); + assert!( + report.contains("PERRY_GC_PROTECT_FROMSPACE_DEPTH=800"), + "{report}" + ); + // The decoded flag names, so a reader never hand-decodes 0x37 again. + assert!( + report.contains("MARKED|ARENA|PINNED|INTERNED|TENURED"), + "{report}" + ); + } + + /// A coherent header must NOT be described as a stale pointer — the + /// verdict has to be able to come out both ways or it is decoration. + #[test] + fn a_coherent_pinned_young_header_is_not_blamed_on_a_stale_pointer() { + let flags = GC_FLAG_MARKED | GC_FLAG_ARENA | GC_FLAG_PINNED; + assert!(header_incoherence(GC_TYPE_MAP, 64, flags).is_none()); + let report = pinned_young_move_report(0x1000, GC_TYPE_MAP, 64, flags); + assert!(report.contains("header coherence: consistent"), "{report}"); + assert!(!report.contains("INCONSISTENT"), "{report}"); + } + + #[test] + fn interned_on_a_string_is_coherent() { + let flags = GC_FLAG_MARKED | GC_FLAG_ARENA | GC_FLAG_PINNED | GC_FLAG_INTERNED; + assert!(header_incoherence(GC_TYPE_STRING, 48, flags).is_none()); + } + + #[test] + fn out_of_range_type_and_size_are_caught() { + let flags = GC_FLAG_MARKED | GC_FLAG_ARENA | GC_FLAG_PINNED; + assert!(header_incoherence(200, 64, flags).is_some()); + assert!(header_incoherence(0, 64, flags).is_some()); + // Smaller than a header, and larger than any nursery object. + assert!(header_incoherence(GC_TYPE_MAP, 2, flags).is_some()); + assert!(header_incoherence(GC_TYPE_MAP, (1 << 20) + 1, flags).is_some()); + } + + /// The refuted hypothesis must not lead. #7645's text named the pin-site + /// scan as THE cause; it is now candidate 5 of 5, and the message says why. + #[test] + fn the_pin_site_scan_is_the_last_candidate_not_the_first() { + let flags = GC_FLAG_MARKED | GC_FLAG_ARENA | GC_FLAG_PINNED; + let report = pinned_young_move_report(0x1000, GC_TYPE_MAP, 64, flags); + let scan = report.find("gc_pin_sites.py").expect("names the tool"); + let rooting = report + .find("not rooted across a collection point") + .expect("names the rooting candidate"); + assert!( + rooting < scan, + "the rooting candidate must precede the pin-site scan:\n{report}" + ); + assert!(report.contains("5. LAST"), "{report}"); + } + + /// TENURED on a young object is normal, not evidence. The message has to + /// say so, because the issue flagged it as suspicious. + #[test] + fn tenured_on_a_young_object_is_explained_rather_than_flagged() { + let flags = GC_FLAG_MARKED | GC_FLAG_ARENA | GC_FLAG_PINNED | GC_FLAG_TENURED; + let report = pinned_young_move_report(0x1000, GC_TYPE_MAP, 64, flags); + assert!(report.contains("NOT an anomaly"), "{report}"); + assert!(report.contains("tenures in place"), "{report}"); + } +} diff --git a/crates/perry/src/commands/compile.rs b/crates/perry/src/commands/compile.rs index 84c09917b8..b8d70ff996 100644 --- a/crates/perry/src/commands/compile.rs +++ b/crates/perry/src/commands/compile.rs @@ -49,6 +49,7 @@ mod update_config; pub(crate) mod resolve; mod resources; mod sandbox_buildrs; +mod shared_tokio; mod strip_dedup; mod targets; pub mod well_known; diff --git a/crates/perry/src/commands/compile/optimized_libs/no_auto.rs b/crates/perry/src/commands/compile/optimized_libs/no_auto.rs index 11eaf30b22..b0ca8ae481 100644 --- a/crates/perry/src/commands/compile/optimized_libs/no_auto.rs +++ b/crates/perry/src/commands/compile/optimized_libs/no_auto.rs @@ -89,6 +89,42 @@ pub(crate) fn resolve_prebuilt_ext_libs( libs.push(path); } None => { + // #7629 — a tokio-using wrapper CANNOT be repaired by building + // it on its own. `cargo build -p perry-ext-http` resolves + // feature unification over that crate's graph alone, so its + // bundled tokio is a different compilation than the prebuilt + // stdlib's, and the two `tokio::runtime::context::CONTEXT` + // thread-locals that result make the program SIGABRT at its + // first socket ("there is no reactor running"). Building it + // *with* perry-stdlib-static would fix tokio but silently + // overwrite the prebuilt stdlib with this invocation's feature + // set — dropping the `external-*-pump` features the no-auto + // flow depends on, trading an abort for a hang. Neither repair + // is available from here, so say what is wrong and what + // produces a coherent pair. (The link-time check in + // `compile/shared_tokio.rs` catches the same defect when the + // archive IS on disk but came from a separate invocation.) + if binding_needs_shared_tokio(module.strip_prefix("node:").unwrap_or(module)) { + eprintln!( + "error: `{}` needs {} and it is not on disk, but \ + PERRY_NO_AUTO_OPTIMIZE=1 forbids the rebuild that would produce \ + one matching the prebuilt libperry_stdlib.a.\n \ + Building `{}` on its own would bundle a SECOND tokio compilation \ + and the program would abort at its first socket with \"there is \ + no reactor running\" (#507, #7629), so it is refused here.\n \ + fix: build the wrapper in the SAME cargo invocation as the stdlib \ + archive:\n \ + cargo build --release -p perry -p perry-runtime-static \ + -p perry-stdlib-static -p {}\n \ + (add the matching `--features perry-stdlib/external-*-pump` this \ + module needs — see run_parity_tests.sh's BUILD_PACKAGES for the \ + canonical set)\n \ + or: unset PERRY_NO_AUTO_OPTIMIZE and let auto-optimize build a \ + coherent set itself.", + module, filename, binding.krate, binding.krate + ); + std::process::exit(1); + } if let Some(workspace_root) = find_perry_workspace_root() { if let Some(path) = build_missing_prebuilt_ext_lib( &workspace_root, diff --git a/crates/perry/src/commands/compile/run_pipeline.rs b/crates/perry/src/commands/compile/run_pipeline.rs index f0528f62e0..b2e638601a 100644 --- a/crates/perry/src/commands/compile/run_pipeline.rs +++ b/crates/perry/src/commands/compile/run_pipeline.rs @@ -5974,6 +5974,35 @@ pub fn run_with_parse_cache( None }; + // #7629 — refuse a link whose wrapper archives bundle a different tokio + // compilation than the stdlib archive. Two tokios means two + // `tokio::runtime::context::CONTEXT` thread-locals, and the wrapper reads + // the one perry-stdlib's runtime never entered: the binary links cleanly + // and SIGABRTs at its first socket with "there is no reactor running". + // The #507 rebuild already prevents that by construction on the + // auto-optimize path; this catches every path that bypasses it. + { + let report = super::shared_tokio::verify_shared_tokio( + stdlib_lib.as_deref(), + &optimized_libs.well_known_libs, + ); + if !report.mismatched.is_empty() { + let stdlib_path = stdlib_lib.clone().unwrap_or_default(); + return Err(anyhow!( + "{}", + super::shared_tokio::mismatch_error_message(&report, &stdlib_path) + )); + } + if verbose > 0 && report.compared_anything() { + for checked in &report.checked { + eprintln!( + " shared-tokio: {} bundles {} (matches stdlib)", + checked.name, checked.tokio_id + ); + } + } + } + // Build & run the per-platform link command. Tier 2.1 final extraction // (v0.5.342) — see crates/perry/src/commands/compile/link.rs. let link_cache_status = build_and_run_link( diff --git a/crates/perry/src/commands/compile/shared_tokio.rs b/crates/perry/src/commands/compile/shared_tokio.rs new file mode 100644 index 0000000000..11dc686530 --- /dev/null +++ b/crates/perry/src/commands/compile/shared_tokio.rs @@ -0,0 +1,498 @@ +//! Shared-tokio archive coherence (#507 invariant, #7629 enforcement). +//! +//! # The invariant +//! +//! `perry-ext-{http,net,ws,fastify,…}` are `staticlib`s: each one *bundles* a +//! copy of every Rust crate it depends on, tokio included. perry-stdlib's +//! archive bundles tokio too, and perry-stdlib is the crate that owns the +//! process's one tokio runtime (`common::async_bridge`). A wrapper's async I/O +//! only works if **both archives bundle the same tokio compilation**, because +//! the runtime context every tokio entry point consults — +//! `tokio::runtime::context::CONTEXT` — is a `thread_local!` whose symbol is +//! mangled with the *compiling crate instance's* hash. Two tokio compilations +//! in one binary means two independent CONTEXT variables: perry-stdlib's +//! runtime enters one, and the wrapper reads the other, which is empty. +//! +//! The observable failure is a Rust panic on a worker thread — and because +//! shipping profiles are `panic = "abort"`, a SIGABRT (exit 134) rather than +//! an error the program could report: +//! +//! ```text +//! thread '' panicked at crates/perry-ext-http/src/server/server.rs:911:13: +//! there is no reactor running, must be called from the context of a Tokio 1.x runtime +//! ``` +//! +//! It surfaces wherever the wrapper first needs the ambient runtime: at a +//! `tokio::spawn` call site in perry-ext-http's accept loop, or one frame +//! lower inside tokio itself (`net/tcp/listener.rs`'s `PollEvented::new` → +//! `Handle::current()`) for perry-ext-net's `TcpListener::bind`. Same cause, +//! same fix — the differing frame is only *where* the wrapper first touched +//! the reactor. +//! +//! # Why a check exists rather than just a fix +//! +//! The auto-optimize path already enforces the invariant by construction: it +//! rebuilds every tokio-using wrapper **in the same cargo invocation** as +//! perry-stdlib-static, so cargo unifies the dependency graph and both +//! archives get one tokio (`optimized_libs/driver.rs`, #507). Nothing checked +//! that it held, so every path that *bypasses* that rebuild — the +//! `PERRY_NO_AUTO_OPTIMIZE` route, the driver's "rebuild produced no archive" +//! fallback, a hand-run `cargo build -p perry-ext-http` — produced a binary +//! that linked cleanly and aborted at the first request, with the cause three +//! stages upstream of the symptom. #7629 sat open through two closes on +//! exactly that gap. +//! +//! This module makes the invariant *checkable*: the tokio compilation id is +//! readable straight out of an archive's member names (rustc names each +//! codegen unit `…tokio-.tokio.…`), so the link path can +//! compare what it is about to link and refuse a pair it knows aborts. +//! +//! The check reports what it compared, not just a verdict — a +//! [`SharedTokioReport`] with an empty `checked` list is a check that did not +//! happen (no stdlib archive, or no tokio in it), and callers surface that +//! distinctly rather than treating it as a pass. + +use std::collections::BTreeSet; +use std::io::{Read, Seek}; +use std::path::{Path, PathBuf}; + +/// One archive that was actually compared, and the tokio compilation it +/// bundles. +#[derive(Debug, Clone, PartialEq, Eq)] +pub(crate) struct CheckedArchive { + /// File name as it will appear on the link line. + pub(crate) name: String, + /// `tokio-` as bundled by that archive. + pub(crate) tokio_id: String, + /// Whether it agrees with the stdlib archive. + pub(crate) matches_stdlib: bool, +} + +/// Outcome of [`verify_shared_tokio`]. `checked` is the live-subject +/// evidence: a report with nothing in it compared nothing. +#[derive(Debug, Clone, Default, PartialEq, Eq)] +pub(crate) struct SharedTokioReport { + /// The tokio compilation bundled by the stdlib archive, when there is one. + pub(crate) stdlib_tokio_id: Option, + /// Every wrapper archive whose tokio was compared against it. + pub(crate) checked: Vec, + /// Wrapper archives that must share tokio but bundle a different one. + pub(crate) mismatched: Vec, +} + +impl SharedTokioReport { + /// Did this run actually compare anything? A gate that reports success + /// without this being true has measured nothing (CLAUDE.md's "a gate must + /// assert its subject was live"). + pub(crate) fn compared_anything(&self) -> bool { + !self.checked.is_empty() + } +} + +/// Read the member names of an `ar`-format archive (`.a` on Unix-likes, +/// `.lib` on Windows — same container). +/// +/// Deliberately parses the container in-process instead of shelling out to +/// `llvm-ar t`, the way `strip_dedup` does: a coherence gate whose tool may be +/// absent is a gate that silently stops gating, which is the failure mode +/// CLAUDE.md's "four ways a gate can be unable to fail" list calls out. The +/// only thing this needs is the 60-byte member headers. +pub(crate) fn archive_member_names(path: &Path) -> std::io::Result> { + let mut file = std::fs::File::open(path)?; + let mut magic = [0u8; 8]; + file.read_exact(&mut magic)?; + if &magic != b"!\n" { + return Ok(Vec::new()); + } + // The whole header chain is walked, but member *payloads* are skipped — + // only the BSD long-name prefix and the GNU string table are ever read. + let mut names = Vec::new(); + let mut gnu_strtab: Vec = Vec::new(); + loop { + let mut header = [0u8; 60]; + match file.read_exact(&mut header) { + Ok(()) => {} + Err(e) if e.kind() == std::io::ErrorKind::UnexpectedEof => break, + Err(e) => return Err(e), + } + if &header[58..60] != b"`\n" { + // Not a well-formed member header — stop rather than guess. + break; + } + let raw_name = String::from_utf8_lossy(&header[0..16]) + .trim_end() + .to_string(); + let size: u64 = String::from_utf8_lossy(&header[48..58]) + .trim() + .parse() + .unwrap_or(0); + let mut consumed: u64 = 0; + let mut name = raw_name.clone(); + if let Some(len_str) = raw_name.strip_prefix("#1/") { + // BSD extended name (what Apple's `ar` and rustc emit on macOS): + // the real name is the first `len` bytes of the payload. + let len: usize = len_str.trim().parse().unwrap_or(0); + let mut buf = vec![0u8; len.min(size as usize)]; + file.read_exact(&mut buf)?; + consumed += buf.len() as u64; + name = String::from_utf8_lossy(&buf) + .trim_end_matches('\0') + .to_string(); + } else if raw_name == "//" { + // GNU long-name string table. Read it; its entries resolve the + // `/` names below. + gnu_strtab = vec![0u8; size as usize]; + file.read_exact(&mut gnu_strtab)?; + consumed += size; + name = String::new(); + } else if let Some(off_str) = raw_name.strip_prefix('/') { + if let Ok(off) = off_str.trim().parse::() { + name = gnu_strtab + .get(off..) + .map(|rest| { + let end = rest + .iter() + .position(|b| *b == b'/' || *b == b'\n') + .unwrap_or(rest.len()); + String::from_utf8_lossy(&rest[..end]).to_string() + }) + .unwrap_or_default(); + } else { + // `/` alone is the symbol table — not a real member name. + name = String::new(); + } + } else { + name = raw_name.trim_end_matches('/').to_string(); + } + if !name.is_empty() { + names.push(name); + } + // Skip the remaining payload, plus the even-alignment pad byte. + let remaining = size.saturating_sub(consumed); + let pad = size % 2; + file.seek_relative((remaining + pad) as i64)?; + } + Ok(names) +} + +/// Every distinct `tokio-` compilation an archive bundles. +/// +/// rustc names each emitted codegen unit +/// `[-.]-..-cgu.N.rcgu.o`, +/// so the tokio compilation id is a dot-separated component. Matching the +/// component (rather than a substring) is what keeps `tokio_util-…` / +/// `tokio_rustls-…` / `tokio_tungstenite-…` from being mistaken for it. +pub(crate) fn tokio_compilation_ids(member_names: &[String]) -> BTreeSet { + let mut ids = BTreeSet::new(); + for name in member_names { + for component in name.split('.') { + let Some(hash) = component.strip_prefix("tokio-") else { + continue; + }; + if !hash.is_empty() && hash.chars().all(|c| c.is_ascii_hexdigit()) { + ids.insert(component.to_string()); + } + } + } + ids +} + +/// The one tokio compilation an archive bundles, if it bundles exactly one. +/// +/// More than one means the archive is itself internally inconsistent (never +/// observed; cargo produces one compilation per feature-unified graph), and +/// `None` means the archive has no tokio at all — a CPU-only wrapper, or a +/// stdlib built without `async-runtime`. +fn archive_tokio_id(path: &Path) -> Option { + let names = archive_member_names(path).ok()?; + let ids = tokio_compilation_ids(&names); + if ids.len() == 1 { + ids.into_iter().next() + } else { + None + } +} + +/// Library basenames (`perry_ext_http`, …) whose archive MUST bundle the same +/// tokio as perry-stdlib's. +/// +/// Derived from the same predicate the auto-optimize rebuild uses to decide +/// which wrappers to fold into its cargo invocation, so the check and the fix +/// can never drift apart. +pub(crate) fn shared_tokio_lib_stems() -> BTreeSet { + super::well_known::iter_well_known() + .filter(|b| { + super::optimized_libs::binding_needs_shared_tokio( + b.package.strip_prefix("node:").unwrap_or(&b.package), + ) + }) + .map(|b| b.lib.clone()) + .collect() +} + +/// Reduce a link-line path to the cargo `lib` name it carries: +/// `…/libperry_ext_http.a` and `…\perry_ext_http.lib` both give +/// `perry_ext_http`. +/// +/// Splits on BOTH separators rather than going through `Path`, because +/// `Path::file_stem` only understands `\` on a Windows host — a cross-target +/// link line handled from a Unix host would otherwise reduce the whole +/// backslash path to one component and the check would silently skip the +/// archive. +fn archive_lib_stem(path: &Path) -> Option<&str> { + let raw = path.to_str()?; + let base = raw.rsplit(['/', '\\']).next()?; + let stem = base + .strip_suffix(".a") + .or_else(|| base.strip_suffix(".lib")) + .unwrap_or(base); + Some(stem.strip_prefix("lib").unwrap_or(stem)) +} + +/// Does this link-line path name a wrapper archive bound by the invariant? +fn is_shared_tokio_archive(path: &Path, stems: &BTreeSet) -> bool { + archive_lib_stem(path).is_some_and(|stem| stems.contains(stem)) +} + +/// Compare the tokio compilation bundled by `stdlib_lib` against every +/// tokio-using wrapper archive on the link line. +/// +/// Pure inspection — no side effects, no processes spawned — so it is safe to +/// run on every link. +pub(crate) fn verify_shared_tokio( + stdlib_lib: Option<&Path>, + well_known_libs: &[PathBuf], +) -> SharedTokioReport { + let mut report = SharedTokioReport::default(); + let Some(stdlib_lib) = stdlib_lib else { + return report; + }; + let Some(stdlib_id) = archive_tokio_id(stdlib_lib) else { + // No tokio in the stdlib archive: either it was built without + // `async-runtime` (then no wrapper can reach a runtime through it and + // the link would fail on `perry_ffi_spawn_*` first), or the archive is + // unreadable. Either way there is nothing to compare, and the empty + // `checked` list says so. + return report; + }; + let stems = shared_tokio_lib_stems(); + report.stdlib_tokio_id = Some(stdlib_id.clone()); + for lib in well_known_libs { + if !is_shared_tokio_archive(lib, &stems) { + continue; + } + let Some(id) = archive_tokio_id(lib) else { + continue; + }; + let entry = CheckedArchive { + name: lib + .file_name() + .map(|n| n.to_string_lossy().into_owned()) + .unwrap_or_else(|| lib.display().to_string()), + tokio_id: id.clone(), + matches_stdlib: id == stdlib_id, + }; + if !entry.matches_stdlib { + report.mismatched.push(entry.clone()); + } + report.checked.push(entry); + } + report +} + +/// Render the failure the way the reader needs it: what disagrees, why the +/// program would abort, and the one command that produces a coherent set. +/// +/// The last part matters more than it looks. #7629's original FATAL-equivalent +/// (the tokio panic) names a source line in perry-ext-http, which is three +/// stages downstream of the mistake and sent every reader to the wrong crate. +pub(crate) fn mismatch_error_message(report: &SharedTokioReport, stdlib_lib: &Path) -> String { + let stdlib_id = report.stdlib_tokio_id.as_deref().unwrap_or(""); + let mut out = String::new(); + out.push_str( + "error: the wrapper archive(s) below bundle a DIFFERENT tokio compilation than the \ + stdlib archive they would be linked with.\n", + ); + out.push_str(&format!( + " {} bundles {}\n", + stdlib_lib.display(), + stdlib_id + )); + for m in &report.mismatched { + out.push_str(&format!(" {} bundles {}\n", m.name, m.tokio_id)); + } + out.push_str( + "\nTwo tokio compilations in one binary means two independent \ + `tokio::runtime::context::CONTEXT` thread-locals. perry-stdlib's runtime enters one; \ + the wrapper reads the other, finds it empty, and the program aborts (SIGABRT, exit 134) \ + at its first socket or `tokio::spawn` with\n \ + \"there is no reactor running, must be called from the context of a Tokio 1.x runtime\"\n\ + — see #507 and #7629. Linking this pair would produce that binary, so the link is \ + refused here instead.\n\n\ + fix: build the wrapper(s) in the SAME cargo invocation as the stdlib archive, e.g.\n \ + cargo build --release -p perry -p perry-runtime-static -p perry-stdlib-static", + ); + for m in &report.mismatched { + if let Some(stem) = archive_lib_stem(Path::new(&m.name)) { + out.push_str(&format!(" -p {}", stem.replace('_', "-"))); + } + } + out.push_str( + "\n (one invocation is what makes cargo unify tokio across them)\n \ + or: unset PERRY_NO_AUTO_OPTIMIZE and let auto-optimize rebuild a coherent set itself.", + ); + out +} + +#[cfg(test)] +mod tests { + use super::*; + + fn names(v: &[&str]) -> Vec { + v.iter().map(|s| s.to_string()).collect() + } + + #[test] + fn tokio_id_is_read_from_a_dot_separated_component() { + let members = names(&[ + "perry_stdlib-7655290ea30235cf.tokio-5aeb62139069856e.tokio.292ca503a36c1d82-cgu.0.rcgu.o.rcgu.o", + "perry_stdlib-7655290ea30235cf.core-df38416008f914c9.core.318a34b566a36fe-cgu.0.rcgu.o.rcgu.o", + ]); + let ids = tokio_compilation_ids(&members); + assert_eq!( + ids.into_iter().collect::>(), + vec!["tokio-5aeb62139069856e".to_string()] + ); + } + + #[test] + fn sibling_tokio_crates_are_not_mistaken_for_tokio() { + // These three all start with "tokio" and all appear next to the real + // one in every archive. A substring match would report four distinct + // "tokio" compilations and make the check useless. + let members = names(&[ + "tokio_util-2e43c96694a42e07.tokio_util.aa67af5cd9fce282-cgu.0.rcgu.o", + "tokio_rustls-844b0d2ba5508268.tokio_rustls.5b080bbd0f54d1f2-cgu.0.rcgu.o", + "tokio_tungstenite-fe452cb16ad32fa1.tokio_tungstenite.6897a3b80a6baa8c-cgu.0.rcgu.o", + ]); + assert!(tokio_compilation_ids(&members).is_empty()); + + let with_real = names(&[ + "tokio_util-2e43c96694a42e07.tokio_util.aa67af5cd9fce282-cgu.0.rcgu.o", + "tokio-01c4c58f10c605f6.tokio.79ef538db9d49d8e-cgu.0.rcgu.o", + ]); + assert_eq!( + tokio_compilation_ids(&with_real) + .into_iter() + .collect::>(), + vec!["tokio-01c4c58f10c605f6".to_string()] + ); + } + + #[test] + fn non_hex_suffix_is_not_a_compilation_id() { + let members = names(&["tokio-notahash.tokio.deadbeef-cgu.0.rcgu.o"]); + assert!(tokio_compilation_ids(&members).is_empty()); + } + + #[test] + fn shared_tokio_stems_cover_the_wrappers_that_own_sockets() { + let stems = shared_tokio_lib_stems(); + // The two archives #7629's witnesses abort in. + assert!(stems.contains("perry_ext_http"), "{stems:?}"); + assert!(stems.contains("perry_ext_net"), "{stems:?}"); + assert!(stems.contains("perry_ext_ws"), "{stems:?}"); + // A CPU-only wrapper must NOT be in the set: it never enters a tokio + // runtime context, so requiring a shared compilation would fail links + // that work. + assert!(!stems.contains("perry_ext_bcrypt"), "{stems:?}"); + } + + #[test] + fn link_line_paths_are_matched_on_both_platform_spellings() { + let stems = shared_tokio_lib_stems(); + assert!(is_shared_tokio_archive( + Path::new("/x/target/release/libperry_ext_http.a"), + &stems + )); + assert!(is_shared_tokio_archive( + Path::new(r"C:\x\target\release\perry_ext_http.lib"), + &stems + )); + assert!(!is_shared_tokio_archive( + Path::new("/x/target/release/libperry_ext_bcrypt.a"), + &stems + )); + } + + /// Round-trip through a real BSD-style archive (what macOS emits) so the + /// container parser is exercised, not just the name matcher. + #[test] + fn bsd_long_names_are_read_back() { + let dir = tempfile::tempdir().expect("tempdir"); + let path = dir.path().join("libfake.a"); + let member = "perry_stdlib-7655290ea30235cf.tokio-5aeb62139069856e.tokio.292ca503a36c1d82-cgu.0.rcgu.o.rcgu.o"; + let payload = b"OBJECTBYTES"; + let name_bytes = member.as_bytes(); + let size = name_bytes.len() + payload.len(); + let mut bytes = Vec::new(); + bytes.extend_from_slice(b"!\n"); + let header = format!( + "{:<16}{:<12}{:<6}{:<6}{:<8}{:<10}`\n", + format!("#1/{}", name_bytes.len()), + 0, + 0, + 0, + "100644", + size + ); + assert_eq!(header.len(), 60); + bytes.extend_from_slice(header.as_bytes()); + bytes.extend_from_slice(name_bytes); + bytes.extend_from_slice(payload); + if size % 2 == 1 { + bytes.push(b'\n'); + } + std::fs::write(&path, &bytes).expect("write archive"); + + let read_back = archive_member_names(&path).expect("read archive"); + assert_eq!(read_back, vec![member.to_string()]); + assert_eq!( + archive_tokio_id(&path).as_deref(), + Some("tokio-5aeb62139069856e") + ); + } + + #[test] + fn a_report_that_compared_nothing_is_not_a_pass() { + // No stdlib archive: nothing was compared, and `compared_anything` + // must say so rather than the caller reading "no mismatches" as proof. + let report = verify_shared_tokio(None, &[PathBuf::from("libperry_ext_http.a")]); + assert!(!report.compared_anything()); + assert!(report.mismatched.is_empty()); + assert!(report.stdlib_tokio_id.is_none()); + } + + #[test] + fn mismatch_message_names_both_ids_and_the_fixing_command() { + let report = SharedTokioReport { + stdlib_tokio_id: Some("tokio-5aeb62139069856e".to_string()), + checked: vec![CheckedArchive { + name: "libperry_ext_http.a".to_string(), + tokio_id: "tokio-01c4c58f10c605f6".to_string(), + matches_stdlib: false, + }], + mismatched: vec![CheckedArchive { + name: "libperry_ext_http.a".to_string(), + tokio_id: "tokio-01c4c58f10c605f6".to_string(), + matches_stdlib: false, + }], + }; + let msg = mismatch_error_message(&report, Path::new("/x/libperry_stdlib.a")); + assert!(msg.contains("tokio-5aeb62139069856e"), "{msg}"); + assert!(msg.contains("tokio-01c4c58f10c605f6"), "{msg}"); + assert!(msg.contains("there is no reactor running"), "{msg}"); + assert!(msg.contains("-p perry-ext-http"), "{msg}"); + assert!(msg.contains("perry-stdlib-static"), "{msg}"); + } +} diff --git a/gc-handoff/REACTOR-NOTES.md b/gc-handoff/REACTOR-NOTES.md new file mode 100644 index 0000000000..982f7e693a --- /dev/null +++ b/gc-handoff/REACTOR-NOTES.md @@ -0,0 +1,188 @@ +# #7629 / #7990 — working notes + +Worktree `/Users/amlug/projects/perry/wt-reactor`, branched from `origin/main` +at `55fd197d5` (v0.5.1500). `CARGO_TARGET_DIR=$HOME/cargo-targets/reactor`. +Host: macOS arm64. + +--- + +## 1. #7629 — root cause: TWO tokio compilations in one binary + +The six aborting gap tests are **one defect**, and it is a *build-graph* defect, +not a runtime one. + +`perry-ext-http` / `perry-ext-net` / `perry-ext-ws` / `perry-ext-fastify` are +`crate-type = ["staticlib"]`. A staticlib physically bundles every Rust crate it +depends on, tokio included. `libperry_stdlib.a` bundles tokio too, and +perry-stdlib is the crate that owns the process's one runtime +(`common::async_bridge`). Both archives land in the final link. + +tokio's runtime context — +`tokio::runtime::context::CONTEXT` — is a `thread_local!`, so its symbol is +mangled with the **compiling crate instance's** metadata hash. Two tokio +compilations therefore mean **two independent CONTEXT variables**. +perry-stdlib's runtime enters one; the wrapper reads the other, finds it empty, +and panics. Shipping profiles are `panic = "abort"`, so that is a SIGABRT +(exit 134) → the harness classifies it CRASH, not FAIL. + +This is not a new discovery, it is a **documented invariant that nothing +checked**. `optimized_libs/driver.rs` states it verbatim (#507): + +> If they're built in a different target-dir than perry-stdlib … the mangled +> hash on `tokio::runtime::context::CONTEXT` differs between the two +> staticlibs — both end up in the final binary as distinct TLS variables. +> perry-stdlib's runtime sets one; `Handle::current()` from inside the wrapper +> reads the other (empty) one and panics with "there is no reactor running". + +### Measured, on this tree + +`ar t | grep -o 'tokio-[0-9a-f]*'` reads the tokio compilation id +straight out of the member names. + +| build | `libperry_stdlib.a` | `libperry_ext_http.a` | `libperry_ext_net.a` | result | +|---|---|---|---|---| +| auto-optimize (`target/perry-auto-…`) | `tokio-692c87888a21349c` | `tokio-692c87888a21349c` | — | **PASS 3/3** | +| `PERRY_NO_AUTO_OPTIMIZE=1` | `tokio-5aeb62139069856e` | `tokio-01c4c58f10c605f6` | `tokio-59c9ffcfa9028790` | **exit 134, 3/3** | + +Three different tokios in the second row, because each archive came from its own +`cargo build -p ` invocation. Cargo resolves feature unification per +invocation, so a one-crate build gets its own tokio compilation. + +### Which paths violate the invariant + +1. **`optimized_libs/no_auto.rs::build_missing_prebuilt_ext_lib`** — literally + `cargo build --release -p perry-ext-http`. Reached whenever + `PERRY_NO_AUTO_OPTIMIZE=1` and the archive is not on disk. This is the one + that produced both witnesses here. +2. **`run_parity_tests.sh`'s node-suite net step** — a second + `cargo build --release -p perry-ext-net -j1` *after* the main build. +3. **The driver's own fallback** when the #507 rebuild produced no archive; it + already prints "CONTEXT panic risk on tokio I/O" and proceeds anyway. +4. **Any hand-run `cargo build -p perry-ext-http`** before a + `PERRY_SKIP_BUILD=1` gap run — which is what the reporting agents did, since + `PERRY_SKIP_BUILD=1` exports `PERRY_NO_AUTO_OPTIMIZE=1` and then builds + nothing. + +### Does `listener.rs:304` need a separate fix? + +**No.** Same cause, same fix. The one-frame difference is only *where the +wrapper first touched the reactor*: perry-ext-http calls `tokio::spawn` +directly at `server.rs:911`, while perry-ext-net calls `TcpListener::bind`, +whose `PollEvented::new` reaches `Handle::current()` one frame deeper inside +tokio. Both were reproduced here and both are explained by the same three-way +tokio split above. + +### Why CI never saw it + +`conformance-smoke` (the 8 gap shards) runs on `ubuntu-latest` and builds every +archive in **one** `cargo build` invocation, so the invariant holds there by +accident. The last `test.yml` run on `main` has all 8 gap shards green while +these six abort on a macOS dev box. That is why "the gap suite is red on main" +and "CI is green" were both true. + +--- + +## 2. The fix + +Three parts, in order of how much they matter. + +**(a) A link-time check that can fail.** +`crates/perry/src/commands/compile/shared_tokio.rs` parses the `ar` container +in-process (no `llvm-ar` dependency — a gate whose tool may be absent is a gate +that silently stops gating), reads each archive's `tokio-` compilation id +out of the member names, and compares `libperry_stdlib.a` against every +tokio-using wrapper on the link line. A mismatch is a hard error naming **both +ids** and the single `cargo build` that fixes it. The check reports what it +compared (`SharedTokioReport::compared_anything`), so a run that compared +nothing is distinguishable from a run that found no mismatch. + +Only wrappers where `binding_needs_shared_tokio` is true are checked — the same +predicate the #507 rebuild uses to decide what to fold into its invocation, so +the check and the fix cannot drift apart. A CPU-only wrapper (bcrypt, argon2) +never enters a tokio context, and requiring a shared compilation there would +fail links that work. + +**(b) Stop manufacturing the mismatch.** +`build_missing_prebuilt_ext_lib` now refuses to build a tokio-using wrapper on +its own under `PERRY_NO_AUTO_OPTIMIZE`, and says what to run instead. It cannot +repair the situation itself: building the wrapper *with* `perry-stdlib-static` +would fix tokio but silently overwrite the prebuilt stdlib with this +invocation's feature set, dropping the `external-*-pump` features the no-auto +flow needs — trading an abort for a hang. + +**(c) Make the harness's own builds coherent.** +`run_parity_tests.sh`: fold `-p perry-ext-net` into `BUILD_PACKAGES` instead of +a second invocation, and under `PERRY_SKIP_BUILD=1` verify every required ext +archive is present in `PERRY_RUNTIME_DIR` before running anything — with the +exact command, instead of leaving the operator to decode six SIGABRTs. + +--- + +## 3. #7990 — the FATAL message was wrong, and the header says why + +Not the same cause as #7629 (that one is a link-graph defect; this is inside the +collector). What is shared is the shape: **an error that names a cause its own +tool refutes.** + +`gc_pin_sites.py` reports OK on this tree, and both of its allowlisted +exceptions are test-only (`gc/malloc.rs`'s `push_test_object`, and the latch +sabotage test), so neither can be reached from a user program. The only +production writers of `GC_FLAG_PINNED` are `pin_object` and +`pin_object_non_young`. The message's stated cause is therefore refuted, exactly +as the issue says. + +### What the reported header actually says + +``` +obj_type=8 size=731 flags=0x37 (MARKED|ARENA|PINNED|INTERNED|TENURED) +``` + +Two of those decode differently than the issue assumed: + +* **`TENURED` on a "young" object is not an anomaly.** `gc/types.rs` is explicit: + *"Non-moving generational: tenured objects stay physically in nursery (no + copying / forwarding-pointer machinery), but the trace pretends they're + old-gen."* So TENURED + nursery-resident is the ordinary state. + +* **`INTERNED` on a `GC_TYPE_MAP` is a contradiction.** `GC_FLAG_INTERNED` is + written in exactly one file — `string/intern.rs`, two sites — and only on + strings. Every other reference reads it, or *preserves* it across a move + (`copying.rs:748`, `oldgen.rs:1932`). Nothing ever sets it on a Map. + +So the header is **not a coherent live Map**. It reads like memory that once +held an interned string. That points at the #7154 rooting class — the same +class the rest of the sweep produces on other seeds — reaching the collector +rather than surfacing later in JS, *not* at pin bookkeeping. It also explains +the ~1-in-16 rate: an unrooted *register* only goes bad when a collection lands +in its window, so it is intermittent; a bad *cache* would be reproducible. + +Note also that the latch check in `move_young` runs **before** the existing +size/plausibility guard a few lines below it, so a garbage header is +attributed to the pin latch before anything asks whether it is a plausible +object at all. + +### What changed + +`gc/pin.rs` grows `header_incoherence()` and `pinned_young_move_report()`, and +`copying.rs`'s reporter calls them. The message now: + +* decodes the flags byte by name (no more hand-decoding `0x37`); +* prints a **coherence verdict** computed from the header at the instant of the + abort — the only moment that evidence exists; +* explains TENURED-on-young instead of leaving it looking suspicious; +* lists five candidates in the order the evidence separates them, with the + pin-site scan **last** and a note saying why it led before; +* points the incoherent case at `PERRY_GC_ZEAL=1 PERRY_GC_PROTECT_FROMSPACE=1 + PERRY_GC_PROTECT_FROMSPACE_DEPTH=800`, because the default depth of 4 misses + a window hundreds of collections wide. + +Unit tests in `pin.rs` pin all of that, including a case built from #7990's +exact header bytes and a case proving the verdict can come out "consistent" +(a verdict that can only say one thing is decoration). + +### What is NOT closed on #7990 + +The underlying fault. This change makes the abort *point at the right +investigation*; it does not find the unrooted slot. Deliberately no CI gate: +at ~6% of runs, a gate would go red on a healthy tree often enough to teach +people to ignore it — the same reasoning that declined to gate #7803's 19%. diff --git a/run_parity_tests.sh b/run_parity_tests.sh index 8d8edae97a..187e289fc1 100755 --- a/run_parity_tests.sh +++ b/run_parity_tests.sh @@ -627,6 +627,20 @@ else fi BUILD_PACKAGES=(-p perry -p perry-runtime -p perry-stdlib -p perry-runtime-static -p perry-stdlib-static) BUILD_FEATURES=() +# #7629 — every tokio-using `perry-ext-*` wrapper this run will link, by +# staticlib stem. Two jobs: +# 1. they must be in the SAME `cargo build` as perry-stdlib-static, or cargo +# resolves feature unification separately for each invocation and the +# wrapper bundles a different tokio compilation than the stdlib archive. +# Two tokios means two `tokio::runtime::context::CONTEXT` thread-locals; +# the wrapper reads the one perry-stdlib's runtime never entered and the +# test SIGABRTs with "there is no reactor running" (exit 134, CRASH not +# FAIL). Perry refuses such a link since #7629, so a split build now shows +# up as a compile error rather than six aborting binaries. +# 2. under PERRY_SKIP_BUILD=1 nothing is built at all, so their presence in +# PERRY_RUNTIME_DIR is a precondition — checked below with the exact +# command instead of leaving the operator to decode a link error per test. +REQUIRED_EXT_LIBS=() needs_wasm_host=0 # The default `test-files/` corpus (the gap suite) under PERRY_NO_AUTO_OPTIMIZE # links the prebuilt `full` stdlib, which is NOT compiled with the @@ -640,6 +654,7 @@ needs_wasm_host=0 # whole well-known set rather than switching on it. if [[ -n "${PERRY_NO_AUTO_OPTIMIZE:-}" && "$TEST_SUITE" == "all" ]]; then BUILD_PACKAGES+=(-p perry-ext-events -p perry-ext-http -p perry-ext-net -p perry-ext-ws -p perry-ext-zlib) + REQUIRED_EXT_LIBS+=(perry_ext_events perry_ext_http perry_ext_net perry_ext_ws perry_ext_zlib) BUILD_FEATURES+=( perry-stdlib/external-events-construct perry-stdlib/external-http-server-pump @@ -648,6 +663,17 @@ if [[ -n "${PERRY_NO_AUTO_OPTIMIZE:-}" && "$TEST_SUITE" == "all" ]]; then perry-stdlib/external-ws-pump perry-stdlib/external-zlib-pump ) + # …and every one of those archives must then be on the link line of EVERY + # test, not just the tests that import the module. `external-zlib-pump` + # makes perry-stdlib's `js_stdlib_process_pending` reference + # `js_ext_zlib_process_pending` unconditionally, so a test that imports + # only `node:http` failed to link with five undefined `_js_ext_zlib_*` + # symbols — the pumps are a property of the ONE prebuilt stdlib, while + # archive selection is per-import. The auto-optimize path never hits this + # because it enables a pump only when it is also routing that module. + # `PERRY_FORCE_WELL_KNOWN` is the in-tree mechanism for exactly this: it + # unions modules into `well_known_iteration_set` regardless of imports. + export PERRY_FORCE_WELL_KNOWN="${PERRY_FORCE_WELL_KNOWN:-events,http,net,ws,zlib}" fi if [[ -n "${PERRY_NO_AUTO_OPTIMIZE:-}" && "$TEST_SUITE" == "node-suite" ]]; then case "$MODULE_FILTER" in @@ -657,6 +683,7 @@ if [[ -n "${PERRY_NO_AUTO_OPTIMIZE:-}" && "$TEST_SUITE" == "node-suite" ]]; then # HTTP fixtures can also emit net + ws well-known owners via the codegen # FFI registry, so build those wrappers too (#4373). BUILD_PACKAGES+=(-p perry-ext-http -p perry-ext-net -p perry-ext-ws) + REQUIRED_EXT_LIBS+=(perry_ext_http perry_ext_net perry_ext_ws) BUILD_FEATURES+=(perry-stdlib/external-http-server-pump perry-stdlib/external-http-client-pump) ;; esac @@ -688,14 +715,22 @@ if [[ "$TEST_SUITE" == "node-suite" ]]; then ;; esac fi -needs_ext_net=0 if [[ "$TEST_SUITE" == "node-suite" ]]; then case "$MODULE_FILTER" in ""|net|net/*) # node-suite/net commonly runs with PERRY_NO_AUTO_OPTIMIZE=1. # That path links prebuilt well-known archives, so build ext-net - # once up front instead of failing on unresolved js_net_* symbols. - needs_ext_net=1 + # up front instead of failing on unresolved js_net_* symbols. + # + # #7629: this used to be its OWN `cargo build -p perry-ext-net` + # after the main one. A second invocation resolves tokio's feature + # unification over perry-ext-net's graph alone, so the archive it + # produced bundled a different tokio than libperry_stdlib.a — and + # `net.createServer().listen()` then aborted inside tokio's + # `TcpListener::bind` ("there is no reactor running"). Folding it + # into BUILD_PACKAGES is the whole fix: one invocation, one tokio. + BUILD_PACKAGES+=(-p perry-ext-net) + REQUIRED_EXT_LIBS+=(perry_ext_net) ;; esac fi @@ -719,11 +754,39 @@ if [[ "$PERRY_SKIP_BUILD" == "0" && "$needs_wasm_host" -eq 1 ]]; then exit 1 fi fi -if [[ "$PERRY_SKIP_BUILD" == "0" && "$needs_ext_net" -eq 1 ]]; then - echo "Building net extension (release)..." - ext_net_jobs="${CARGO_BUILD_JOBS:-1}" - if ! cargo build --release --quiet -p perry-ext-net -j "$ext_net_jobs" 2>/dev/null; then - echo -e "${RED}Failed to build net extension library${NC}" +# #7629 — PERRY_SKIP_BUILD=1 exports PERRY_NO_AUTO_OPTIMIZE=1 and then builds +# NOTHING, so every wrapper archive the run needs must already be in +# PERRY_RUNTIME_DIR *and* must have come from the same `cargo build` as the +# stdlib archive beside it. Presence is what this can check cheaply; perry's +# own link-time check (crates/perry/src/commands/compile/shared_tokio.rs) +# compares the bundled tokio compilations and refuses an incoherent pair. +# +# Without this, a missing wrapper sent perry down `build_missing_prebuilt_ext_lib`, +# which built it in a fresh one-crate invocation — the exact split that made six +# gap tests SIGABRT on `main` for weeks while every CI shard stayed green +# (the gap job runs on ubuntu and its archives come from one invocation). +if [[ "$PERRY_SKIP_BUILD" == "1" && "${#REQUIRED_EXT_LIBS[@]}" -gt 0 ]]; then + missing_ext_libs=() + missing_ext_pkgs=() + for ext_stem in "${REQUIRED_EXT_LIBS[@]}"; do + if [[ "$HOST_PLATFORM" == "windows" ]]; then + ext_file="${ext_stem}.lib" + else + ext_file="lib${ext_stem}.a" + fi + if [[ ! -f "$PERRY_RUNTIME_DIR_SHELL/$ext_file" ]]; then + missing_ext_libs+=("$ext_file") + missing_ext_pkgs+=("-p" "${ext_stem//_/-}") + fi + done + if [[ "${#missing_ext_libs[@]}" -gt 0 ]]; then + echo -e "${RED}PERRY_SKIP_BUILD=1 but these ext archives are missing from $PERRY_RUNTIME_DIR_SHELL:${NC}" >&2 + printf ' %s\n' "${missing_ext_libs[@]}" >&2 + echo "Build them in ONE invocation with the runtime/stdlib archives — a separate" >&2 + echo "per-crate build gives the wrapper its own tokio compilation and the tests" >&2 + echo "abort with \"there is no reactor running\" (#7629):" >&2 + echo " cargo build --release ${BUILD_PACKAGES[*]} ${BUILD_FEATURE_ARGS[*]}" >&2 + echo "Or re-run with PERRY_SKIP_BUILD=0 to have this script do it." >&2 exit 1 fi fi From 7235dfe36a36b02fdce4600facf3a7f7d2db52a2 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Ralph=20K=C3=BCpper?= Date: Thu, 13 Aug 2026 00:27:37 +0200 Subject: [PATCH 2/7] test(parity): route ext-wrapper gap tests through auto-optimize (#7629) MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit One prebuilt stdlib cannot serve the mixed gap corpus: the `external-*-pump` features are a property of that single archive while ext-archive selection is per-import, so a stdlib built with `external-zlib-pump` fails to link every test that does not import node:zlib, and one built without the pumps links but never drains the wrapper's queues. There is no subset that satisfies both. Forcing every wrapper archive onto every link (PERRY_FORCE_WELL_KNOWN) does work but costs 2.2s -> 37.7s per compile, measured — 17x, which is the whole point of PERRY_SKIP_BUILD. Instead, the 23 of 554 gap tests that import an ext-routed module drop PERRY_NO_AUTO_OPTIMIZE for their own compile; the other 531 keep the fast prebuilt path. --- gc-handoff/REACTOR-NOTES.md | 31 +++++++++++++++ run_parity_tests.sh | 78 ++++++++++++++++++++----------------- 2 files changed, 74 insertions(+), 35 deletions(-) diff --git a/gc-handoff/REACTOR-NOTES.md b/gc-handoff/REACTOR-NOTES.md index 982f7e693a..6c690ffa0f 100644 --- a/gc-handoff/REACTOR-NOTES.md +++ b/gc-handoff/REACTOR-NOTES.md @@ -116,6 +116,37 @@ a second invocation, and under `PERRY_SKIP_BUILD=1` verify every required ext archive is present in `PERRY_RUNTIME_DIR` before running anything — with the exact command, instead of leaving the operator to decode six SIGABRTs. +**(d) A second, independent defect the first one was hiding.** +With coherent tokio, the no-auto gap path still failed — now at *link*, with +five undefined `_js_ext_zlib_*`. The `external-*-pump` features are a property +of the ONE prebuilt stdlib, while ext-archive selection is per-import: a stdlib +built with `external-zlib-pump` references `js_ext_zlib_process_pending` +unconditionally, so it cannot link a test that does not import `node:zlib`. The +auto-optimize path never hits this because it enables a pump only when it is +also routing that module. `run_parity_tests.sh` now exports the in-tree +`PERRY_FORCE_WELL_KNOWN=events,http,net,ws,zlib` for that path, which unions +those modules into `well_known_iteration_set` regardless of imports. + +This is worth stating plainly: the harness comment claiming the ext-package +build "compensates" for no-auto was **not true before this change** — the +recipe it describes fails to link. That branch of the harness had never been +run green. + +## 2b. Validation + +All on `b8a230366` (the fix), archives from ONE cargo invocation. + +| step | result | +|---|---| +| `libperry_{stdlib,ext_http,ext_net,ext_ws}.a` tokio ids after one invocation | all `tokio-5aeb62139069856e` | +| `test_gap_fetch_request_from_node_incoming_message`, `PERRY_NO_AUTO_OPTIMIZE=1` | `len=55 match=true`, exit 0 (was exit 134, 3/3) | +| `test_gap_net_connect_bound_value`, `PERRY_NO_AUTO_OPTIMIZE=1` | full round trip, exit 0 (was exit 134, 3/3) | +| `perry -v` on both | prints `shared-tokio: … (matches stdlib)` for http/net/ws — the check is visibly live | +| **sabotage**: `cargo build --release -p perry-ext-http` alone → id diverges to `tokio-01c4c58f10c605f6` | compile **refused**, exit 1, both ids named, no binary produced | +| restore by **rebuilding** the full invocation (not `git checkout`) | ids coherent again, compiles pass again | +| `cargo test -p perry --bin perry shared_tokio` | 11 passed | +| `cargo test -p perry-runtime --lib gc::pin::` | 6 passed | + --- ## 3. #7990 — the FATAL message was wrong, and the header says why diff --git a/run_parity_tests.sh b/run_parity_tests.sh index 187e289fc1..027b6158fe 100755 --- a/run_parity_tests.sh +++ b/run_parity_tests.sh @@ -627,8 +627,8 @@ else fi BUILD_PACKAGES=(-p perry -p perry-runtime -p perry-stdlib -p perry-runtime-static -p perry-stdlib-static) BUILD_FEATURES=() -# #7629 — every tokio-using `perry-ext-*` wrapper this run will link, by -# staticlib stem. Two jobs: +# #7629 — every tokio-using `perry-ext-*` wrapper this run will link from the +# prebuilt set, by staticlib stem. Two jobs: # 1. they must be in the SAME `cargo build` as perry-stdlib-static, or cargo # resolves feature unification separately for each invocation and the # wrapper bundles a different tokio compilation than the stdlib archive. @@ -640,41 +640,25 @@ BUILD_FEATURES=() # 2. under PERRY_SKIP_BUILD=1 nothing is built at all, so their presence in # PERRY_RUNTIME_DIR is a precondition — checked below with the exact # command instead of leaving the operator to decode a link error per test. +# The `all` suite adds nothing here on purpose: its ext-routed tests take the +# auto-optimize path per-test (see `test_routes_to_ext_wrapper`), which builds +# its own coherent archives, so no prebuilt ext archive is required for it. REQUIRED_EXT_LIBS=() needs_wasm_host=0 -# The default `test-files/` corpus (the gap suite) under PERRY_NO_AUTO_OPTIMIZE -# links the prebuilt `full` stdlib, which is NOT compiled with the -# `external-*` pump features. Any test whose module routes through a -# well-known ext wrapper (events / http / net / ws / zlib) then links against -# a stdlib with no pump and fails — reported as an untriaged NEW gap failure -# with no hint that the run mode caused it. Measured: 7 such false regressions -# (test_gap_events_import_4995, 5x http/fetch, test_gap_net_connect_bound_value), -# all of which pass with auto-optimize. node-suite already compensates below; -# do the same here. There is no MODULE_FILTER for this suite, so build the -# whole well-known set rather than switching on it. -if [[ -n "${PERRY_NO_AUTO_OPTIMIZE:-}" && "$TEST_SUITE" == "all" ]]; then - BUILD_PACKAGES+=(-p perry-ext-events -p perry-ext-http -p perry-ext-net -p perry-ext-ws -p perry-ext-zlib) - REQUIRED_EXT_LIBS+=(perry_ext_events perry_ext_http perry_ext_net perry_ext_ws perry_ext_zlib) - BUILD_FEATURES+=( - perry-stdlib/external-events-construct - perry-stdlib/external-http-server-pump - perry-stdlib/external-http-client-pump - perry-stdlib/external-net-pump - perry-stdlib/external-ws-pump - perry-stdlib/external-zlib-pump - ) - # …and every one of those archives must then be on the link line of EVERY - # test, not just the tests that import the module. `external-zlib-pump` - # makes perry-stdlib's `js_stdlib_process_pending` reference - # `js_ext_zlib_process_pending` unconditionally, so a test that imports - # only `node:http` failed to link with five undefined `_js_ext_zlib_*` - # symbols — the pumps are a property of the ONE prebuilt stdlib, while - # archive selection is per-import. The auto-optimize path never hits this - # because it enables a pump only when it is also routing that module. - # `PERRY_FORCE_WELL_KNOWN` is the in-tree mechanism for exactly this: it - # unions modules into `well_known_iteration_set` regardless of imports. - export PERRY_FORCE_WELL_KNOWN="${PERRY_FORCE_WELL_KNOWN:-events,http,net,ws,zlib}" -fi +# Modules the well-known flip routes to a `perry-ext-*` staticlib. A test that +# imports one of these cannot be served by the prebuilt stdlib at all — see +# `test_routes_to_ext_wrapper` below and the per-test override at the compile +# site, which is where the gap suite's http/net/events failures came from. +EXT_ROUTED_MODULES='http|https|http2|net|ws|zlib|events' + +# Does this test import a module the well-known flip routes to a `perry-ext-*` +# wrapper? Matches both spellings (`node:http` and `http`), both quote styles, +# and both `import … from` and `require(…)` — `test_gap_net_connect_bound_value` +# reaches `net` only through `createRequire(import.meta.url)`, so an +# `^import`-anchored match would miss it. +test_routes_to_ext_wrapper() { + grep -qE "(from|import|require\()[[:space:]]*\(?[\"'](node:)?($EXT_ROUTED_MODULES)[\"']" "$1" +} if [[ -n "${PERRY_NO_AUTO_OPTIMIZE:-}" && "$TEST_SUITE" == "node-suite" ]]; then case "$MODULE_FILTER" in ""|http|http/*|https|https/*|http2|http2/*) @@ -1365,6 +1349,30 @@ for (( selected_i = 0; selected_i < JOURNAL_TOTAL; selected_i++ )); do if [[ "$test_name" == test_parity_* || "$test_id" == node-suite/* ]]; then compile_env="PERRY_ALLOW_UNIMPLEMENTED=1" fi + # #7629 — a test that routes a module to a `perry-ext-*` wrapper cannot be + # served by ONE prebuilt stdlib. The `external-*-pump` features are a + # property of that single archive while ext-archive selection is + # per-import, so a stdlib built with (say) `external-zlib-pump` references + # `js_ext_zlib_process_pending` unconditionally and fails to link every + # test that does not import `node:zlib`; a stdlib built without the pumps + # links, but the wrapper's queues are never drained. There is no subset + # that satisfies both, which is why the "build the ext packages too" + # compensation this script used to rely on never actually worked. + # + # Auto-optimize has no such problem: it enables a pump exactly when it is + # also routing that module. So let it handle these tests specifically, + # instead of forcing every wrapper archive onto every link + # (`PERRY_FORCE_WELL_KNOWN` does work, but it costs 2.2s -> 37.7s per + # compile — measured — which is 17x the whole point of PERRY_SKIP_BUILD). + # Scoped to the `all` suite: node-suite selects one module at a time, so its + # prebuilt stdlib and its ext archives DO agree and the per-module setup + # above is coherent. It is the mixed corpus that cannot be served. + if [[ -n "${PERRY_NO_AUTO_OPTIMIZE:-}" && "$TEST_SUITE" == "all" ]] && + test_routes_to_ext_wrapper "$parity_test_file"; then + # `-u` and not `PERRY_NO_AUTO_OPTIMIZE=`: perry tests the variable with + # `var_os(...).is_some()`, so an empty-but-set value still counts as on. + compile_env="-u PERRY_NO_AUTO_OPTIMIZE $compile_env" + fi compile_flags=() if [[ -n "$BACKEND_FLAG" ]]; then compile_flags+=("$BACKEND_FLAG") From c9626e68b62a01a9693495a3a540cd73e17fbbb6 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Ralph=20K=C3=BCpper?= Date: Thu, 13 Aug 2026 00:33:05 +0200 Subject: [PATCH 3/7] fix(link): let the archive comparison decide, not a prediction (#7629) MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit The no-auto path warned-and-refused when a tokio-using wrapper archive was missing. Refusing there is a prediction: two cargo invocations CAN unify to the same tokio, and those links work. Warn and build instead, and let the link-time check — which compares the tokio compilation ids in the archives it is about to link — be the thing that fails. Evidence over heuristic, and it cannot fail a build that would have worked. --- .../compile/optimized_libs/no_auto.rs | 61 ++++++++++--------- gc-handoff/REACTOR-NOTES.md | 48 +++++++++++---- 2 files changed, 66 insertions(+), 43 deletions(-) diff --git a/crates/perry/src/commands/compile/optimized_libs/no_auto.rs b/crates/perry/src/commands/compile/optimized_libs/no_auto.rs index b0ca8ae481..fc5f669233 100644 --- a/crates/perry/src/commands/compile/optimized_libs/no_auto.rs +++ b/crates/perry/src/commands/compile/optimized_libs/no_auto.rs @@ -89,41 +89,42 @@ pub(crate) fn resolve_prebuilt_ext_libs( libs.push(path); } None => { - // #7629 — a tokio-using wrapper CANNOT be repaired by building - // it on its own. `cargo build -p perry-ext-http` resolves - // feature unification over that crate's graph alone, so its - // bundled tokio is a different compilation than the prebuilt - // stdlib's, and the two `tokio::runtime::context::CONTEXT` - // thread-locals that result make the program SIGABRT at its - // first socket ("there is no reactor running"). Building it - // *with* perry-stdlib-static would fix tokio but silently - // overwrite the prebuilt stdlib with this invocation's feature - // set — dropping the `external-*-pump` features the no-auto - // flow depends on, trading an abort for a hang. Neither repair - // is available from here, so say what is wrong and what - // produces a coherent pair. (The link-time check in - // `compile/shared_tokio.rs` catches the same defect when the - // archive IS on disk but came from a separate invocation.) + // #7629 — a tokio-using wrapper cannot be repaired from here. + // Building it alone gives it its own tokio compilation (cargo + // unifies features per invocation); building it *with* + // perry-stdlib-static would fix tokio but silently overwrite + // the prebuilt stdlib with this invocation's feature set, + // dropping the `external-*-pump` features the no-auto flow + // depends on — trading an abort for a hang. + // + // So warn, build anyway, and let the link-time check in + // `compile/shared_tokio.rs` decide: it compares the tokio + // compilation ids in the actual archives, which is evidence + // rather than a prediction. Refusing here instead would also + // fail the cases where the two invocations happen to unify to + // the same tokio, and those link and run correctly. if binding_needs_shared_tokio(module.strip_prefix("node:").unwrap_or(module)) { eprintln!( - "error: `{}` needs {} and it is not on disk, but \ - PERRY_NO_AUTO_OPTIMIZE=1 forbids the rebuild that would produce \ - one matching the prebuilt libperry_stdlib.a.\n \ - Building `{}` on its own would bundle a SECOND tokio compilation \ - and the program would abort at its first socket with \"there is \ - no reactor running\" (#507, #7629), so it is refused here.\n \ - fix: build the wrapper in the SAME cargo invocation as the stdlib \ - archive:\n \ + "warning: `{}` needs {}, which is not on disk. \ + PERRY_NO_AUTO_OPTIMIZE=1 forbids the specialized rebuild, so the \ + wrapper can only be built in its OWN cargo invocation — and cargo \ + resolves feature unification per invocation, so its bundled tokio \ + is very likely a different compilation than the prebuilt \ + libperry_stdlib.a's. Two tokio compilations means two \ + `tokio::runtime::context::CONTEXT` thread-locals and the program \ + aborts at its first socket with \"there is no reactor running\" \ + (#507, #7629).\n \ + The link refuses that pair once the archives can be compared, so \ + this build may fail after the wrapper finishes. To get it right \ + the first time, build the wrapper in the SAME cargo invocation as \ + the stdlib archive:\n \ cargo build --release -p perry -p perry-runtime-static \ -p perry-stdlib-static -p {}\n \ - (add the matching `--features perry-stdlib/external-*-pump` this \ - module needs — see run_parity_tests.sh's BUILD_PACKAGES for the \ - canonical set)\n \ - or: unset PERRY_NO_AUTO_OPTIMIZE and let auto-optimize build a \ - coherent set itself.", - module, filename, binding.krate, binding.krate + (plus the `--features perry-stdlib/external-*-pump` this module \ + needs), or unset PERRY_NO_AUTO_OPTIMIZE and let auto-optimize \ + build a coherent set itself.", + module, filename, binding.krate ); - std::process::exit(1); } if let Some(workspace_root) = find_perry_workspace_root() { if let Some(path) = build_missing_prebuilt_ext_lib( diff --git a/gc-handoff/REACTOR-NOTES.md b/gc-handoff/REACTOR-NOTES.md index 6c690ffa0f..524ad5b5a2 100644 --- a/gc-handoff/REACTOR-NOTES.md +++ b/gc-handoff/REACTOR-NOTES.md @@ -117,20 +117,42 @@ archive is present in `PERRY_RUNTIME_DIR` before running anything — with the exact command, instead of leaving the operator to decode six SIGABRTs. **(d) A second, independent defect the first one was hiding.** -With coherent tokio, the no-auto gap path still failed — now at *link*, with -five undefined `_js_ext_zlib_*`. The `external-*-pump` features are a property -of the ONE prebuilt stdlib, while ext-archive selection is per-import: a stdlib -built with `external-zlib-pump` references `js_ext_zlib_process_pending` -unconditionally, so it cannot link a test that does not import `node:zlib`. The +With coherent tokio the no-auto gap path still failed — now at *link*, with five +undefined `_js_ext_zlib_*`. The `external-*-pump` features are a property of the +ONE prebuilt stdlib, while ext-archive selection is per-import: a stdlib built +with `external-zlib-pump` references `js_ext_zlib_process_pending` +unconditionally, so it cannot link a test that does not import `node:zlib`; a +stdlib built *without* the pumps links, but the wrapper's queues are never +drained. **No subset of pump features serves a mixed corpus.** The auto-optimize path never hits this because it enables a pump only when it is -also routing that module. `run_parity_tests.sh` now exports the in-tree -`PERRY_FORCE_WELL_KNOWN=events,http,net,ws,zlib` for that path, which unions -those modules into `well_known_iteration_set` regardless of imports. - -This is worth stating plainly: the harness comment claiming the ext-package -build "compensates" for no-auto was **not true before this change** — the -recipe it describes fails to link. That branch of the harness had never been -run green. +also routing that module. + +Worth stating plainly: the harness comment claiming its ext-package build +"compensates" for no-auto was **not true** — the recipe it describes fails to +link. That branch had never been run green. + +The first attempt was `PERRY_FORCE_WELL_KNOWN=events,http,net,ws,zlib`, the +in-tree mechanism for unioning modules into `well_known_iteration_set` +regardless of imports. It works, and it is 17x too slow to keep — measured on +one trivial gap test, same host, back to back: + +| | compile | +|---|---| +| no-auto, no force | **2.2 s** | +| no-auto + `PERRY_FORCE_WELL_KNOWN` | **37.7 s** | + +(five extra archives, 193 MB, through strip-dedup on every link). At 554 tests +that turns a ~30 min gap run into ~5 h, which defeats the entire point of +`PERRY_SKIP_BUILD=1`. Measuring this before keeping it is the reason it is not +in the final change. + +What landed instead: the **23 of 554** gap tests that import an ext-routed +module (`http|https|http2|net|ws|zlib|events`, matched in both spellings, both +quote styles, and through `require(...)` — `net_connect_bound_value` reaches +`net` only via `createRequire`) drop `PERRY_NO_AUTO_OPTIMIZE` for their own +compile. The other 531 keep the 2.2 s prebuilt path. Scoped to the `all` suite: +node-suite selects one module at a time, so its prebuilt stdlib and its ext +archives already agree. ## 2b. Validation From a91a3325ac74afba310d535715293204a6f328f6 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Ralph=20K=C3=BCpper?= Date: Thu, 13 Aug 2026 02:16:05 +0200 Subject: [PATCH 4/7] docs(gc-handoff): record the gap-suite result for #7629 --- gc-handoff/REACTOR-NOTES.md | 36 ++++++++++++++++++++++++++++++++++++ 1 file changed, 36 insertions(+) diff --git a/gc-handoff/REACTOR-NOTES.md b/gc-handoff/REACTOR-NOTES.md index 524ad5b5a2..42c52fc8ec 100644 --- a/gc-handoff/REACTOR-NOTES.md +++ b/gc-handoff/REACTOR-NOTES.md @@ -169,6 +169,42 @@ All on `b8a230366` (the fix), archives from ONE cargo invocation. | `cargo test -p perry --bin perry shared_tokio` | 11 passed | | `cargo test -p perry-runtime --lib gc::pin::` | 6 passed | +### The full gap suite + +`PERRY_SKIP_BUILD=1 ./run_parity_tests.sh --filter test_gap_`, archives built with +the harness's own recipe (`-p perry -p perry-runtime -p perry-stdlib +-p perry-runtime-static -p perry-stdlib-static`), macOS arm64, 58 minutes: + +``` +Parity Pass: 538 Parity Fail: 15 Compile Fail: 1 Crashed: 0 +``` + +**`Crashed: 0`.** All six #7629 witnesses PASS, plus `test_gap_events_import_4995`. + +Against the committed Linux snapshot (`test-parity/gap_snapshot.json`, 15 entries): + +* 14 of the 15 reproduce. +* `test_gap_iterator_helpers_2874` passes here (host difference or fixed since). +* **2 failures are not in the snapshot, and neither is caused by this change:** + * `test_gap_specabi_reassign` — an output mismatch + (`plain: 99 101 2` vs `plain: 0 0 2`, `captured: 77:2` vs `captured: 0:2`). + A spec-ABI codegen defect; nothing in this change can alter program output. + * `test_gap_zlib_4917_level` — `zlib.deflateRawSync` / `inflateRawSync`. + `js_zlib_deflate_raw_sync` and `js_zlib_inflate_raw_sync` exist **only** in + `perry-stdlib/src/zlib.rs`; `perry-ext-zlib` does not define them. The + auto-optimize flip strips `compression-gzip` from the stdlib when it routes + `node:zlib` to the wrapper ("the ext crate carries all codecs, so nothing is + lost", `driver.rs`) — which is false for the *raw* sync entry points, so the + link fails with two undefined symbols. Verified both ways on this tree: the + no-auto path links it (full stdlib supplies them, exit 0) and the + auto-optimize path does not. That makes it red under the DEFAULT + `scripts/run_gap_tests.sh` on `main` today, and it is not in the snapshot + either. It is deliberately **not** routed around here: excluding `zlib` from + the ext-routed set would hide a real API gap to make a number green. + +There is no `test-parity/gap_snapshot.macos.json` in the tree, so no macOS gap +baseline has ever been recorded; the comparison above is against the Linux one. + --- ## 3. #7990 — the FATAL message was wrong, and the header says why From 64177e030c6d455f32a4c9ab7080d83dbb6fedfb Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Ralph=20K=C3=BCpper?= Date: Thu, 13 Aug 2026 02:17:39 +0200 Subject: [PATCH 5/7] docs(gc-handoff): link the two defects the #7629 gap run exposed (#8005, #8006) --- gc-handoff/REACTOR-NOTES.md | 2 ++ 1 file changed, 2 insertions(+) diff --git a/gc-handoff/REACTOR-NOTES.md b/gc-handoff/REACTOR-NOTES.md index 42c52fc8ec..a591cb774a 100644 --- a/gc-handoff/REACTOR-NOTES.md +++ b/gc-handoff/REACTOR-NOTES.md @@ -189,6 +189,7 @@ Against the committed Linux snapshot (`test-parity/gap_snapshot.json`, 15 entrie * `test_gap_specabi_reassign` — an output mismatch (`plain: 99 101 2` vs `plain: 0 0 2`, `captured: 77:2` vs `captured: 0:2`). A spec-ABI codegen defect; nothing in this change can alter program output. + Filed as **#8006**. * `test_gap_zlib_4917_level` — `zlib.deflateRawSync` / `inflateRawSync`. `js_zlib_deflate_raw_sync` and `js_zlib_inflate_raw_sync` exist **only** in `perry-stdlib/src/zlib.rs`; `perry-ext-zlib` does not define them. The @@ -201,6 +202,7 @@ Against the committed Linux snapshot (`test-parity/gap_snapshot.json`, 15 entrie `scripts/run_gap_tests.sh` on `main` today, and it is not in the snapshot either. It is deliberately **not** routed around here: excluding `zlib` from the ext-routed set would hide a real API gap to make a number green. + Filed as **#8005**. There is no `test-parity/gap_snapshot.macos.json` in the tree, so no macOS gap baseline has ever been recorded; the comparison above is against the Linux one. From acf9e6138b93c719e154c99d78bf13b85ba2e6ba Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Ralph=20K=C3=BCpper?= Date: Thu, 13 Aug 2026 02:18:02 +0200 Subject: [PATCH 6/7] docs(gc-handoff): correct the no-auto arm's description (warns, does not refuse) --- gc-handoff/REACTOR-NOTES.md | 22 ++++++++++++++-------- 1 file changed, 14 insertions(+), 8 deletions(-) diff --git a/gc-handoff/REACTOR-NOTES.md b/gc-handoff/REACTOR-NOTES.md index a591cb774a..92724795ff 100644 --- a/gc-handoff/REACTOR-NOTES.md +++ b/gc-handoff/REACTOR-NOTES.md @@ -102,19 +102,25 @@ the check and the fix cannot drift apart. A CPU-only wrapper (bcrypt, argon2) never enters a tokio context, and requiring a shared compilation there would fail links that work. -**(b) Stop manufacturing the mismatch.** -`build_missing_prebuilt_ext_lib` now refuses to build a tokio-using wrapper on -its own under `PERRY_NO_AUTO_OPTIMIZE`, and says what to run instead. It cannot -repair the situation itself: building the wrapper *with* `perry-stdlib-static` -would fix tokio but silently overwrite the prebuilt stdlib with this -invocation's feature set, dropping the `external-*-pump` features the no-auto -flow needs — trading an abort for a hang. +**(b) Warn where the mismatch is manufactured.** +`build_missing_prebuilt_ext_lib` now says what it is about to do and why it +usually ends badly, then builds anyway and lets (a) decide. It deliberately does +**not** refuse: refusing there is a prediction, and two cargo invocations *can* +unify to the same tokio — those links work, and a check that reads the actual +archives should not fail them. (The first draft did refuse; it was softened +after noticing it would fail `scripts/run_doc_tests.sh`-shaped builds that had +never been shown to be broken.) It cannot repair the situation either: building +the wrapper *with* `perry-stdlib-static` would fix tokio but silently overwrite +the prebuilt stdlib with this invocation's feature set, dropping the +`external-*-pump` features the no-auto flow needs — trading an abort for a hang. **(c) Make the harness's own builds coherent.** `run_parity_tests.sh`: fold `-p perry-ext-net` into `BUILD_PACKAGES` instead of a second invocation, and under `PERRY_SKIP_BUILD=1` verify every required ext archive is present in `PERRY_RUNTIME_DIR` before running anything — with the -exact command, instead of leaving the operator to decode six SIGABRTs. +exact command, instead of leaving the operator to decode six SIGABRTs. (For the +`all` suite that list is now empty by design: its ext-routed tests take the +auto-optimize path per test, see (d), so no prebuilt ext archive is required.) **(d) A second, independent defect the first one was hiding.** With coherent tokio the no-auto gap path still failed — now at *link*, with five From 8ec3800ce78fd10330cce08a00ea13ce999b8318 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Ralph=20K=C3=BCpper?= Date: Thu, 13 Aug 2026 02:18:59 +0200 Subject: [PATCH 7/7] docs(changelog): match the landed no-auto behaviour and record the gap result --- changelog.d/7629-shared-tokio-unification.md | 43 ++++++++++++++------ 1 file changed, 31 insertions(+), 12 deletions(-) diff --git a/changelog.d/7629-shared-tokio-unification.md b/changelog.d/7629-shared-tokio-unification.md index 04aba968a5..226bc32fcf 100644 --- a/changelog.d/7629-shared-tokio-unification.md +++ b/changelog.d/7629-shared-tokio-unification.md @@ -55,25 +55,34 @@ Three different tokios in the middle row, one per `cargo build -p `. which is the same predicate the #507 rebuild uses, so check and fix cannot drift. The report records what it compared, so "compared nothing" is distinguishable from "found no mismatch". -* `optimized_libs/no_auto.rs` no longer builds a tokio-using wrapper on its own - under `PERRY_NO_AUTO_OPTIMIZE` (`cargo build -p perry-ext-http`, which is - what manufactured the split). It cannot repair the situation either — - building the wrapper *with* `perry-stdlib-static` would overwrite the - prebuilt stdlib with this invocation's feature set and drop the - `external-*-pump` features, trading an abort for a hang — so it says what to - run instead. +* `optimized_libs/no_auto.rs` now warns before building a tokio-using wrapper + on its own under `PERRY_NO_AUTO_OPTIMIZE` (`cargo build -p perry-ext-http`, + which is what manufactured the split), naming the hazard and the command that + avoids it, then lets the link check decide. It deliberately does not refuse: + refusing there is a prediction, and two invocations *can* unify to the same + tokio. It cannot repair the situation either — building the wrapper *with* + `perry-stdlib-static` would overwrite the prebuilt stdlib with this + invocation's feature set and drop the `external-*-pump` features, trading an + abort for a hang. * `run_parity_tests.sh`: `-p perry-ext-net` moves into `BUILD_PACKAGES` (it was a *second* `cargo build -p perry-ext-net -j1`, i.e. the same split); `PERRY_SKIP_BUILD=1` now verifies the required ext archives are present in `PERRY_RUNTIME_DIR` before running anything, with the exact command; and the - no-auto gap path exports `PERRY_FORCE_WELL_KNOWN=events,http,net,ws,zlib`. + 23 of 554 gap tests that import an ext-routed module take the auto-optimize + path per test. -That last one is a second, independent defect the first fix uncovered: the -`external-*-pump` features are a property of the ONE prebuilt stdlib while +That last one addresses a second, independent defect the first fix uncovered: +the `external-*-pump` features are a property of the ONE prebuilt stdlib while archive selection is per-import, so a stdlib built with `external-zlib-pump` failed to link any test that did not import `node:zlib` (five undefined -`_js_ext_zlib_*`). The auto-optimize path never hits it because it enables a -pump only when it is also routing that module. +`_js_ext_zlib_*`), and one built without them links but never drains the +wrapper's queues. No subset serves a mixed corpus, so the "build the ext +packages too" compensation this script documented had never worked. Forcing +every wrapper archive onto every link (`PERRY_FORCE_WELL_KNOWN`) does fix it and +was measured at 2.2 s -> 37.7 s per compile — 17x, which is the whole point of +`PERRY_SKIP_BUILD` — so the per-test route was taken instead. The auto-optimize +path never hits the defect because it enables a pump only when it is also +routing that module. **Why CI stayed green.** The 8 gap shards run on `ubuntu-latest` and build every archive in one `cargo build`, so the invariant held there by accident. @@ -112,3 +121,13 @@ case built from #7990's exact header bytes. The underlying fault is not fixed this makes the abort point at the right investigation. No CI gate was added: at ~6% of runs it would go red on a healthy tree often enough to be ignored, the same reasoning that declined to gate #7803's 19%. + +### Validation + +Full gap suite on macOS arm64 (`PERRY_SKIP_BUILD=1 ./run_parity_tests.sh +--filter test_gap_`): **538 pass / 15 parity-fail / 1 compile-fail / 0 crashed**. +All six witnesses pass. 14 of the Linux snapshot's 15 entries reproduce; two +failures outside it are unrelated to this change and are filed as #8005 +(`perry-ext-zlib` has no `deflateRawSync`/`inflateRawSync`, so the `node:zlib` +flip breaks the link — red under the default runner too) and #8006 +(`test_gap_specabi_reassign` reads reassigned values back as 0).