fix(link): refuse a link whose ext wrappers bundle a different tokio (#7629) - #7999
Conversation
…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.
|
Caution Review failedThe pull request is closed. ℹ️ Recent review info⚙️ Run configurationConfiguration used: defaults Review profile: CHILL Plan: Pro Plus Run ID: 📒 Files selected for processing (9)
📝 WalkthroughWalkthroughThe PR adds archive-level shared-Tokio validation to compiler and parity-test workflows. It also improves pinned-young GC failure diagnostics with header-coherence analysis, decoded flags, ordered investigation candidates, and unit tests. ChangesShared Tokio archive coherence
Pinned young-object diagnostics
Estimated code review effort: 4 (Complex) | ~45 minutes Sequence Diagram(s)sequenceDiagram
participant ParityRunner
participant CargoBuild
participant CompilePipeline
participant SharedTokioVerifier
participant Archives
ParityRunner->>CargoBuild: build runtime, stdlib, and required extensions
ParityRunner->>CompilePipeline: compile ext-routed test
CompilePipeline->>SharedTokioVerifier: verify_shared_tokio(stdlib, wrappers)
SharedTokioVerifier->>Archives: read Tokio compilation IDs
Archives-->>SharedTokioVerifier: archive identities
SharedTokioVerifier-->>CompilePipeline: coherent report or mismatch error
CompilePipeline-->>ParityRunner: linked test or actionable failure
Possibly related PRs
Suggested labels: Suggested reviewers: ✨ Finishing Touches📝 Generate docstrings
🧪 Generate unit tests (beta)
Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out. Comment |
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.
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.
Closes #7629. Rewrites the FATAL text tracked by #7990 (does not close it — see below).
#7629 — the six aborting gap tests are one build-graph defect
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_onfinishedandtest_gap_net_connect_bound_valuedie with SIGABRT (exit 134) — CRASH, not FAIL — after a Rust panic on a worker thread:
Five panic at perry-ext-http's
tokio::spawn;net_connect_bound_valuepanics one framelower, inside tokio's
net/tcp/listener.rs(perry-ext-net'sTcpListener::bind→PollEvented::new→Handle::current()). Thelistener.rs:304path does NOT need aseparate fix — the differing frame is only where each wrapper first touched the reactor.
Both were reproduced here and both are explained by the same cause.
Root cause
perry-ext-*wrappers arestaticlibs, so each bundles its own copy of tokio;libperry_stdlib.abundles one too, and perry-stdlib owns the process's only runtime.tokio's
runtime::context::CONTEXTis athread_local!, so its symbol carries thecompiling 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.This is not a new discovery; it is a documented invariant that nothing checked.
optimized_libs/driver.rsstates the failure verbatim (#507) and prevents it on theauto-optimize path by rebuilding every tokio-using wrapper in the same cargo invocation
as perry-stdlib-static. Every path that bypassed that rebuild produced a binary that linked
cleanly and aborted at its first socket, with the cause three stages upstream of the symptom.
Measured on
55fd197d5, reading the compilation id straight out of the archive membernames (
ar t <archive> | grep -o 'tokio-[0-9a-f]*'):libperry_stdlib.alibperry_ext_http.alibperry_ext_net.atokio-692c8788…tokio-692c8788…PERRY_NO_AUTO_OPTIMIZE=1tokio-5aeb6213…tokio-01c4c58f…tokio-59c9ffcf…tokio-5aeb6213…tokio-5aeb6213…tokio-5aeb6213…Three different tokios in the middle row — one per
cargo build -p <one crate>, becausecargo resolves feature unification per invocation.
Which paths violated the invariant
optimized_libs/no_auto.rs::build_missing_prebuilt_ext_lib— literallycargo build --release -p perry-ext-http, reached wheneverPERRY_NO_AUTO_OPTIMIZE=1and the archive is not on disk. This produced both witnesses here.
run_parity_tests.sh's node-suite net step — a secondcargo build --release -p perry-ext-net -j1after the main build.cargo build -p perry-ext-httpbefore aPERRY_SKIP_BUILD=1gap run —which is the fast path agents use, and
PERRY_SKIP_BUILD=1exportsPERRY_NO_AUTO_OPTIMIZE=1and then builds nothing.Why CI stayed green while the gap suite was red
The 8
conformance-smokegap shards run onubuntu-latestand build every archive in onecargo build, so the invariant held there by accident. The failure is reachable only froma build configuration CI does not use.
The fix
A link-time check that can fail. New
crates/perry/src/commands/compile/shared_tokio.rsreads each archive's
tokio-<hash>compilation id out of thearmember names and refusesa link whose wrappers disagree with the stdlib archive, naming both ids and the single
cargo buildthat fixes it. The container is parsed in-process rather than throughllvm-ar, so the check cannot silently stop gating when a tool is absent. Only wrapperswhere
binding_needs_shared_tokioholds are checked — the same predicate the #507 rebuilduses, so the check and the fix cannot drift apart.
SharedTokioReport::compared_anything()makes "compared nothing" distinguishable from "found no mismatch", and a unit test asserts
an empty report is not read as a pass.
Warn where the mismatch is manufactured.
build_missing_prebuilt_ext_libnow says whatit is about to do and why it usually ends badly, then builds anyway and lets the link check
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. It cannot repair the situation either, because
building the wrapper with
perry-stdlib-staticwould overwrite the prebuilt stdlib withthis invocation's feature set and drop the
external-*-pumpfeatures, trading an abort fora hang.
Make the harness's own builds coherent.
run_parity_tests.shfolds-p perry-ext-netinto
BUILD_PACKAGES, and underPERRY_SKIP_BUILD=1verifies the required ext archives arepresent in
PERRY_RUNTIME_DIRbefore running anything, with the exact command.A second, independent defect this uncovered
With coherent tokio the no-auto gap path still failed — now at link, with five undefined
_js_ext_zlib_*. Theexternal-*-pumpfeatures are a property of the ONE prebuilt stdlibwhile ext-archive selection is per-import: a stdlib built with
external-zlib-pumpreferences
js_ext_zlib_process_pendingunconditionally and cannot link a test that doesnot import
node:zlib; one built without the pumps links, but never drains the wrapper'squeues. No subset of pump features serves a mixed corpus, so the "build the ext packages
too" compensation this script documented had never actually worked.
The first attempt was
PERRY_FORCE_WELL_KNOWN=events,http,net,ws,zlib— the in-treemechanism for unioning modules into
well_known_iteration_setregardless of imports. Itworks, and it is 17x too slow to keep. Measured on one trivial gap test, same host, back to
back:
PERRY_FORCE_WELL_KNOWN(five extra archives, 193 MB, through strip-dedup on every link — ~30 min of gap suite
becomes ~5 h, which defeats the point of
PERRY_SKIP_BUILD=1).What landed instead: the 23 of 554 gap tests that import an ext-routed module drop
PERRY_NO_AUTO_OPTIMIZEfor their own compile; the other 531 keep the 2.2 s prebuilt path.The detector matches both spellings, both quote styles and
require(...), becausetest_gap_net_connect_bound_valuereachesnetonly throughcreateRequire. Scoped to theallsuite: node-suite selects one module at a time, so its prebuilt stdlib and its extarchives already agree.
#7990 — the FATAL that pointed at the wrong crate
#7645's
[gc-pin-latch]abort asserted "some site setsGC_FLAG_PINNEDwithout goingthrough
gc::pin_object" and told the reader to runscripts/gc_pin_sites.py. That toolreports OK, and both of its allowlisted exceptions are test-only — so the message sent
every reader at a hypothesis its own remedy refutes.
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_TENUREDon a nursery-residentobject is ordinary (the non-moving generational path tenures in place), and ranks five
candidates by that evidence with the pin-site scan last.
The verdict is load-bearing, not decoration.
GC_FLAG_INTERNEDis written in exactly onefile (
string/intern.rs) and only onGC_TYPE_STRING, so #7990's reported header(
obj_type=8= Map,flags=0x37includingINTERNED) is not a coherent Map — it readsas 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. The message now says so and
points at
PERRY_GC_ZEAL=1 PERRY_GC_PROTECT_FROMSPACE=1 PERRY_GC_PROTECT_FROMSPACE_DEPTH=800.#7990 is not closed by this PR. The underlying fault is still open; this makes the abort
point at the right investigation. No CI gate was added for it: at ~6% of runs a gate would
go red on a healthy tree often enough to be ignored — the same reasoning that declined to
gate #7803's 19%.
Validation
compilation ids read out of the archives as the mechanism.
perry(shared_tokio) and 6 inperry-runtime(gc::pin), allgreen. They include a sabotage-shaped case built from gc: pin latch aborts relocating a PINNED young Map on a preflight-skipped cycle (zod dep-corpus, exit 134) — and gc_pin_sites.py says OK #7990's exact header bytes and a
case proving the coherence verdict can come out both ways.
cargo fmt --all -- --checkclean;scripts/check_file_size.shOK.gc-handoff/REACTOR-NOTES.md.Gap-suite result
PERRY_SKIP_BUILD=1 ./run_parity_tests.sh --filter test_gap_on macOS arm64, archives builtwith the harness's own recipe, 58 minutes:
Crashed: 0— all six #7629 witnesses pass, plustest_gap_events_import_4995.Against the committed Linux snapshot (15 entries): 14 reproduce,
test_gap_iterator_helpers_2874passes here, and two failures are not in it — neithercaused by this change:
test_gap_specabi_reassign— an output mismatch (plain: 99 101 2vs0 0 2). Aspec-ABI codegen defect; nothing here can alter program output.
test_gap_zlib_4917_level—js_zlib_deflate_raw_sync/js_zlib_inflate_raw_syncexistonly in
perry-stdlib/src/zlib.rs;perry-ext-zlibdoes not define them, and theauto-optimize flip strips
compression-gzipfrom the stdlib when it routesnode:zlibtothe wrapper. Verified both ways on this tree: the no-auto path links it (exit 0), the
auto-optimize path does not — so it is red under the DEFAULT
scripts/run_gap_tests.shonmaintoday, and is likewise not in the snapshot. Deliberately not routed aroundhere: dropping
zlibfrom the ext-routed set would hide a real API gap to make a numbergreen.
(There is no
test-parity/gap_snapshot.macos.jsonin the tree, so no macOS gap baseline hasever been recorded; the comparison is against the Linux one.)
Summary by CodeRabbit
Bug Fixes
Tests
Documentation