-
-
Notifications
You must be signed in to change notification settings - Fork 157
fix(stdlib): build with --no-default-features again, unbreaking the auto-optimize relink (#7764) #7772
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
base: main
Are you sure you want to change the base?
fix(stdlib): build with --no-default-features again, unbreaking the auto-optimize relink (#7764) #7772
Changes from all commits
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -0,0 +1,13 @@ | ||
| ### Fixed | ||
|
|
||
| - **`perry-stdlib` builds with `--no-default-features` again (#7764).** That is the configuration the auto-optimize relink uses, so while it was broken every `perry` compile that triggered auto-optimize silently fell back to the prebuilt archives, and ad-hoc builds needed `PERRY_NO_AUTO_OPTIMIZE=1` as a workaround. | ||
|
|
||
| Twelve errors, from two causes, both violations of the contract `common/mod.rs` states in prose: *"Always-on code that references it must also be `#[cfg(feature = "async-runtime")]`-gated."* | ||
|
|
||
| **One** was #7745's omission, exactly as the issue diagnosed: the `js_set_native_events_dispatch` registration referenced `crate::events` without the `#[cfg(feature = "bundled-events")]` that gates the module. The neighbouring registrations in the same function are gated (`database-sqlite` on the next line), which is what makes it an omission rather than a decision. | ||
|
|
||
| **Eleven** were `worker_threads` — always-on, and referencing `common::async_bridge` across five files. Neither obvious repair works: two sites are value-producing (`js_promise_new_for_native_resolution`), so `#[cfg]` on the statement leaves nothing to return; and gating the whole `worker_threads` module is worse, because it has no feature of its own, so its FFI symbols would vanish from the stripped archive and a program importing `node:worker_threads` would fail to LINK — trading a build error for the #7629 family of failure. | ||
|
|
||
| So `worker_threads/async_shim.rs` provides the four entry points in both configurations: forwarding to `async_bridge` when it is compiled in, and settling **inline** when it is not. That is not invented semantics — the queue exists to hand work to the pump, and with no pump there is nothing to hand it to, so doing the same work synchronously reaches the same observable end state. The pinning `js_promise_new_for_native_resolution` performs is likewise a consequence of deferral, and an inline settle spans no collection point, so a plain `js_promise_new` is its correct counterpart. | ||
|
|
||
| Verified in BOTH directions — `cargo build -p perry-stdlib` and `--no-default-features` each build clean — because the first cut of the shim accidentally imported itself, which only the default-features build could see. |
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -0,0 +1,80 @@ | ||
| //! #7764: the four `common::async_bridge` entry points `worker_threads` needs, | ||
| //! available in BOTH feature configurations. | ||
| //! | ||
| //! `common/mod.rs` states the contract this exists to satisfy: | ||
| //! | ||
| //! > Tokio-backed promise/runtime bridge — only needed when an async feature … | ||
| //! > pulls in `async-runtime`. **Always-on code that references it must also be | ||
| //! > `#[cfg(feature = "async-runtime")]`-gated.** | ||
| //! | ||
| //! `worker_threads` is always-on and referenced it in eleven places across five | ||
| //! files, so `cargo build -p perry-stdlib --no-default-features` did not | ||
| //! compile. That is the configuration the auto-optimize relink uses, so every | ||
| //! `perry` compile that triggered auto-optimize fell back to the prebuilt | ||
| //! archives with a warning, and ad-hoc builds needed `PERRY_NO_AUTO_OPTIMIZE=1`. | ||
| //! | ||
| //! Gating each call site individually was not an option: two of them are | ||
| //! value-producing (`js_promise_new_for_native_resolution`) and the rest settle | ||
| //! a promise, so `#[cfg]` on the statement leaves nothing to return. Gating the | ||
| //! whole `worker_threads` module was worse — it has no feature of its own, so | ||
| //! its FFI symbols would vanish from the stripped archive and a program that | ||
| //! imports `node:worker_threads` would fail to LINK, which is the #7629 family | ||
| //! of failure rather than a fix. | ||
| //! | ||
| //! So: forward when the bridge is compiled in, and settle INLINE when it is not. | ||
| //! That is not an invented semantic. The queue exists to hand work to the pump; | ||
| //! with no pump there is nothing to hand it to, and doing the same work | ||
| //! synchronously reaches the same observable end state (the promise settles). | ||
| //! The pinning `js_promise_new_for_native_resolution` performs is likewise a | ||
| //! consequence of deferral — it keeps the promise alive across the window | ||
| //! between creation and the pump's resolution — and an inline settle spans no | ||
| //! collection point, so a plain `js_promise_new` is the correct counterpart. | ||
|
|
||
| #[cfg(feature = "async-runtime")] | ||
| pub(crate) use crate::common::async_bridge::{ | ||
| ensure_pump_registered, js_promise_new_for_native_resolution, queue_deferred_resolution, | ||
| queue_promise_resolution, | ||
| }; | ||
|
|
||
| #[cfg(not(feature = "async-runtime"))] | ||
| mod inline { | ||
| /// No bridge means no pump to register. | ||
| pub(crate) fn ensure_pump_registered() {} | ||
|
|
||
|
Comment on lines
+39
to
+43
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. 🩺 Stability & Availability | 🟠 Major | ⚡ Quick win 🧩 Analysis chain🏁 Script executed: #!/bin/bash
set -euo pipefail
rg -n -C 8 \
'ensure_pump_registered|js_register_stdlib_pump|js_register_stdlib_has_active|js_worker_threads_(process_pending|has_pending|channels_process_pending|channels_has_pending)' \
crates/perry-stdlib crates/perry-runtimeRepository: PerryTS/perry Length of output: 50369 🏁 Script executed: #!/bin/bash
set -euo pipefail
printf '--- worker async_shim feature gates ---\n'
sed -n '1,140p' crates/perry-stdlib/src/worker_threads/async_shim.rs
printf '\n--- worker pump outline and relevant sections ---\n'
ast-grep outline crates/perry-stdlib/src/worker_threads/worker_pump.rs --view expanded || true
rg -n -C 8 'js_worker_threads_(process_pending|has_pending)|start_worker|start_stdin_reader|worker_threads_process_pending|worker_threads_has_pending|ensure_pump_registered|queue_promise'| crates/perry-stdlib/src/worker_threads src/worker_threads crates/perry-runtime/src/common crates/perry-runtime/src/event_pump 2>/dev/null || true
printf '\n--- async_bridge registrations and active/pending methods ---\n'
sed -n '430,475p' crates/perry-stdlib/src/common/async_bridge.rs
rg -n -C 5 'js_register_wait_driver|ensure_gc_scanner_registered|js_stdlib_init_dispatch|ensure_pump_registered|js_run_stdlib_pump|js_stdlib_process_pending|js_stdlib_has_active_handles' crates/perry-runtime crates -g '*.rs' | head -240
printf '\n--- Cargo features for perry-stdlib/runtime ---\n'
rg -n 'async-runtime|perry-stdlib|perry-runtime' Cargo.toml crates -g 'Cargo.toml' | head -160Repository: PerryTS/perry Length of output: 242 🏁 Script executed: #!/bin/bash
set -euo pipefail
printf '%s\n' '--- worker async_shim feature gates ---'
sed -n '1,140p' crates/perry-stdlib/src/worker_threads/async_shim.rs
printf '%s\n' ''
printf '%s\n' '--- worker pump outline and relevant sections ---'
ast-grep outline crates/perry-stdlib/src/worker_threads/worker_pump.rs --view expanded || true
rg -n -C 8 'js_worker_threads_(process_pending|has_pending)|start_worker|start_stdin_reader|worker_threads_process_pending|worker_threads_has_pending|ensure_pump_registered|queue_promise' \
crates/perry-stdlib/src/worker_threads crates/perry-runtime/src/common crates/perry-runtime/src/event_pump 2>/dev/null || true
printf '%s\n' ''
printf '%s/n' '--- async_bridge registrations and active/pending methods ---'
sed -n '430,475p' crates/perry-stdlib/src/common/async_bridge.rs
rg -n -C 5 'js_register_wait_driver|ensure_gc_scanner_registered|js_stdlib_init_dispatch|ensure_pump_registered|js_run_stdlib_pump|js_stdlib_process_pending|js_stdlib_has_active_handles' \
crates/perry-runtime crates -g '*.rs' | head -240
printf '%s\n' ''
printf '%$s\n' '--- Cargo features for perry-stdlib/runtime ---'
rg -n 'async-runtime|perry-stdlib|perry-runtime' Cargo.toml crates -g 'Cargo.toml' | head -160Repository: PerryTS/perry Length of output: 46426 Provide a no-default worker pump registration path.
🤖 Prompt for AI AgentsSource: MCP tools |
||
| /// # Safety | ||
| /// Mirrors `async_bridge::js_promise_new_for_native_resolution`. | ||
| /// | ||
| /// No pinning: pinning guards the deferral window, and there is none here. | ||
| pub(crate) unsafe fn js_promise_new_for_native_resolution() -> *mut perry_runtime::Promise { | ||
| perry_runtime::js_promise_new() | ||
| } | ||
|
Comment on lines
+44
to
+50
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. 🩺 Stability & Availability | 🟠 Major | 🏗️ Heavy lift 🧩 Analysis chain🏁 Script executed: #!/bin/bash
set -euo pipefail
echo "Files:"
git ls-files | rg 'crates/perry-stdlib/src/(worker_threads(_test)?|common/async_bridge\.rs)' || true
echo
echo "async_shim outline:"
ast-grep outline crates/perry-stdlib/src/worker_threads/async_shim.rs --view expanded || true
echo
echo "worker_threads relevant sections:"
wc -l crates/perry-stdlib/src/worker_threads.rs crates/perry-stdlib/src/worker_threads/async_shim.rs
sed -n '1,90p' crates/perry-stdlib/src/worker_threads/async_shim.rs
sed -n '930,990p' crates/perry-stdlib/src/worker_threads.rs
echo
echo "Search promise pins/unpins and js_promise_new:"
rg -n "js_promise_new|js_promise_(pin|unpin)|Promise|native_resolution|resolve" crates/perry-stdlib/src crates/perry-rt crates/perry-runtime crates/perry-rt-bindings 2>/dev/null | head -n 200Repository: PerryTS/perry Length of output: 28993 🏁 Script executed: #!/bin/bash
set -euo pipefail
echo "Locate async_bridge docs and perry_runtime Promise APIs:"
rg -n "js_promise_new|js_promise_new_for_native_resolution|pinning|deferral|queued settlement|unpin" crates/perry-stdlib/src crates 2>/dev/null | head -n 300
fd -a 'async_bridge\.rs' .
fd -a -i 'promise|worker_threads' . | head -n 200Repository: PerryTS/perry Length of output: 40429 🏁 Script executed: #!/bin/bash
set -euo pipefail
echo "async_bridge full:"
cat -n crates/perry-stdlib/src/common/async_bridge.rs | sed -n '1,110p'
cat -n crates/perry-stdlib/src/common/async_bridge.rs | sed -n '470,555p'
echo
echo "worker_threads full context around promised termination:"
cat -n crates/perry-stdlib/src/worker_threads.rs | sed -n '960,1025p'
echo
echo "direct_message call sites:"
cat -n crates/perry-stdlib/src/worker_threads/direct_message.rs | sed -n '1,140p'
echo
echo "promises tests related to 859 native pin:"
cat -n test-files/test_issue_859_native_promise_pin.ts | sed -n '1,220p'Repository: PerryTS/perry Length of output: 22648 Keep the bridge Promise contract balanced for async-runtime too. With 📍 Affects 2 files
🤖 Prompt for AI AgentsSource: MCP tools |
||
|
|
||
| /// Settle now rather than queueing for a pump that does not exist. | ||
| pub(crate) fn queue_promise_resolution(promise_ptr: usize, is_success: bool, result_bits: u64) { | ||
| if promise_ptr == 0 { | ||
| return; | ||
| } | ||
| let promise = promise_ptr as *mut perry_runtime::Promise; | ||
| let value = f64::from_bits(result_bits); | ||
| if is_success { | ||
| perry_runtime::js_promise_resolve(promise, value); | ||
| } else { | ||
| perry_runtime::js_promise_reject(promise, value); | ||
| } | ||
| } | ||
|
|
||
| /// As above, running the converter inline. The `Send + 'static` bound is | ||
| /// kept so the two configurations accept the same call sites. | ||
| pub(crate) fn queue_deferred_resolution<F>(promise_ptr: usize, is_success: bool, converter: F) | ||
| where | ||
| F: FnOnce() -> u64 + Send + 'static, | ||
| { | ||
| queue_promise_resolution(promise_ptr, is_success, converter()); | ||
| } | ||
|
Comment on lines
+52
to
+73
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. 🩺 Stability & Availability | 🔴 Critical | 🏗️ Heavy lift 🧩 Analysis chain🏁 Script executed: #!/bin/bash
set -euo pipefail
rg -n -C 10 \
'queue_(deferred_)?resolution|std::thread::spawn|worker_messaging_error_value|js_promise_(resolve|reject)|js_string_from_bytes|js_error_new' \
crates/perry-stdlib/src/worker_threads
rg -n -C 8 \
'thread-local arenas|main thread|converter' \
crates/perry-stdlib/src/common/async_bridge.rsRepository: PerryTS/perry Length of output: 36754 🏁 Script executed: #!/bin/bash
set -euo pipefail
printf '--- Rust files under perry-runtime/thread and promise ---\n'
git ls-files -- crates/perry-runtime | rg 'thread|promise|value|gc' | sed -n '1,200p'
printf '\n--- Runtime imports in async_bridge.rs ---\n'
rg -n 'js_(resolve|reject|promise_)|thread-local|arena|RuntimeRootVisitor|RuntimeHandleScope|main_thread' crates/perry-stdlib/src/common/async_bridge.rs crates/perry-runtime | sed -n '1,240p'Repository: PerryTS/perry Length of output: 242 🏁 Script executed: #!/bin/bash
set -euo pipefail
printf '%s\n' '--- Runtime files likely to contain thread arena/runtime context ---'
git ls-files -- crates/perry-runtime | rg 'thread|promise|value|gc|main' | sed -n '1,200p' || true
printf '%s\n' ''
printf '%s\n' '--- Async bridge import/context ---'
sed -n '1,80p' crates/perry-stdlib/src/common/async_bridge.rs
printf '%s\n' ''
printf '%s\n' '--- Direct message rejection path with spawned worker worker ack ---'
sed -n '130,180p' crates/perry-stdlib/src/worker_threads/direct_message.rsRepository: PerryTS/perry Length of output: 13680 🏁 Script executed: #!/bin/bash
set -euo pipefail
printf '%s\n' '--- thread.rs arena definitions/usages ---'
sed -n '1,240p' crates/perry-runtime/src/thread.rs
printf '%s\n' ''
printf '%s\n' '--- promise API/resolve/reject definitions ---'
rg -n -C 8 'fn js_promise_(new|resolve|reject|resol|rejec)|js_promise_new_for_native_resolution|RuntimeHandleScope|RuntimeRootVisitor|ThreadLocal|thread_local!' crates/perry-runtime/src/promise crates/perry-runtime/src/gc/roots crates/perry-runtime/src/gc/mod.rs | sed -n '1,260p'
printf '%s\n' ''
printf '%s\n' '--- inline shim resolution functions ---'
sed -n '4,85p' crates/perry-stdlib/src/worker_threads/async_shim.rsRepository: PerryTS/perry Length of output: 39964 🏁 Script executed: #!/bin/bash
set -euo pipefail
printf '%s\n' '--- locate RuntimeHandleScope definitions ---'
rg -n -C 8 'struct RuntimeHandleScope|impl RuntimeHandleScope|enum RuntimeHandleScope|RuntimeHandleScope' crates/perry-runtime/src/gc/roots crates/perry-runtime/src/gc/mod.rs crates/perry-runtime/src/gc | sed -n '1,220p'
printf '%s\n' ''
printf '%s\n' '--- locate thread-local arena initializers and allocation symbols ---'
rg -n -C 5 'thread_local|THREAD_LOCAL|PERRY_THREAD_LOCAL|gc_init|GcInit|Arena|alloc|malloc|thread_spawn|worker_thread|spawn' crates/perry-runtime/src/gc/mod.rs crates/perry-runtime/src/gc/roots.rs crates/perry-runtime/src/gc/roots crates/perry-runtime/src/thread.rs crates/perry-runtime/src/promise/mod.rs | sed -n '1,260p'
printf '%s\n' ''
printf '%s\n' '--- direct_message imports and start call paths ---'
sed -n '1,110p' crates/perry-stdlib/src/worker_threads/direct_message.rs
rg -n -C 6 'js_worker_threads_post_message_to_thread|wait_for_direct_message_ack|DirectMessaging|worker_messaging_error_value|queue_worker_messaging_rejection' crates/perry-stdlib/src/worker_threadsRepository: PerryTS/perry Length of output: 50371 Do not run deferred promise converters on background threads.
🤖 Prompt for AI AgentsSource: MCP tools |
||
| } | ||
|
|
||
| #[cfg(not(feature = "async-runtime"))] | ||
| pub(crate) use inline::{ | ||
| ensure_pump_registered, js_promise_new_for_native_resolution, queue_deferred_resolution, | ||
| queue_promise_resolution, | ||
| }; | ||
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
🩺 Stability & Availability | 🟠 Major | ⚡ Quick win
🧩 Analysis chain
🏁 Script executed:
Repository: PerryTS/perry
Length of output: 26635
🏁 Script executed:
Repository: PerryTS/perry
Length of output: 28996
🏁 Script executed:
Repository: PerryTS/perry
Length of output: 17175
Balance the native-resolution promise on immediate termination.
When
async-runtimeis enabled,js_promise_new_for_native_resolution()pins the Promise. Theresolved_nowbranch callsperry_runtime::js_promise_resolve()directly, sounpin_promise_after_native_resolution()is never run throughjs_stdlib_process_pending(). Repeated termination of exited workers leaves pinned Promises that cannot be collected. Route native Promise creation and settlement throughasync_shimin both branches, or add a branch that directly callsunpin_promise_after_native_resolution()before resolve only for theasync-runtimepath.🤖 Prompt for AI Agents
Source: MCP tools