From d97b74d97f628049a4dba2cdb2cdccf3ead72d8c Mon Sep 17 00:00:00 2001 From: Till Schneidereit Date: Tue, 29 Sep 2026 14:50:14 +0200 Subject: [PATCH 1/9] Update the Rust toolchain to 1.98 Needed to pull in a newer `wasi-libc`, which fixes `__wasilibc_reset_preopens` to close open FDs in addition to resetting the preopens. Without calling that function right before wizening, all FDs opened during init are retained, including `stdout` and `stderr`. That means printing anything at wizening time causes a trap when printing post resumption. This needed light changes to `crown` as well, but nothing major. --- crown/rust-toolchain.toml | 2 +- crown/src/bare_value_in_gc_struct.rs | 5 ++++- crown/src/common.rs | 22 ++++++++++++++++++++++ crown/src/trace_in_no_trace.rs | 4 +++- crown/src/unrooted_must_root.rs | 22 ++++++++++++++-------- rust-toolchain.toml | 2 +- 6 files changed, 45 insertions(+), 12 deletions(-) diff --git a/crown/rust-toolchain.toml b/crown/rust-toolchain.toml index e654de22..d9a04d30 100644 --- a/crown/rust-toolchain.toml +++ b/crown/rust-toolchain.toml @@ -1,6 +1,6 @@ [toolchain] # Keep this in sync with the project workspace's channel. -channel = "1.96" +channel = "1.98" components = [ "clippy", diff --git a/crown/src/bare_value_in_gc_struct.rs b/crown/src/bare_value_in_gc_struct.rs index 6d7c68b2..2d67c2ab 100644 --- a/crown/src/bare_value_in_gc_struct.rs +++ b/crown/src/bare_value_in_gc_struct.rs @@ -25,6 +25,7 @@ use rustc_middle::ty; use rustc_session::declare_tool_lint; use rustc_span::symbol::Symbol; +use crate::common::TyCtxtExt as _; use crate::common::{match_def_path_suffix, LateContextExt}; use crate::symbols; @@ -37,7 +38,9 @@ declare_tool_lint! { pub fn register(lint_store: &mut LintStore) { let symbols = Symbols::new(); lint_store.register_lints(&[BARE_VALUE_IN_GC_STRUCT]); - lint_store.register_late_pass(move |_| Box::new(BareValuePass::new(symbols.clone()))); + lint_store.register_late_lint_pass(Box::new(move |_| { + Box::new(BareValuePass::new(symbols.clone())) + })); } pub(crate) struct BareValuePass { diff --git a/crown/src/common.rs b/crown/src/common.rs index 694b0a37..71a6a044 100644 --- a/crown/src/common.rs +++ b/crown/src/common.rs @@ -35,6 +35,28 @@ impl<'tcx> LateContextExt for LateContext<'tcx> { /// check if a DefId's path matches the given absolute type path /// usage e.g. with /// `match_def_path(cx, id, &["core", "option", "Option"])` +/// Attribute lookups on [`TyCtxt`]. +pub trait TyCtxtExt { + /// Whether the item `did` has an attribute whose path is `attrs`. + fn has_attrs_with_path(self, did: impl Into, attrs: &[Symbol]) -> bool; +} + +impl TyCtxtExt for TyCtxt<'_> { + fn has_attrs_with_path(self, did: impl Into, attrs: &[Symbol]) -> bool { + self.get_attrs_by_path(did.into(), attrs).next().is_some() + } +} + +/// The item an alias type names: the associated type, opaque type or type alias. +pub fn alias_def_id(kind: ty::AliasTyKind<'_>) -> DefId { + match kind { + ty::AliasTyKind::Projection { def_id } => def_id.into(), + ty::AliasTyKind::Inherent { def_id } => def_id.into(), + ty::AliasTyKind::Opaque { def_id } => def_id.into(), + ty::AliasTyKind::Free { def_id } => def_id.into(), + } +} + pub fn match_def_path(cx: &LateContext, def_id: DefId, path: &[Symbol]) -> bool { let def_path = cx.tcx.def_path(def_id); let krate = &cx.tcx.crate_name(def_path.krate); diff --git a/crown/src/trace_in_no_trace.rs b/crown/src/trace_in_no_trace.rs index 7590a6de..e64da592 100644 --- a/crown/src/trace_in_no_trace.rs +++ b/crown/src/trace_in_no_trace.rs @@ -34,7 +34,9 @@ empty Traceable (like primitive types). Consider removing the wrapper."; pub fn register(lint_store: &mut LintStore) { let symbols = Symbols::new(); lint_store.register_lints(&[TRACE_IN_NO_TRACE, EMPTY_TRACE_IN_NO_TRACE]); - lint_store.register_late_pass(move |_| Box::new(NotracePass::new(symbols.clone()))); + lint_store.register_late_lint_pass(Box::new(move |_| { + Box::new(NotracePass::new(symbols.clone())) + })); } /// Lint for ensuring safe usage of NoTrace wrappers diff --git a/crown/src/unrooted_must_root.rs b/crown/src/unrooted_must_root.rs index 8a75b4de..d7bd55e2 100644 --- a/crown/src/unrooted_must_root.rs +++ b/crown/src/unrooted_must_root.rs @@ -9,6 +9,7 @@ use rustc_session::declare_tool_lint; use rustc_span::def_id::{DefId, LocalDefId}; use rustc_span::symbol::{sym, Symbol}; +use crate::common::{alias_def_id, TyCtxtExt as _}; use crate::common::{in_derive_expn, match_def_path, LateContextExt}; use crate::symbols; @@ -21,7 +22,9 @@ declare_tool_lint! { pub fn register(lint_store: &mut LintStore) { let symbols = Symbols::new(); lint_store.register_lints(&[UNROOTED_MUST_ROOT]); - lint_store.register_late_pass(move |_| Box::new(UnrootedPass::new(symbols.clone()))); + lint_store.register_late_lint_pass(Box::new(move |_| { + Box::new(UnrootedPass::new(symbols.clone())) + })); } /// Lint for ensuring safe usage of unrooted pointers @@ -75,7 +78,7 @@ fn associated_type_has_attr<'tcx>( &[sym.crown, sym.unrooted_must_root_lint, attr], ); }, - ty::Alias(alias) + ty::Alias(_, alias) if matches!( alias.kind, ty::AliasTyKind::Projection { .. } @@ -84,7 +87,7 @@ fn associated_type_has_attr<'tcx>( ) => { return cx.tcx.has_attrs_with_path( - alias.kind.def_id(), + alias_def_id(alias.kind), &[sym.crown, sym.unrooted_must_root_lint, attr], ) }, @@ -104,7 +107,10 @@ fn associated_type_has_attr<'tcx>( /// flagged as usual). fn must_root_allows_self_return(sym: &Symbols, cx: &LateContext, did: DefId) -> bool { cx.tcx - .get_attrs_by_path(did, &[sym.crown, sym.unrooted_must_root_lint, sym.must_root]) + .get_attrs_by_path( + did, + &[sym.crown, sym.unrooted_must_root_lint, sym.must_root], + ) .any(|attr| { attr.meta_item_list().is_some_and(|list| { list.iter() @@ -164,7 +170,7 @@ fn is_unrooted_ty<'tcx>( let inner = substs.type_at(0); match inner.kind() { ty::Adt(did, _) => !has_attr(did.did(), sym.allow_unrooted_in_rc), - ty::Alias(alias) + ty::Alias(_, alias) if matches!( alias.kind, ty::AliasTyKind::Projection { .. } @@ -172,7 +178,7 @@ fn is_unrooted_ty<'tcx>( | ty::AliasTyKind::Free { .. } ) => { - !has_attr(alias.kind.def_id(), sym.allow_unrooted_in_rc) + !has_attr(alias_def_id(alias.kind), sym.allow_unrooted_in_rc) }, _ => true, } @@ -271,7 +277,7 @@ fn is_unrooted_ty<'tcx>( false }, ty::FnDef(..) | ty::FnPtr(..) => false, - ty::Alias(alias) + ty::Alias(_, alias) if matches!( alias.kind, ty::AliasTyKind::Projection { .. } @@ -279,7 +285,7 @@ fn is_unrooted_ty<'tcx>( | ty::AliasTyKind::Free { .. } ) => { - let def_id = alias.kind.def_id(); + let def_id = alias_def_id(alias.kind); if has_attr(def_id, sym.must_root) { ret = true; false diff --git a/rust-toolchain.toml b/rust-toolchain.toml index 73ffd0bc..0a005a38 100644 --- a/rust-toolchain.toml +++ b/rust-toolchain.toml @@ -1,5 +1,5 @@ [toolchain] -channel = "1.96" +channel = "1.98" components = [ "clippy", From 136b17e391aff0e4ff88b65c68399e818a1609ca Mon Sep 17 00:00:00 2001 From: Till Schneidereit Date: Tue, 29 Sep 2026 14:50:14 +0200 Subject: [PATCH 2/9] Load JSON modules and dynamic imports The module load hook compiles a request with `with { type: "json" }` as a JSON module, cached independently of a JavaScript module of the same file, and refuses every other module type with a message naming the specifier. For a dynamic `import()`, the hook loads the module's own imports before handing the module over, and the engine links and evaluates it in a promise job after `import()` has returned. Before this, every dynamic import failed with "module record has unexpected status: New". --- README.md | 8 + crates/core-runtime/src/module.rs | 328 +++++++++++++++++++++++++++--- crates/js/src/module.rs | 15 +- 3 files changed, 323 insertions(+), 28 deletions(-) diff --git a/README.md b/README.md index f62c353c..e954df82 100644 --- a/README.md +++ b/README.md @@ -59,6 +59,14 @@ console.log(greet("world")); starlingmonkey main.js ``` +JSON modules are imported with an import attribute, and `import()` loads a module +dynamically: + +```js +import config from "./config.json" with { type: "json" }; +const { greet } = await import("./greet.js"); +``` + For quick one-liners, use `-e`: ```bash diff --git a/crates/core-runtime/src/module.rs b/crates/core-runtime/src/module.rs index 7869955e..4b81a842 100644 --- a/crates/core-runtime/src/module.rs +++ b/crates/core-runtime/src/module.rs @@ -46,6 +46,7 @@ use js::error::ExnThrown; use js::gc::handle::Heap; use js::gc::scope::Scope; use js::heap::Trace; +use js::module::ModuleType; use js::module_raw::{transform_str_to_source_text, SetModulePrivate}; use js::native::{GCHandle, HandleObject, JSNative, JSObject, JSString, JSTracer, Value}; use js::prelude::{HandleValue, RootScope}; @@ -237,16 +238,24 @@ pub fn settled_module_evaluation<'s>( if !promise.is_rejected() { return Ok(None); } + Err(rejection_message(scope, &promise, context)) +} +/// `context: reason` for the rejected module evaluation `promise`. +pub(crate) fn rejection_message( + scope: &Scope<'_>, + promise: &js::Promise<'_>, + context: &str, +) -> String { // Re-raise the reason so the shared capture path formats it the same way a // thrown exception is formatted, then clear it again. let Some(reason) = promise.result(scope) else { - return Err(context.to_string()); + return context.to_string(); }; js::exception::set_pending(scope, reason, js::native::ExceptionStackBehavior::Capture); match ExnThrown::capture(scope).message { - Some(message) => Err(format!("{context}: {message}")), - None => Err(context.to_string()), + Some(message) => format!("{context}: {message}"), + None => context.to_string(), } } @@ -258,13 +267,20 @@ pub fn settled_module_evaluation<'s>( /// a module graph. /// /// Resolution strategy: -/// 1. Check the module registry for an exact match: native modules by name, -/// already-compiled file modules by canonical path. +/// 1. For a JavaScript request, check the module registry for an exact match: +/// native modules by name, already-compiled file modules by canonical path. /// 2. Resolve via `oxc_resolver` against the referencing module's directory /// (file-backed modules carry their canonical path in their module /// private), or against the loader's base path for pathless referrers /// (eval scripts, native modules). /// +/// A request with `with { type: "json" }` compiles the file as a JSON module. +/// Every other module type is refused. +/// +/// For a dynamic `import()`, the hook also loads the module's own imports +/// before handing it over, and the engine links and evaluates it in a promise +/// job. +/// /// Every call ends either by handing the module to /// `finish_loading_imported_module` or by returning `false`, which leaves the /// engine to fail the load with the exception this reported. @@ -288,20 +304,36 @@ unsafe extern "C" fn module_load_hook( return false; }; let specifier = jsstr_to_string(&scope, specifier_str); + let kind = match js::module::get_module_request_type(&scope, request) { + ModuleType::JavaScript => ModuleKind::JavaScript, + ModuleType::JSON => ModuleKind::Json, + _ => { + let message = format!( + "Cannot import '{specifier}': only JavaScript and JSON modules are supported" + ); + let c_msg = CString::new(message).unwrap_or_else(|_| c"Unsupported module type".into()); + js::error::report_error_ascii(&scope, &c_msg); + return false; + } + }; - // 1. Check the module registry for an exact match. - let cached = registry(|reg| { - reg.borrow() - .get(&specifier) - .map(|entry| entry.module_obj.get(&scope)) - }); + // 1. Check the module registry for an exact match. The `json:`-prefixed keys + // hold JSON modules, which a JavaScript import never gets. + let cached = match kind { + ModuleKind::JavaScript if !specifier.starts_with("json:") => registry(|reg| { + reg.borrow() + .get(&specifier) + .map(|entry| entry.module_obj.get(&scope)) + }), + ModuleKind::JavaScript | ModuleKind::Json => None, + }; // 2. Resolve via filesystem using oxc_resolver, relative to the referrer. let module = match cached { Some(obj) => obj, None => { let base_dir = referrer_base_dir(&scope, referrer); - match resolve_file_module(&scope, &specifier, base_dir) { + match resolve_file_module(&scope, &specifier, base_dir, kind) { Ok(obj) => obj, Err(ModuleLoadError::Failed(msg)) => { let c_msg = @@ -315,6 +347,14 @@ unsafe extern "C" fn module_load_hook( } }; + // SAFETY: SpiderMonkey passes the hook a rooted `payload` handle. + let payload_handle = unsafe { GCHandle::from_raw(payload) }; + if js::module::is_dynamic_import_payload(&scope, payload_handle) + && js::module::load_requested_modules(&scope, module).is_err() + { + return false; + } + // SAFETY: this is the load hook, passing its own arguments, and this is its only call. unsafe { js::module::finish_loading_imported_module( @@ -353,7 +393,15 @@ fn referrer_base_dir( base_path(|bp| bp.borrow().clone()) } -/// Resolve a specifier to a file on disk, compile it as a module, and cache it. +/// The kinds of module the loader compiles from a file. +#[derive(Clone, Copy, PartialEq, Eq)] +enum ModuleKind { + JavaScript, + Json, +} + +/// Resolve a specifier to a file on disk, compile it as a `kind` module, and +/// cache it. /// /// Only compiles the module. Loading its own imports, linking and evaluation /// are handled by SpiderMonkey's module pipeline. @@ -361,6 +409,7 @@ fn resolve_file_module<'r>( scope: &'r Scope, specifier: &str, base_dir: Option, + kind: ModuleKind, ) -> Result, ModuleLoadError> { let base_dir = base_dir .ok_or_else(|| format!("Module '{}' not found (no base path configured)", specifier))?; @@ -386,11 +435,30 @@ fn resolve_file_module<'r>( let canonical_path = std::fs::canonicalize(&resolved_path) .unwrap_or_else(|_| lexically_normalize(resolved_path)); let canonical_key = canonical_path.to_string_lossy().to_string(); + // A `.json` file imported as JavaScript is refused rather than compiled as a + // script, which would fail on its syntax or silently evaluate it. + if kind == ModuleKind::JavaScript + && canonical_path + .extension() + .is_some_and(|extension| extension.eq_ignore_ascii_case("json")) + { + return Err(ModuleLoadError::Failed(format!( + "Cannot import '{specifier}' as JavaScript: it resolves to the JSON file '{canonical_key}'. \ + Import it with `with {{ type: \"json\" }}`." + ))); + } + // A file imported both as JavaScript and as JSON is two modules, so the JSON + // one is cached under a `json:`-prefixed key, which no canonical path has, + // and which the registry lookup by specifier skips. + let cache_key = match kind { + ModuleKind::JavaScript => canonical_key.clone(), + ModuleKind::Json => format!("json:{canonical_key}"), + }; // Check if already compiled under the canonical path let cached = registry(|reg| { reg.borrow() - .get(&canonical_key) + .get(&cache_key) .map(|entry| entry.module_obj.get(scope)) }); if let Some(obj) = cached { @@ -406,21 +474,32 @@ fn resolve_file_module<'r>( CString::new(canonical_key.as_bytes()).map_err(|_| "Invalid filename".to_string())?; let options = js::compile::options(scope, c_filename, 1); let mut src = transform_str_to_source_text(&source); - // SAFETY: `options` and `src` are valid for the duration of this call. - let module = unsafe { js::module::compile_module(scope, options.ptr, &mut src) } - .map_err(|_| ModuleLoadError::CompileThrew)?; - - // Store the canonical path in the module private: the load hook reads it - // to resolve this module's own relative imports against its directory. - let path_str = js::JSString::from_str(scope, &canonical_key) - .map_err(|_| "Failed to allocate module path string".to_string())?; - // SAFETY: `module` is the rooted module object `compile_module` returned, and `path_str` is - // rooted. - unsafe { SetModulePrivate(module.as_raw(), &path_str.as_value()) }; + let module = match kind { + ModuleKind::JavaScript => { + // SAFETY: `options` and `src` are valid for the duration of this call. + let module = unsafe { js::module::compile_module(scope, options.ptr, &mut src) } + .map_err(|_| ModuleLoadError::CompileThrew)?; + + // Store the canonical path in the module private: the load hook reads it + // to resolve this module's own relative imports against its directory. + let path_str = js::JSString::from_str(scope, &canonical_key) + .map_err(|_| "Failed to allocate module path string".to_string())?; + // SAFETY: `module` is the rooted module object `compile_module` returned, and + // `path_str` is rooted. + unsafe { SetModulePrivate(module.as_raw(), &path_str.as_value()) }; + module + } + // A JSON module has no imports, so it needs no path. + // SAFETY: `options` and `src` are valid for the duration of this call. + ModuleKind::Json => { + unsafe { js::module::compile_json_module(scope, options.ptr, &mut src) } + .map_err(|_| ModuleLoadError::CompileThrew)? + } + }; registry(|reg| { reg.borrow_mut().insert( - canonical_key, + cache_key, ModuleEntry { module_obj: Heap::from(module), }, @@ -1283,4 +1362,201 @@ mod tests { assert_eq!(read_global_f64(&scope, "globalThis._file"), 888.0); } } + + #[test] + fn json_import_evaluates_to_the_parsed_value() { + let dir = test_tempdir(); + std::fs::write(dir.path().join("data.json"), r#"{ "n": 7, "s": "seven" }"#).unwrap(); + + let rt = test_runtime(); + rt.reset_module_loader(dir.path().to_path_buf()); + let scope = rt.default_global(); + unsafe { + let result = evaluate_module( + &scope, + r#" + import data from "./data.json" with { type: "json" }; + globalThis._jsonN = data.n; + globalThis._jsonS = data.s; + "#, + "entry.mjs", + ); + assert!(result.is_ok(), "JSON module import failed"); + assert_eq!(read_global_f64(&scope, "globalThis._jsonN"), 7.0); + assert_eq!(read_global_string(&scope, "globalThis._jsonS"), "seven"); + } + } + + #[test] + fn invalid_json_module_fails_to_load() { + let dir = test_tempdir(); + std::fs::write(dir.path().join("bad.json"), r#"{ "n": "#).unwrap(); + + let rt = test_runtime(); + rt.reset_module_loader(dir.path().to_path_buf()); + let scope = rt.default_global(); + unsafe { + let result = evaluate_module( + &scope, + r#"import data from "./bad.json" with { type: "json" };"#, + "entry.mjs", + ); + assert!(result.is_err(), "an invalid JSON module must not load"); + } + } + + #[test] + fn json_file_imported_as_javascript_is_refused() { + let dir = test_tempdir(); + std::fs::write(dir.path().join("data.json"), r#"{ "n": 7 }"#).unwrap(); + + let rt = test_runtime(); + rt.reset_module_loader(dir.path().to_path_buf()); + let scope = rt.default_global(); + unsafe { + let result = evaluate_module(&scope, r#"import data from "./data.json";"#, "entry.mjs"); + assert!( + result.is_err(), + "a JSON file imported as JavaScript must be refused" + ); + let message = ExnThrown::capture(&scope).message.unwrap_or_default(); + assert!(message.contains(r#"with { type: "json" }"#), "{message}"); + } + } + + #[test] + fn unsupported_module_type_is_refused() { + let dir = test_tempdir(); + std::fs::write(dir.path().join("style.css"), "body {}\n").unwrap(); + + let rt = test_runtime(); + rt.reset_module_loader(dir.path().to_path_buf()); + let scope = rt.default_global(); + unsafe { + let result = evaluate_module( + &scope, + r#"import style from "./style.css" with { type: "css" };"#, + "entry.mjs", + ); + assert!(result.is_err(), "a CSS module import must be refused"); + let message = ExnThrown::capture(&scope).message.unwrap_or_default(); + assert!( + message.contains("only JavaScript and JSON modules are supported"), + "{message}" + ); + } + } + + #[test] + fn dynamic_import_loads_the_module_graph_after_the_call_returns() { + let dir = test_tempdir(); + std::fs::write(dir.path().join("c.js"), "export const BASE = 10;\n").unwrap(); + std::fs::write( + dir.path().join("b.js"), + r#" + import { BASE } from "./c.js"; + globalThis._order.push("b evaluated"); + export const DOUBLED = BASE * 2; + "#, + ) + .unwrap(); + + let rt = test_runtime(); + rt.reset_module_loader(dir.path().to_path_buf()); + let scope = rt.default_global(); + unsafe { + let result = evaluate_module( + &scope, + r#" + globalThis._order = []; + import("./b.js").then((m) => { globalThis._dynamic = m.DOUBLED; }); + globalThis._order.push("import() returned"); + "#, + "entry.mjs", + ); + assert!(result.is_ok(), "module evaluation failed"); + crate::event_loop::run_microtasks(&scope); + assert_eq!(read_global_f64(&scope, "globalThis._dynamic"), 20.0); + assert_eq!( + read_global_string(&scope, "globalThis._order.join()"), + "import() returned,b evaluated" + ); + } + } + + #[test] + fn top_level_await_on_dynamic_import() { + let dir = test_tempdir(); + std::fs::write(dir.path().join("helper.js"), "export const VALUE = 5;\n").unwrap(); + std::fs::write(dir.path().join("data.json"), r#"{ "n": 3 }"#).unwrap(); + + let rt = test_runtime(); + rt.reset_module_loader(dir.path().to_path_buf()); + let scope = rt.default_global(); + unsafe { + let result = evaluate_module( + &scope, + r#" + import data from "./data.json" with { type: "json" }; + const { VALUE } = await import("./helper.js"); + const json = await import("./data.json", { with: { type: "json" } }); + globalThis._sum = VALUE + json.default.n; + globalThis._sameJson = json.default === data; + "#, + "entry.mjs", + ); + assert!(result.is_ok(), "module evaluation failed"); + crate::event_loop::run_microtasks(&scope); + assert_eq!(read_global_f64(&scope, "globalThis._sum"), 8.0); + assert_eq!( + read_global_string(&scope, "String(globalThis._sameJson)"), + "true" + ); + } + } + + #[test] + fn dynamic_import_of_a_missing_module_rejects() { + let dir = test_tempdir(); + + let rt = test_runtime(); + rt.reset_module_loader(dir.path().to_path_buf()); + let scope = rt.default_global(); + unsafe { + let result = evaluate_module( + &scope, + r#" + import("./missing.js").catch((e) => { globalThis._error = e.message; }); + "#, + "entry.mjs", + ); + assert!(result.is_ok(), "module evaluation failed"); + crate::event_loop::run_microtasks(&scope); + assert!(read_global_string(&scope, "globalThis._error") + .contains("Cannot resolve module './missing.js'")); + } + } + + #[test] + fn dynamic_import_of_a_module_with_a_missing_dependency_rejects() { + let dir = test_tempdir(); + std::fs::write(dir.path().join("present.js"), "import './missing.js';\n").unwrap(); + + let rt = test_runtime(); + rt.reset_module_loader(dir.path().to_path_buf()); + let scope = rt.default_global(); + unsafe { + let result = evaluate_module( + &scope, + r#" + import("./present.js").catch((e) => { globalThis._error = e.message; }); + "#, + "entry.mjs", + ); + assert!(result.is_ok(), "module evaluation failed"); + crate::event_loop::run_microtasks(&scope); + assert!(read_global_string(&scope, "globalThis._error") + .contains("Cannot resolve module './missing.js'")); + } + } } diff --git a/crates/js/src/module.rs b/crates/js/src/module.rs index 1736ed5f..39ea1919 100644 --- a/crates/js/src/module.rs +++ b/crates/js/src/module.rs @@ -14,8 +14,9 @@ use crate::{Object, Promise}; use mozjs::gc::{Handle, HandleObject, HandleValue}; use mozjs::jsapi::mozilla::Utf8Unit; use mozjs::jsapi::HandleValue as RawHandleValue; +pub use mozjs::jsapi::ModuleType; use mozjs::jsapi::{ - ExceptionStackBehavior, JSObject, JSScript, JSString, ModuleErrorBehaviour, ModuleType, + ExceptionStackBehavior, JSObject, JSScript, JSString, ModuleErrorBehaviour, ReadOnlyCompileOptions, SourceText, }; use mozjs::jsval::UndefinedValue; @@ -286,6 +287,10 @@ pub fn load_requested_modules(scope: &Scope<'_>, module_record: Object) -> Resul /// received. The hook must call this exactly once per invocation unless it /// returns `false`. /// +/// A promise `payload` belongs to a dynamic `import()`. The hook must have +/// loaded `result`'s own imports with [`load_requested_modules`] first, and the +/// engine then links and evaluates `result` in a promise job. +/// /// # Safety /// /// Only valid from within a module load hook, with that hook's own arguments. @@ -303,8 +308,14 @@ pub unsafe fn finish_loading_imported_module( module_request, payload, result, - false, + is_dynamic_import_payload(scope, payload), ) }; ExnThrown::check(ok) } + +/// Whether a load hook's `payload` belongs to a dynamic `import()`, whose +/// payload is its promise, rather than to loading a static module graph. +pub fn is_dynamic_import_payload(scope: &Scope<'_>, payload: HandleValue) -> bool { + Object::from_value(scope, payload).is_ok_and(|object| object.cast::().is_ok()) +} From 2b48e0ed52a7b1ea5917835f4e707ba35df6154c Mon Sep 17 00:00:00 2001 From: Till Schneidereit Date: Tue, 29 Sep 2026 14:51:29 +0200 Subject: [PATCH 3/9] Refuse Wizer snapshots with pending work, and close file descriptors first During `pre_initialize`, the runtime enables recording stacks for the creation of tasks, timers, interest handles and async-promise futures using `js::stack::current_origin`. If any of them are still active at snapshot time, `EventLoop::ensure_idle_for_snapshot` prints errors for them, including the recorded stacks. `mark_resumed_from_snapshot` becomes `prepare_for_snapshot`, which also flushes stdio and calls `__wasilibc_reset_preopens` to close any open FDs, including those for `stdout` and `stderr`. --- crates/core-runtime/src/config.rs | 4 +- crates/core-runtime/src/event_loop.rs | 175 ++++++++++++++++-- .../core-runtime/src/event_loop/interest.rs | 26 ++- crates/core-runtime/src/lib.rs | 27 ++- crates/core-runtime/src/runtime.rs | 7 + crates/core-runtime/tests/snapshot_idle.rs | 122 ++++++++++++ crates/js/src/promise.rs | 81 +++++--- crates/js/src/stack.rs | 71 ++++++- crates/libstarling/src/serve_wasm.rs | 107 ++++++----- scripts/test-wizer.sh | 34 +++- starling/src/lib.rs | 2 +- 11 files changed, 551 insertions(+), 105 deletions(-) create mode 100644 crates/core-runtime/tests/snapshot_idle.rs diff --git a/crates/core-runtime/src/config.rs b/crates/core-runtime/src/config.rs index 138fa8f1..920ae6b0 100644 --- a/crates/core-runtime/src/config.rs +++ b/crates/core-runtime/src/config.rs @@ -132,7 +132,9 @@ pub struct RuntimeConfig { #[command(flatten)] pub serve_limits: ServeLimits, - /// Pre-initialize the runtime (used during wizer snapshot). + /// Pre-initialize the runtime for a Wizer snapshot. The runtime then records where each piece + /// of asynchronous work is created, for reporting work still pending when the snapshot is + /// taken (see [`EventLoop::ensure_idle_for_snapshot`](crate::event_loop::EventLoop::ensure_idle_for_snapshot)). #[arg(skip)] pub pre_initialize: bool, } diff --git a/crates/core-runtime/src/event_loop.rs b/crates/core-runtime/src/event_loop.rs index ee466d61..8a4097ea 100644 --- a/crates/core-runtime/src/event_loop.rs +++ b/crates/core-runtime/src/event_loop.rs @@ -188,6 +188,51 @@ pub enum StepOutcome { Progressed, } +/// A `setTimeout`/`setInterval` timer, keyed by the id JS sees. +struct JsTimer { + /// The task the timer controls. An interval re-queues itself under the same one. + task: TaskId, + /// Where the timer was created, kept for each re-queue of an interval. + origin: Option>, +} + +// --------------------------------------------------------------------------- +// Pending work +// --------------------------------------------------------------------------- + +/// One thing that keeps an event loop alive, as [`EventLoop::pending_work`] reports it. +#[derive(Debug)] +pub struct PendingWork { + pub kind: PendingKind, + /// The JS call stack that created the work, if origins were being recorded then. See + /// [`js::stack::record_origins`]. + pub origin: Option>, +} + +/// The kinds of [`PendingWork`]. +#[derive(Debug, Clone, Copy, PartialEq, Eq)] +pub enum PendingKind { + /// A task waiting for its deadline, labelled with its [`Task::kind`]. + Timer(&'static str), + /// A task waiting to be signalled ready or to run, labelled with its [`Task::kind`]. + Task(&'static str), + /// External interest held through an [`InterestHandle`], such as a stream being read. + Interest, + /// An async-promise future, such as a `fetch` in flight. + Future, +} + +impl std::fmt::Display for PendingKind { + fn fmt(&self, f: &mut std::fmt::Formatter<'_>) -> std::fmt::Result { + match self { + PendingKind::Timer(kind) => write!(f, "a pending `{kind}` timer"), + PendingKind::Task(kind) => write!(f, "a queued `{kind}` task"), + PendingKind::Interest => f.write_str("an operation that keeps the event loop alive"), + PendingKind::Future => f.write_str("a host operation in flight"), + } + } +} + // --------------------------------------------------------------------------- // Task trait // --------------------------------------------------------------------------- @@ -233,6 +278,9 @@ struct TaskEntry { /// The reading comes from `platform::clock`, not `std::time`, so a deadline /// taken before a Wizer snapshot is already past when the instance resumes. deadline: Option, + /// Where the task was queued, if origins were being recorded then. See + /// [`js::stack::record_origins`]. + origin: Option>, } impl TaskEntry { @@ -341,7 +389,7 @@ pub struct EventLoop { tasks: RefCell, /// HTML's per-loop "map of setTimeout and setInterval IDs": the timer ids /// JS sees, mapped to the internal task they control. - js_timers: RefCell>, + js_timers: RefCell>, /// Scratch buffer for [`step`](Self::step)'s per-batch dispatch list, /// reused across steps so the hot path stays allocation-free. batch_buf: RefCell>, @@ -384,11 +432,14 @@ impl EventLoop { /// Returns the [`TaskId`] assigned to this task. pub fn queue(&self, task: Box) -> TaskId { let id = self.next_task_id(); + // Read before `tasks` is borrowed: recording the origin can GC, which traces the loop. + let origin = js::stack::current_origin(); self.tasks.borrow_mut().insert(TaskEntry { id, task, ready: false, deadline: None, + origin, }); id } @@ -399,11 +450,14 @@ impl EventLoop { /// external event (e.g. resolved promises, `queueMicrotask` work). pub fn queue_ready(&self, task: Box) -> TaskId { let id = self.next_task_id(); + // Read before `tasks` is borrowed: recording the origin can GC, which traces the loop. + let origin = js::stack::current_origin(); self.tasks.borrow_mut().insert(TaskEntry { id, task, ready: true, deadline: None, + origin, }); self.notify.notify(1); id @@ -416,15 +470,27 @@ impl EventLoop { /// similar, use a deadline of `Instant::now()`. pub fn queue_timer(&self, task: Box, deadline: Instant) -> TaskId { let id = self.next_task_id(); + self.insert_timer(id, task, deadline, js::stack::current_origin()); + id + } + + /// Insert a timer task under `id`, and wake the driver so it can re-evaluate the earliest + /// timer deadline. + fn insert_timer( + &self, + id: TaskId, + task: Box, + deadline: Instant, + origin: Option>, + ) { self.tasks.borrow_mut().insert(TaskEntry { id, task, ready: false, deadline: Some(deadline), + origin, }); - // Wake the driver so it can re-evaluate the earliest timer deadline. self.notify.notify(1); - id } /// Cancel a queued task, removing it from the event loop. @@ -444,8 +510,12 @@ impl EventLoop { make_task: impl FnOnce(u64) -> Box, ) -> u64 { let timer_id = self.allocate_js_timer_id(scope); - let task_id = self.queue_timer(make_task(timer_id), deadline); - self.js_timers.borrow_mut().insert(timer_id, task_id); + let task = self.next_task_id(); + let origin = js::stack::current_origin(); + self.insert_timer(task, make_task(timer_id), deadline, origin.clone()); + self.js_timers + .borrow_mut() + .insert(timer_id, JsTimer { task, origin }); timer_id } @@ -461,17 +531,11 @@ impl EventLoop { task: Box, deadline: Instant, ) { - if self.js_timers.borrow().get(&timer_id) != Some(&id) { - return; - } - self.tasks.borrow_mut().insert(TaskEntry { - id, - task, - ready: false, - deadline: Some(deadline), - }); - // Wake the driver so it can re-evaluate the earliest timer deadline. - self.notify.notify(1); + let origin = match self.js_timers.borrow().get(&timer_id) { + Some(timer) if timer.task == id => timer.origin.clone(), + _ => return, + }; + self.insert_timer(id, task, deadline, origin); } /// Cancel a JS timer by its `setTimeout`/`setInterval` id. @@ -479,11 +543,11 @@ impl EventLoop { /// Invalid IDs are silently ignored per spec. pub fn clear_js_timer(&self, timer_id: u64) { let removed = self.js_timers.borrow_mut().remove(&timer_id); - if let Some(task_id) = removed { + if let Some(timer) = removed { // If the task is still queued, remove it. If it is currently // running (an interval clearing itself from its own callback), // the now-missing map entry suppresses the re-queue instead. - self.cancel_if_queued(task_id); + self.cancel_if_queued(timer.task); } } @@ -602,6 +666,81 @@ impl EventLoop { js::promise::has_pending_futures(self.loop_id) } + /// `Ok` if this loop holds no work, and otherwise `Err` describing each piece of work it holds + /// and where it was created, for a caller about to take a Wizer snapshot, which cannot capture + /// pending work. + pub fn ensure_idle_for_snapshot(&self) -> Result<(), String> { + use std::fmt::Write as _; + + let work = self.pending_work(); + if work.is_empty() { + return Ok(()); + } + let mut message = String::from( + "the application's top level finished evaluating with asynchronous work still \ + pending, which a snapshot cannot hold. Await or cancel each of these before the top \ + level finishes:", + ); + for item in work { + match item.origin { + Some(origin) => { + let _ = write!(message, "\n - {}, created at:", item.kind); + for frame in origin.lines() { + let _ = write!(message, "\n {frame}"); + } + } + None => { + let _ = write!( + message, + "\n - {}, created with no JavaScript on the stack", + item.kind + ); + } + } + } + Err(message) + } + + /// Everything that keeps this loop alive: its tasks, its external interest, and the + /// async-promise futures it owns. Each entry's origin is set if origins were being recorded + /// when the work was created (see [`js::stack::record_origins`]). + /// + /// Tasks come first, in the order they were queued, then interest, then futures. + pub fn pending_work(&self) -> Vec { + let tasks = self.tasks.borrow(); + let mut entries: Vec<&TaskEntry> = tasks.entries.values().collect(); + entries.sort_unstable_by_key(|entry| entry.id.0); + let mut work: Vec = entries + .into_iter() + .map(|entry| PendingWork { + kind: match entry.deadline { + Some(_) => PendingKind::Timer(entry.task.kind()), + None => PendingKind::Task(entry.task.kind()), + }, + origin: entry.origin.clone(), + }) + .collect(); + let interest = self.interest.origins(); + let unrecorded = self.interest.count() as usize - interest.len(); + work.extend(interest.into_iter().map(|origin| PendingWork { + kind: PendingKind::Interest, + origin: Some(origin), + })); + work.extend((0..unrecorded).map(|_| PendingWork { + kind: PendingKind::Interest, + origin: None, + })); + work.extend( + js::promise::pending_future_origins(self.loop_id) + .into_iter() + .map(|origin| PendingWork { + kind: PendingKind::Future, + origin, + }), + ); + work + } + /// An [`EventListener`] resolving on the next readiness notification: a task became ready, a /// timer was queued. It must be awaited, and completes on its first poll if the notification /// arrived before it was created. diff --git a/crates/core-runtime/src/event_loop/interest.rs b/crates/core-runtime/src/event_loop/interest.rs index b18f3faa..ee98b86c 100644 --- a/crates/core-runtime/src/event_loop/interest.rs +++ b/crates/core-runtime/src/event_loop/interest.rs @@ -17,7 +17,8 @@ //! The event loop is considered alive when either the task queue has //! pending work or the interest count is positive. -use std::cell::Cell; +use std::cell::{Cell, RefCell}; +use std::collections::HashMap; use std::rc::Rc; use event_listener::Event; @@ -26,6 +27,10 @@ use event_listener::Event; /// [`InterestHandle`]s. struct InterestState { count: Cell, + /// The origin of each held handle acquired while origins were being recorded, by the key in + /// its [`InterestHandle::origin`]. See [`js::stack::record_origins`]. + origins: RefCell>>, + next_origin_key: Cell, /// The owning loop's driver notification: a release may happen while the /// owning loop is parked in its await branch (the handle dropped during /// another loop's turn), and must wake it so it can observe `Done`. @@ -49,6 +54,8 @@ impl InterestTracker { Self { state: Rc::new(InterestState { count: Cell::new(0), + origins: RefCell::new(HashMap::new()), + next_origin_key: Cell::new(0), notify, }), } @@ -61,11 +68,23 @@ impl InterestTracker { self.state .count .set(count.checked_add(1).expect("interest count overflow")); + let origin = js::stack::current_origin().map(|origin| { + let key = self.state.next_origin_key.get(); + self.state.next_origin_key.set(key + 1); + self.state.origins.borrow_mut().insert(key, origin); + key + }); InterestHandle { state: Rc::clone(&self.state), + origin, } } + /// The origins of the held handles acquired while origins were being recorded. + pub fn origins(&self) -> Vec> { + self.state.origins.borrow().values().cloned().collect() + } + /// Returns `true` if at least one interest is held. pub fn has_interest(&self) -> bool { self.state.count.get() > 0 @@ -82,6 +101,8 @@ impl InterestTracker { /// is active at the time, and wakes the loop's driver. pub struct InterestHandle { state: Rc, + /// The key of this handle's entry in [`InterestState::origins`], if it has one. + origin: Option, } impl InterestHandle { @@ -98,6 +119,9 @@ impl Drop for InterestHandle { .checked_sub(1) .expect("InterestHandle outlived its tracker's count"), ); + if let Some(key) = self.origin { + self.state.origins.borrow_mut().remove(&key); + } // Wake the owning loop's driver — interest may have dropped to zero // while it was parked. self.state.notify.notify(1); diff --git a/crates/core-runtime/src/lib.rs b/crates/core-runtime/src/lib.rs index 0695ac86..434c0be6 100644 --- a/crates/core-runtime/src/lib.rs +++ b/crates/core-runtime/src/lib.rs @@ -118,11 +118,10 @@ pub unsafe fn evaluate_content_script( let pending = if module_mode { module::settled_module_evaluation(scope, value, "Script evaluation failed")? - .map(js::gc::handle::RootedHeap::new) } else { None }; - Ok(ScriptEvaluation { pending }) + Ok(ScriptEvaluation::new(pending)) } /// What a content script left running: its evaluation promise, if a top-level `await` means it is @@ -133,12 +132,36 @@ pub struct ScriptEvaluation { } impl ScriptEvaluation { + /// The evaluation of a module whose evaluation promise is `pending`, as + /// [`module::settled_module_evaluation`] returns it. `None` is a finished evaluation. + pub fn new(pending: Option>) -> Self { + Self { + pending: pending.map(js::gc::handle::RootedHeap::new), + } + } + /// Whether the script has finished evaluating. pub fn is_finished(&self, scope: &js::gc::scope::Scope<'_>) -> bool { self.pending .as_ref() .is_none_or(|promise| !promise.get(scope).is_pending()) } + + /// `Err` with `context: reason` if the script's top-level `await` rejected after this + /// evaluation was created, and `Ok` otherwise, including while it is still pending. + pub fn rejection(&self, scope: &js::gc::scope::Scope<'_>, context: &str) -> Result<(), String> { + match &self.pending { + Some(promise) => { + let promise = promise.get(scope); + if promise.is_rejected() { + Err(module::rejection_message(scope, &promise, context)) + } else { + Ok(()) + } + } + None => Ok(()), + } + } } /// Initialize the runtime and evaluate the script (the synchronous portion of an diff --git a/crates/core-runtime/src/runtime.rs b/crates/core-runtime/src/runtime.rs index 0851e672..4b7f9bcf 100644 --- a/crates/core-runtime/src/runtime.rs +++ b/crates/core-runtime/src/runtime.rs @@ -290,6 +290,11 @@ impl Runtime { // Create the default global and register builtins. drop(rt.new_global()); + if config.pre_initialize { + let scope = rt.default_global(); + // SAFETY: the context lives as long as `rt`, whose `Drop` stops the recording. + unsafe { js::stack::record_origins(Some(scope.raw_cx_no_gc())) }; + } rt.run_initializer_script(config)?; Ok(rt) @@ -458,6 +463,8 @@ impl Runtime { impl Drop for Runtime { fn drop(&mut self) { + // SAFETY: stopping the recording has no precondition. + unsafe { js::stack::record_origins(None) }; // Futures left by loops that were never canceled, and those spawned with no loop active, // have to go while the context is alive. js::promise::cancel_all_pending_futures(); diff --git a/crates/core-runtime/tests/snapshot_idle.rs b/crates/core-runtime/tests/snapshot_idle.rs new file mode 100644 index 00000000..5b0fafdb --- /dev/null +++ b/crates/core-runtime/tests/snapshot_idle.rs @@ -0,0 +1,122 @@ +// SPDX-License-Identifier: Apache-2.0-WITH-LLVM-exception + +//! `EventLoop::ensure_idle_for_snapshot`, and the origins a runtime configured for a snapshot +//! records for the work it reports. +//! +//! The cases are grouped in a single test function because `JSEngine` can only be initialized +//! once per process. + +use core_runtime::config::RuntimeConfig; +use core_runtime::event_loop::timer::install_timer_globals; +use core_runtime::event_loop::{with_event_loop, EventLoop}; +use core_runtime::runtime::Runtime; +use js::compile::evaluate_with_filename; +use js::gc::scope::Scope; + +/// Evaluate `code` from `filename` with `event_loop` active. +fn eval_on(scope: &Scope<'_>, event_loop: &EventLoop, filename: &str, code: &str) { + with_event_loop(event_loop, |_| { + evaluate_with_filename(scope, code, filename, 1) + .unwrap_or_else(|_| panic!("{filename} threw")); + }); +} + +#[test] +fn pending_work_is_reported_with_its_origin() { + let config = RuntimeConfig { + pre_initialize: true, + ..Default::default() + }; + let rt = Runtime::init(&config).expect("runtime init"); + let scope = rt.default_global(); + install_timer_globals(&scope, scope.global()); + + // A loop that holds nothing can be snapshotted. + assert!(EventLoop::new().ensure_idle_for_snapshot().is_ok()); + + // A timer is reported with the stack that created it, and clearing it leaves the loop idle. + { + let el = EventLoop::new(); + eval_on( + &scope, + &el, + "arm.js", + "function arm() {\n return setTimeout(() => {}, 60000);\n}\nglobalThis.timer = arm();", + ); + let message = el.ensure_idle_for_snapshot().unwrap_err(); + assert!( + message.contains("a pending `timeout` timer, created at:\n arm@arm.js:2:"), + "{message}" + ); + assert!(message.contains("\n @arm.js:4:"), "{message}"); + eval_on(&scope, &el, "clear.js", "clearTimeout(globalThis.timer);"); + assert!(el.ensure_idle_for_snapshot().is_ok()); + } + + // An interval keeps the origin of the call that created it each time it re-queues itself. + { + let el = EventLoop::new(); + eval_on( + &scope, + &el, + "interval.js", + "globalThis.ticks = 0;\nglobalThis.interval = setInterval(() => { globalThis.ticks++; }, 0);", + ); + while evaluate_with_filename(&scope, "globalThis.ticks", "ticks.js", 1) + .unwrap() + .to_int32() + == 0 + { + std::thread::sleep(std::time::Duration::from_millis(1)); + with_event_loop(&el, |el| el.step(&scope)); + } + let message = el.ensure_idle_for_snapshot().unwrap_err(); + assert!( + message.contains("a pending `interval` timer, created at:\n @interval.js:2:"), + "{message}" + ); + eval_on( + &scope, + &el, + "clear.js", + "clearInterval(globalThis.interval);", + ); + assert!(el.ensure_idle_for_snapshot().is_ok()); + } + + // Interest acquired with no script running has no origin, and releasing it leaves the loop + // idle. + { + let el = EventLoop::new(); + let interest = el.acquire_interest_handle(); + let message = el.ensure_idle_for_snapshot().unwrap_err(); + assert!( + message.contains( + "an operation that keeps the event loop alive, created with no JavaScript on the \ + stack" + ), + "{message}" + ); + drop(interest); + assert!(el.ensure_idle_for_snapshot().is_ok()); + } + + // Once recording stops, work is still reported, without an origin. + { + // SAFETY: stopping the recording has no precondition. + unsafe { js::stack::record_origins(None) }; + let el = EventLoop::new(); + eval_on( + &scope, + &el, + "late.js", + "globalThis.late = setTimeout(() => {}, 60000);", + ); + let message = el.ensure_idle_for_snapshot().unwrap_err(); + assert!( + message.contains("a pending `timeout` timer, created with no JavaScript on the stack"), + "{message}" + ); + eval_on(&scope, &el, "clear.js", "clearTimeout(globalThis.late);"); + } +} diff --git a/crates/js/src/promise.rs b/crates/js/src/promise.rs index 3d48a3fb..96c1e3f9 100644 --- a/crates/js/src/promise.rs +++ b/crates/js/src/promise.rs @@ -366,13 +366,13 @@ impl<'s> Stack<'s, Promise> { /// Stores the `Promise` in a `RootedHeap` for GC safety /// and queues the future for later execution via `drain_promises`. pub fn spawn(&self, js_promise: PromiseFuture) { - let heap_promise = RootedHeap::new(*self); - - let owner = current_future_owner(|owner_cell| owner_cell.get()); - pending_futures(|f| { - f.borrow_mut() - .push((owner, heap_promise, js_promise.future)); - }); + let pending = PendingPromise { + owner: current_future_owner(|owner_cell| owner_cell.get()), + promise: RootedHeap::new(*self), + future: js_promise.future, + origin: crate::stack::current_origin(), + }; + pending_futures(|f| f.borrow_mut().push(pending)); } } @@ -502,14 +502,24 @@ use std::pin::Pin; /// Callback that sets a resolved value on a `MutableHandleValue`. type ResolveCallback = Box FnOnce(&'s Scope<'_>) -> Result, ExnThrown>>; -/// A pending promise paired with its future, tagged with the id of the event loop that owns it (the -/// loop active when it was spawned), so concurrent per-request loops drive and settle only their own -/// futures. Id 0 means unowned (spawned with no active loop), can be driven by any loop. -pub(crate) type PendingPromise = ( - u64, - RootedHeap, - Pin + 'static>>, -); +/// A pending promise paired with its future. +pub(crate) struct PendingPromise { + /// The id of the event loop that owns the future (the loop active when it was spawned), so + /// concurrent per-request loops drive and settle only their own futures. Id 0 means unowned + /// (spawned with no active loop), and any loop can drive it. + owner: u64, + promise: RootedHeap, + future: Pin + 'static>>, + /// Where the future was spawned, if origins were being recorded then. See + /// [`crate::stack::record_origins`]. + origin: Option>, +} + +impl PendingPromise { + fn owned_by(&self, owner: u64) -> bool { + self.owner == owner || self.owner == 0 + } +} /// The outcome of an async method — either resolve with a convertible value /// or reject with an error message. @@ -664,8 +674,8 @@ fn take_pending_futures() -> Vec { pub fn cancel_pending_future(promise: Stack) -> bool { let mut removed = false; let mut drop_matching = |queue: &RefCell>| { - queue.borrow_mut().retain(|(_owner, boxed, _)| { - let matches = *boxed == promise; + queue.borrow_mut().retain(|pending| { + let matches = pending.promise == promise; removed |= matches; !matches }); @@ -688,9 +698,7 @@ pub fn cancel_pending_future(promise: Stack) -> bool { pub fn cancel_pending_futures_for(owner: u64) { debug_assert_ne!(owner, 0, "unowned futures belong to no single loop"); let drop_owned = |queue: &RefCell>| { - queue - .borrow_mut() - .retain(|(future_owner, _, _)| *future_owner != owner); + queue.borrow_mut().retain(|pending| pending.owner != owner); }; pending_futures(drop_owned); active_futures(drop_owned); @@ -710,11 +718,30 @@ pub fn cancel_all_pending_futures() { /// An event loop uses this to stay alive while *its own* async I/O (e.g. a `fetch`) is in flight, /// even with no tasks or timers — without being held alive by another request's loop's futures. pub fn has_pending_futures(owner: u64) -> bool { - let owned_by_caller = |future: &PendingPromise| future.0 == owner || future.0 == 0; + let owned_by_caller = |future: &PendingPromise| future.owned_by(owner); active_futures(|a| a.borrow().iter().any(owned_by_caller)) || pending_futures(|f| f.borrow().iter().any(owned_by_caller)) } +/// The origins of the async-promise futures owned by `owner` (or unowned) that are pending, one +/// per future, with `None` for a future spawned while origins were not being recorded. See +/// [`crate::stack::record_origins`]. +pub fn pending_future_origins(owner: u64) -> Vec>> { + let mut origins = Vec::new(); + let mut collect = |queue: &RefCell>| { + origins.extend( + queue + .borrow() + .iter() + .filter(|future| future.owned_by(owner)) + .map(|future| future.origin.clone()), + ); + }; + active_futures(&mut collect); + pending_futures(&mut collect); + origins +} + /// A promise object whose future completed, paired with its outcome — returned by /// [`poll_pending_futures`] for [`settle_completed_futures`] to settle. pub type CompletedFuture = (RootedHeap, PromiseOutcome); @@ -741,15 +768,15 @@ pub fn poll_pending_futures( let mut still_pending: Vec = Vec::with_capacity(futures.len()); let mut completed: Vec = Vec::new(); - for (future_owner, boxed, mut future) in futures { + for mut pending in futures { // Leave another loop's future for that loop's wait to poll and settle. - if future_owner != owner && future_owner != 0 { - still_pending.push((future_owner, boxed, future)); + if !pending.owned_by(owner) { + still_pending.push(pending); continue; } - match future.as_mut().poll(task_cx) { - std::task::Poll::Ready(outcome) => completed.push((boxed, outcome)), - std::task::Poll::Pending => still_pending.push((future_owner, boxed, future)), + match pending.future.as_mut().poll(task_cx) { + std::task::Poll::Ready(outcome) => completed.push((pending.promise, outcome)), + std::task::Poll::Pending => still_pending.push(pending), } } diff --git a/crates/js/src/stack.rs b/crates/js/src/stack.rs index dd6a9fef..be24d14c 100644 --- a/crates/js/src/stack.rs +++ b/crates/js/src/stack.rs @@ -15,14 +15,81 @@ //! Most functions accept an optional `*mut JSPrincipals` for security-filtered //! access. Pass `std::ptr::null_mut()` for unprivileged access. -use crate::gc::scope::Scope; +use crate::gc::scope::{RootScope, Scope}; use mozjs::gc::HandleObject; -use mozjs::jsapi::{JSObject, JSPrincipals, SavedFrameSelfHosted, StackCapture, StackFormat}; +use mozjs::jsapi::{ + JSContext as RawJSContext, JSObject, JSPrincipals, JSString, SavedFrameSelfHosted, + StackCapture, StackFormat, +}; use mozjs::rust::wrappers2; use mozjs::rust::{MutableHandleObject, MutableHandleString}; +use std::cell::Cell; +use std::rc::Rc; use super::error::ExnThrown; +crate::instance_local! { + /// The context [`current_origin`] captures stacks on, and null while origins are not being + /// recorded. + static ORIGIN_CX: Cell<*mut RawJSContext> = const { Cell::new(std::ptr::null_mut()) }; +} + +/// Record where asynchronous work is created from now on, capturing stacks on `cx`, or stop +/// recording for `None`. While recording, [`current_origin`] returns the current JS call stack. +/// +/// # Safety +/// +/// `cx` must stay a valid context until recording is stopped. +pub unsafe fn record_origins(cx: Option<*mut RawJSContext>) { + ORIGIN_CX.with(|cell| cell.set(cx.unwrap_or(std::ptr::null_mut()))); +} + +/// The current JS call stack, formatted as `Error.prototype.stack` formats one, for describing +/// where asynchronous work being created comes from. +/// +/// Returns `None` while [`record_origins`] is not recording, when no realm is entered or no +/// script is running, and when capturing the stack fails. A failed capture's exception is +/// cleared. +pub fn current_origin() -> Option> { + // The most frames an origin holds, counted from the call that created the work. + const MAX_FRAMES: u32 = 16; + + let cx = ORIGIN_CX.with(Cell::get); + // SAFETY: `record_origins`' caller keeps a non-null `cx` valid while recording. + if cx.is_null() || unsafe { mozjs::jsapi::GetCurrentRealmOrNull(cx) }.is_null() { + return None; + } + // SAFETY: `cx` is valid, as above, and has a realm entered. + let scope = unsafe { RootScope::from_current_realm(cx) }; + let frame = match capture_saved_frame(&scope, MAX_FRAMES) { + Ok(frame) => frame?, + Err(ExnThrown) => { + crate::exception::clear(&scope); + return None; + } + }; + mozjs::rooted!(in(cx) let mut formatted = std::ptr::null_mut::()); + // SAFETY: null principals request unfiltered frames, and `frame` is a rooted `SavedFrame`. + let built = unsafe { + build_stack_string( + &scope, + std::ptr::null_mut(), + frame.handle(), + formatted.handle_mut(), + 0, + StackFormat::Default, + ) + }; + if built.is_err() { + crate::exception::clear(&scope); + return None; + } + let formatted = std::ptr::NonNull::new(formatted.get())?; + let text = crate::conversion::jsstr_to_string(&scope, formatted); + let text = text.trim_end(); + (!text.is_empty()).then(|| Rc::from(text)) +} + /// Capture the current JavaScript call stack. /// /// # Safety diff --git a/crates/libstarling/src/serve_wasm.rs b/crates/libstarling/src/serve_wasm.rs index 934e7ef1..11058d2d 100644 --- a/crates/libstarling/src/serve_wasm.rs +++ b/crates/libstarling/src/serve_wasm.rs @@ -60,11 +60,6 @@ enum Startup { Pending(OwnedInvocation, core_runtime::ScriptEvaluation), /// A request is currently driving it. Concurrent requests wait. Driving, - /// The script has finished evaluating, but whatever background work it left behind is not - /// being driven yet. Only [`pre_initialize`] leaves a loop in this state: it drives evaluation - /// before the snapshot is taken, and the task that would go on driving the leftovers cannot - /// cross a snapshot, so the first request of the resumed instance starts it. - Evaluated(OwnedInvocation), /// Driven to completion (or there never was one). Done, } @@ -87,22 +82,33 @@ fn install_runtime(pair: (Rc, *mut js::native::RawJSContext, ServeTimeo /// /// A failed startup is final: this returns its error on every later call without retrying. fn runtime() -> Result<(Rc, *mut js::native::RawJSContext, ServeTimeouts), String> { + runtime_for(false) +} + +/// [`runtime`], creating a runtime configured for a Wizer snapshot if `pre_initialize` is set and +/// none exists yet. +fn runtime_for( + pre_initialize: bool, +) -> Result<(Rc, *mut js::native::RawJSContext, ServeTimeouts), String> { if let Some(pair) = RUNTIME.with(|cell| cell.borrow().clone()) { return Ok(pair); } if let Some(failure) = STARTUP_FAILURE.with(|cell| cell.borrow().clone()) { return Err(failure); } - start_runtime().inspect_err(|failure| { + start_runtime(pre_initialize).inspect_err(|failure| { STARTUP_FAILURE.with(|cell| *cell.borrow_mut() = Some(failure.clone())); }) } -/// Create the runtime for [`runtime`]. -fn start_runtime() -> Result<(Rc, *mut js::native::RawJSContext, ServeTimeouts), String> { +/// Create the runtime for [`runtime_for`]. +fn start_runtime( + pre_initialize: bool, +) -> Result<(Rc, *mut js::native::RawJSContext, ServeTimeouts), String> { // `wasmtime serve` passes the guest no arguments, so the HTTP entry point is configured // through `STARLINGMONKEY_CONFIG` instead (an empty one yields the defaults: `./index.js`). - let config = RuntimeConfig::from_env().map_err(|e| e.to_string())?; + let mut config = RuntimeConfig::from_env().map_err(|e| e.to_string())?; + config.pre_initialize = pre_initialize; config.validate_serve_timeouts()?; super::apply_pre_init_config(&config)?; crate::register_builtins(); @@ -126,12 +132,36 @@ fn start_runtime() -> Result<(Rc, *mut js::native::RawJSContext, ServeT Ok(pair) } -/// Record that this instance's state is being captured in a Wizer snapshot. The resumed -/// instance's first request runs the resume fixups (see [`fix_up_after_resume`]), which advance -/// its clocks by the monotonic clock reading taken here. Every caller is a Wizer entry point, and -/// calls this last, since a reading taken afterwards would sit past the recorded one and so still -/// be in the resumed instance's future. -pub fn mark_resumed_from_snapshot() { +/// Prepare this instance's state to be captured in a Wizer snapshot. Every caller is a Wizer +/// entry point, and calls this last. +/// +/// Buffered standard output is flushed, then wasi-libc closes every file descriptor it holds, +/// including stdio and the preopened directories. Their host handles do not exist in the resumed +/// instance, and libc opens stdio and the preopens again on first use there. Recording where +/// asynchronous work is created stops, so the resumed instance does not record it. +/// +/// The resumed instance's first request runs the resume fixups (see [`fix_up_after_resume`]), +/// which advance its clocks by the monotonic clock reading taken here. The reading is taken last, +/// since one taken afterwards would sit past the recorded one and so still be in the resumed +/// instance's future. +pub fn prepare_for_snapshot() { + unsafe extern "C" { + fn fflush(stream: *mut std::ffi::c_void) -> std::ffi::c_int; + fn __wasilibc_reset_preopens(); + } + + use std::io::Write as _; + let _ = std::io::stdout().flush(); + let _ = std::io::stderr().flush(); + // SAFETY: `fflush(NULL)` flushes every open C stdio stream. `__wasilibc_reset_preopens` takes + // libc's own locks, and nothing holds a descriptor across this call. + unsafe { + fflush(std::ptr::null_mut()); + __wasilibc_reset_preopens(); + } + // SAFETY: stopping the recording has no precondition. + unsafe { js::stack::record_origins(None) }; + RESUMED_FROM_SNAPSHOT.with(|resumed| resumed.set(true)); platform::clock::record_snapshot_reading(); } @@ -139,43 +169,43 @@ pub fn mark_resumed_from_snapshot() { /// Initializes the runtime until it's ready for Wizer snapshotting. /// /// This entails initializing the JS runtime, registering builtins, running the top-level script to -/// completion (including async work), and checking whether the result is a valid snapshot input -/// state. +/// completion, including a top-level `await`, and checking whether the result is a valid snapshot +/// input state. The script must register a `fetch` listener, and must leave no asynchronous work +/// behind once its top level has finished. pub async fn pre_initialize() -> Result<(), String> { - let (_runtime, raw_cx, _) = runtime()?; + let (_runtime, raw_cx, _) = runtime_for(true)?; let Startup::Pending(invocation, evaluation) = STARTUP.with(|cell| std::mem::replace(&mut *cell.borrow_mut(), Startup::Driving)) else { // Nothing to evaluate: `runtime` was already stood up, so this is a second call. - mark_resumed_from_snapshot(); + prepare_for_snapshot(); return Ok(()); }; - drive_startup(raw_cx, invocation.state().event_loop(), &evaluation).await; - // SAFETY: `runtime` entered the default global's realm for the process lifetime. let scope = unsafe { js::gc::scope::RootScope::from_current_realm(raw_cx) }; - // Throw an error instead of creating a snapshot that can't possibly serve requests. - if evaluated_without_listener(&scope, &evaluation) { - return Err(crate::serve_common::NO_FETCH_LISTENER.to_string()); + // The loop is driven only while the top level is unfinished, so work it merely queued, such + // as a timer, is refused below rather than run under Wizer. + if !evaluation.is_finished(&scope) { + drive_startup(raw_cx, invocation.state().event_loop(), &evaluation).await; } - // If any external async tasks are active, that means component model resources are held, making - // a snapshot impossible. - if invocation - .state() - .event_loop() - .has_active_external_async_tasks() - { + evaluation.rejection(&scope, "Script evaluation failed")?; + if !evaluation.is_finished(&scope) { return Err( - "Host I/O pending when evaluation finished. Ensure all I/O is finished by `await`ing it." + "the script's top-level `await` never settled: the event loop ran out of \ + work while it was still pending" .to_string(), ); } + // Throw an error instead of creating a snapshot that can't possibly serve requests. + if evaluated_without_listener(&scope, &evaluation) { + return Err(crate::serve_common::NO_FETCH_LISTENER.to_string()); + } + invocation.state().event_loop().ensure_idle_for_snapshot()?; // Define every standard class the global has not resolved yet, so the snapshot holds them // all and an instance restored from it defines none of them on first use. js::class::enumerate_standard_classes(&scope, scope.global().handle()) .map_err(|_| "defining the standard classes failed".to_string())?; - // Store the event loop, in case it has tasks to resume after snapshot restoration. - STARTUP.with(|cell| *cell.borrow_mut() = Startup::Evaluated(invocation)); + STARTUP.with(|cell| *cell.borrow_mut() = Startup::Done); Ok(()) } @@ -225,8 +255,6 @@ async fn ensure_started(raw_cx: *mut js::native::RawJSContext) { loop { enum Action { Drive(OwnedInvocation, core_runtime::ScriptEvaluation), - /// Evaluated before the snapshot was taken, so only its leftovers need a driver. - Keep(OwnedInvocation), Wait, Ready, } @@ -237,7 +265,6 @@ async fn ensure_started(raw_cx: *mut js::native::RawJSContext) { *state = Startup::Driving; Action::Drive(invocation, evaluation) } - Startup::Evaluated(invocation) => Action::Keep(invocation), Startup::Driving => { *state = Startup::Driving; Action::Wait @@ -247,12 +274,6 @@ async fn ensure_started(raw_cx: *mut js::native::RawJSContext) { }); match action { Action::Ready => return, - Action::Keep(invocation) => { - // The listener check and the missing-listener report already ran before the - // snapshot, where failing the check refused the snapshot outright. - keep_startup_loop_running(raw_cx, invocation); - return; - } Action::Drive(invocation, evaluation) => { let mut driving = Driving { pending: Some((invocation, evaluation)), diff --git a/scripts/test-wizer.sh b/scripts/test-wizer.sh index f05df341..7baa7509 100755 --- a/scripts/test-wizer.sh +++ b/scripts/test-wizer.sh @@ -138,21 +138,35 @@ check "a listener-less script is refused even when it awaits first" \ "no \`fetch\` listener added during evaluation" \ "$gate_output" -# Work the top level leaves running is background work, not part of evaluation: it survives into -# the snapshot as the timer it is and goes on running once the instance serves. +# A snapshot holds memory, not a running event loop, so work the top level leaves behind once it +# has finished evaluating is refused, with the stack that created it. cat >leftover.mjs <<'JS' globalThis.ticks = 0; -const timer = setInterval(() => { globalThis.ticks++; }, 10); +setInterval(() => { globalThis.ticks++; }, 10); await new Promise((resolve) => setTimeout(resolve, 50)); +addEventListener('fetch', (event) => event.respondWith(new Response('ok'))); +JS +leftover_output="$(snapshot snap-leftover.wasm "leftover.mjs" || true)" +check "background work left by the top level fails the snapshot" \ + "a pending \`interval\` timer, created at:" \ + "$leftover_output" +check "the refusal names where the work was created" \ + "leftover.mjs:2:" \ + "$leftover_output" + +# The snapshot holds no host handles: libc's stdio streams, opened by a top-level write, are closed +# before it is taken, and the resumed instance opens them again on its first write. +cat >stdio.mjs <<'JS' +console.log('written under Wizer'); addEventListener('fetch', (event) => { - clearInterval(timer); - event.respondWith(new Response('ticks=' + (globalThis.ticks > 0 ? 'running' : 'stalled'))); + console.log('written by the handler'); + event.respondWith(new Response('logged')); }); JS -snapshot snap-leftover.wasm "leftover.mjs" >/dev/null -check "background work left by the top level keeps running after a resume" \ - "ticks=running" \ - "$(serve_and_get snap-leftover.wasm "leftover.mjs" 18354)" +snapshot snap-stdio.wasm "stdio.mjs" >/dev/null +check "a snapshot taken after a top-level write can write again" \ + "logged" \ + "$(serve_and_get snap-stdio.wasm "stdio.mjs" 18354)" # Host I/O still running when evaluation ends is a handle belonging to the snapshotting process, # and the snapshot carries memory rather than handles — so it is refused rather than resumed into a @@ -178,7 +192,7 @@ await new Promise((resolve) => setTimeout(resolve, 100)); addEventListener('fetch', (event) => event.respondWith(new Response('ok'))); JS check "host I/O still in flight fails the snapshot" \ - "still had host I/O in flight" \ + "a host operation in flight, created at:" \ "$(snapshot snap-inflight.wasm "inflight.mjs" || true)" else echo "SKIP - host I/O still in flight fails the snapshot (no hanging upstream)" diff --git a/starling/src/lib.rs b/starling/src/lib.rs index 6c2683f6..4f3764dc 100644 --- a/starling/src/lib.rs +++ b/starling/src/lib.rs @@ -106,7 +106,7 @@ mod wasm_entry { } // Last, so that the clock reading it records is past every timestamp the // snapshot holds. - libstarling::serve_wasm::mark_resumed_from_snapshot(); + libstarling::serve_wasm::prepare_for_snapshot(); } } From 45b37b571e85fe89a888921dca38cca3efcc3a3f Mon Sep 17 00:00:00 2001 From: Till Schneidereit Date: Tue, 29 Sep 2026 17:55:57 +0200 Subject: [PATCH 4/9] Fail a command whose top-level await rejects or never settles Running a module as a command handed its evaluation promise to the event loop and dropped it, so a top level that rejected after awaiting a timer, or awaited a promise nothing could settle, exited with status 0 and no message. `ScriptEvaluation::settled`, called by `core_runtime::run`, the wasm `libstarling::run` and Wizer pre-initialization, checks both once the loop has run out of work. --- README.md | 2 +- crates/core-runtime/src/lib.rs | 113 +++++++++++++++++++++------ crates/libstarling/src/lib.rs | 22 ++++-- crates/libstarling/src/serve_wasm.rs | 9 +-- starling/tests/exit_status.rs | 64 +++++++++++++++ 5 files changed, 168 insertions(+), 42 deletions(-) diff --git a/README.md b/README.md index e954df82..825491a9 100644 --- a/README.md +++ b/README.md @@ -616,7 +616,7 @@ impl Fetcher { ``` The method returns the `Promise` to JS immediately, and the future is queued on -the event loop. `core_runtime::event_loop::run_to_completion`, which +the event loop. `ScriptEvaluation::run_to_completion`, which `libstarling::run` drives for you, polls it and settles the `Promise` with the future's `Ok`/`Err`. Two other constructors cover the cases `new` doesn't: `PromiseFuture::new_void` for futures resolving to `()`, and `PromiseFuture::from_value` diff --git a/crates/core-runtime/src/lib.rs b/crates/core-runtime/src/lib.rs index 434c0be6..db82e154 100644 --- a/crates/core-runtime/src/lib.rs +++ b/crates/core-runtime/src/lib.rs @@ -28,31 +28,35 @@ use crate::runtime::Runtime; /// in either ES module mode (default) or legacy script mode /// 6. Delegates to `drive_event_loop` to run the event loop. /// -/// The `drive_event_loop` callback receives the `Runtime` and -/// `InvocationState` and is responsible for driving the event loop to -/// completion using whatever executor and timer mechanism the embedding -/// provides. The invocation arrives **unregistered** from GC tracing: the -/// callback must register it once it sits at its final address, most simply -/// by wrapping it in [`invocation::InvocationGuard`] or +/// The `drive_event_loop` callback receives the `Runtime`, the +/// `InvocationState` and the script's [`ScriptEvaluation`], and is responsible +/// for driving the event loop to completion using whatever executor and timer +/// mechanism the embedding provides. The invocation arrives unregistered +/// from GC tracing: the callback must register it once it sits at its final +/// address, most simply by wrapping it in [`invocation::InvocationGuard`] or /// [`invocation::OwnedInvocation`], whose drop also handles the required -/// unregistration, and then drive [`event_loop::run_to_completion`]. +/// unregistration, and then drive [`ScriptEvaluation::run_to_completion`]. /// Driving the loop without registering leaves task-held JS objects /// invisible to the GC. /// /// On native targets, the callback typically creates an async runtime, e.g. -/// Tokio, and calls `block_on(run_to_completion(..., tokio::time::sleep))`. +/// Tokio, and calls `block_on(evaluation.run_to_completion(..., tokio::time::sleep))`. /// On WASIp3, it spawns the event loop via `wit_bindgen::spawn`. pub fn run( config: config::RuntimeConfig, drive_event_loop: impl FnOnce( std::rc::Rc, invocation::InvocationState, + &ScriptEvaluation, ) -> Result<(), String>, ) -> Result<(), String> { - match setup(config)? { - Some((runtime, invocation)) => drive_event_loop(runtime, invocation), - None => Ok(()), - } + let Some((runtime, invocation, evaluation)) = setup(config)? else { + return Ok(()); + }; + drive_event_loop(std::rc::Rc::clone(&runtime), invocation, &evaluation)?; + let settled = evaluation.settled(&runtime.default_global(), "Script evaluation failed"); + drop(evaluation); + settled } /// The content script's source and the filename to report it under: the `-e` string when there is @@ -162,6 +166,56 @@ impl ScriptEvaluation { None => Ok(()), } } + + /// Whether the script's top-level `await` rejected. + pub fn is_rejected(&self, scope: &js::gc::scope::Scope<'_>) -> bool { + self.pending + .as_ref() + .is_some_and(|promise| promise.get(scope).is_rejected()) + } + + /// Run `event_loop` until it has nothing left, or until the script's top-level `await` + /// rejects. A rejection ends the run once the current step finishes, leaving the loop's + /// remaining tasks unrun, and cancels its pending futures. + /// + /// # Safety + /// + /// `raw_cx` must be a valid JSContext pointer whose default global's realm is entered, and + /// which stays valid for the lifetime of this future. + pub async unsafe fn run_to_completion( + &self, + raw_cx: *mut js::native::RawJSContext, + event_loop: &event_loop::EventLoop, + sleep: S, + ) where + S: Fn(std::time::Duration) -> F, + F: std::future::Future, + { + // SAFETY: guaranteed by this function's caller. + unsafe { + event_loop::run_until(raw_cx, event_loop, sleep, |scope| self.is_rejected(scope)).await; + } + // SAFETY: guaranteed by this function's caller. + let scope = unsafe { js::gc::scope::RootScope::from_current_realm(raw_cx) }; + if self.is_rejected(&scope) { + event_loop.cancel_pending_futures(); + } + } + + /// `Err` with `context: reason` if the script's top-level `await` rejected, `Err` with a + /// message that the top-level `await` never settled if it did not, and `Ok` if it fulfilled. Called once the event loop + /// has run out of work. + pub fn settled(&self, scope: &js::gc::scope::Scope<'_>, context: &str) -> Result<(), String> { + self.rejection(scope, context)?; + if !self.is_finished(scope) { + return Err( + "the script's top-level `await` never settled: the event loop ran out of work \ + while it was still pending" + .to_string(), + ); + } + Ok(()) + } } /// Initialize the runtime and evaluate the script (the synchronous portion of an @@ -223,27 +277,35 @@ fn init_and_eval( /// any asynchronous work remains. /// /// Returns: -/// - `Ok(Some((runtime, invocation)))` if the script left work in the event -/// loop. The caller must call [`Runtime::register_invocation`] once the -/// invocation reaches its final (stable) location, drive -/// [`event_loop::run_to_completion`] to completion, then call -/// [`Runtime::unregister_invocation`] before dropping the runtime. +/// - `Ok(Some((runtime, invocation, evaluation)))` if the script left work in +/// the event loop. The caller must call [`Runtime::register_invocation`] once +/// the invocation reaches its final (stable) location, drive +/// [`ScriptEvaluation::run_to_completion`] to completion, then call +/// [`Runtime::unregister_invocation`] and check +/// [`ScriptEvaluation::settled`] before dropping the runtime. /// - `Ok(None)` if the script completed without scheduling any async work. -/// - `Err(_)` if the script failed to parse or threw during top-level -/// evaluation. +/// - `Err(_)` if the script failed to parse, threw during top-level +/// evaluation, or awaits a promise that nothing can settle. /// /// Splitting this out lets the wasm32 cdylib driver hold an async event-loop /// future on its own stack instead of bridging through a sync callback. pub fn setup( config: config::RuntimeConfig, -) -> Result, invocation::InvocationState)>, String> { - // Command mode drives the loop to completion, so the script's evaluation needs no separate - // wait: its promise is one more thing the loop settles on the way. - let (runtime, invocation, _evaluation) = init_and_eval(config)?; +) -> Result< + Option<( + std::rc::Rc, + invocation::InvocationState, + ScriptEvaluation, + )>, + String, +> { + let (runtime, invocation, evaluation) = init_and_eval(config)?; if !invocation.event_loop().is_alive() { - return Ok(None); + let settled = evaluation.settled(&runtime.default_global(), "Script evaluation failed"); + drop(evaluation); + return settled.map(|()| None); } - Ok(Some((runtime, invocation))) + Ok(Some((runtime, invocation, evaluation))) } /// Like [`setup`], but always returns the runtime — for **serve mode**, where the @@ -281,6 +343,7 @@ mod tests { fn noop_driver( _runtime: std::rc::Rc, _invocation: invocation::InvocationState, + _evaluation: &ScriptEvaluation, ) -> Result<(), String> { Err("script unexpectedly left the event loop alive".to_string()) } diff --git a/crates/libstarling/src/lib.rs b/crates/libstarling/src/lib.rs index e337b79e..3115c7ad 100644 --- a/crates/libstarling/src/lib.rs +++ b/crates/libstarling/src/lib.rs @@ -1,7 +1,5 @@ // SPDX-License-Identifier: Apache-2.0-WITH-LLVM-exception -use core_runtime::event_loop::run_to_completion; - // Re-export everything from core-runtime. pub use core_runtime::*; @@ -93,9 +91,8 @@ pub async fn run(config: config::RuntimeConfig) -> Result<(), String> { ); } - let (runtime, invocation) = match core_runtime::setup(config)? { - Some(pair) => pair, - None => return Ok(()), + let Some((runtime, invocation, evaluation)) = core_runtime::setup(config)? else { + return Ok(()); }; // Register `invocation` for GC tracing now that it has reached its final, @@ -119,11 +116,15 @@ pub async fn run(config: config::RuntimeConfig) -> Result<(), String> { // SAFETY: `raw_cx` is valid for the duration of the await — `scope` // keeps the `Runtime` alive and the realm entered. unsafe { - run_to_completion(raw_cx, el, platform::clock::sleep).await; + evaluation + .run_to_completion(raw_cx, el, platform::clock::sleep) + .await; } + let settled = evaluation.settled(&scope, "Script evaluation failed"); + drop(evaluation); drop(scope); - Ok(()) + settled } /// Drive the event loop on native targets using a tokio current-thread @@ -132,6 +133,7 @@ pub async fn run(config: config::RuntimeConfig) -> Result<(), String> { fn drive_event_loop_native( runtime: std::rc::Rc, invocation: invocation::InvocationState, + evaluation: &core_runtime::ScriptEvaluation, ) -> Result<(), String> { // Enable both the timer and IO drivers: timers back `setTimeout`, and IO backs // `fetch`'s async HTTP transport (which uses tokio TCP sockets). @@ -160,7 +162,11 @@ fn drive_event_loop_native( // Since the caller owns both and `block_on` runs synchronously on // the same thread, this is guaranteed. let raw_cx = unsafe { scope.raw_cx_no_gc() }; - unsafe { run_to_completion(raw_cx, el, tokio::time::sleep).await } + unsafe { + evaluation + .run_to_completion(raw_cx, el, tokio::time::sleep) + .await + } }); drop(scope); diff --git a/crates/libstarling/src/serve_wasm.rs b/crates/libstarling/src/serve_wasm.rs index 11058d2d..5c22feae 100644 --- a/crates/libstarling/src/serve_wasm.rs +++ b/crates/libstarling/src/serve_wasm.rs @@ -188,14 +188,7 @@ pub async fn pre_initialize() -> Result<(), String> { if !evaluation.is_finished(&scope) { drive_startup(raw_cx, invocation.state().event_loop(), &evaluation).await; } - evaluation.rejection(&scope, "Script evaluation failed")?; - if !evaluation.is_finished(&scope) { - return Err( - "the script's top-level `await` never settled: the event loop ran out of \ - work while it was still pending" - .to_string(), - ); - } + evaluation.settled(&scope, "Script evaluation failed")?; // Throw an error instead of creating a snapshot that can't possibly serve requests. if evaluated_without_listener(&scope, &evaluation) { return Err(crate::serve_common::NO_FETCH_LISTENER.to_string()); diff --git a/starling/tests/exit_status.rs b/starling/tests/exit_status.rs index 48008b57..ead3bba8 100644 --- a/starling/tests/exit_status.rs +++ b/starling/tests/exit_status.rs @@ -108,6 +108,70 @@ fn throwing_script_exits_with_code_one() { ); } +/// A top level that rejects once a timer has fired, after evaluation handed off to the event +/// loop, reports the error and exits with 1. +#[test] +fn late_top_level_rejection_exits_with_code_one() { + let dir = tempfile::tempdir().unwrap(); + let main = dir.path().join("main.mjs"); + std::fs::write( + &main, + "await new Promise((resolve) => setTimeout(resolve, 10));\n\ + throw new Error('late failure');", + ) + .unwrap(); + let out = Command::new(env!("CARGO_BIN_EXE_starlingmonkey")) + .arg(&main) + .output() + .expect("failed to run starling"); + let stderr = String::from_utf8_lossy(&out.stderr); + assert_eq!(out.status.code(), Some(1), "stderr: {stderr}"); + assert!(stderr.contains("late failure"), "stderr: {stderr}"); +} + +/// A top level that rejects while an interval keeps the event loop busy reports the error and +/// exits with 1, without waiting for the interval. +#[test] +fn late_rejection_with_work_left_exits_with_code_one() { + let dir = tempfile::tempdir().unwrap(); + let main = dir.path().join("main.mjs"); + std::fs::write( + &main, + "setInterval(() => {}, 5);\n\ + await new Promise((resolve) => setTimeout(resolve, 10));\n\ + throw new Error('late failure');", + ) + .unwrap(); + let out = Command::new(env!("CARGO_BIN_EXE_starlingmonkey")) + .arg(&main) + .output() + .expect("failed to run starling"); + let stderr = String::from_utf8_lossy(&out.stderr); + assert_eq!(out.status.code(), Some(1), "stderr: {stderr}"); + assert!(stderr.contains("late failure"), "stderr: {stderr}"); +} + +/// A top-level `await` on a promise nothing settles exits with 1, whether or not the event loop +/// had other work first. +#[test] +fn unsettled_top_level_await_exits_with_code_one() { + for source in [ + "await new Promise(() => {});", + "setTimeout(() => {}, 10);\nawait new Promise(() => {});", + ] { + let out = Command::new(env!("CARGO_BIN_EXE_starlingmonkey")) + .args(["-e", source]) + .output() + .expect("failed to run starling"); + let stderr = String::from_utf8_lossy(&out.stderr); + assert_eq!(out.status.code(), Some(1), "{source}: stderr: {stderr}"); + assert!( + stderr.contains("never settled"), + "{source}: stderr: {stderr}" + ); + } +} + /// `--help` exits with 0 and an unknown flag with clap's usage-error status 2. #[test] fn argument_errors_use_clap_exit_statuses() { From 47694070a2a52d4f0ca7a5a054ef256ce1fbff05 Mon Sep 17 00:00:00 2001 From: Till Schneidereit Date: Tue, 29 Sep 2026 15:05:58 +0200 Subject: [PATCH 5/9] Print console output by level and report unhandled rejections `console.log`, `info` and `debug` print the bare message on stdout, and `warn` and `error` print it on stderr, prefixed with the level. Every level used to go to stdout behind a prefix. `js::jobs` tracks promises that are rejected while no handler is attached, and `run_microtasks` reports each one still unhandled after the drain on stderr, as `Uncaught (in promise)` with the reason's message, location and stack. A module's evaluation promise is marked handled when evaluation returns it, since the caller reports its rejection itself. Additionally, report more details, including the stack, on where a top-level error was thrown. --- README.md | 14 ++- .../cpp-builtins/cpp/builtins/console.cpp | 53 +++++------- crates/core-runtime/src/event_loop.rs | 29 +++++-- crates/core-runtime/src/module.rs | 5 +- crates/core-runtime/src/runtime.rs | 3 + crates/js/src/error.rs | 42 ++++++++- crates/js/src/exception.rs | 5 +- crates/js/src/jobs.rs | 74 +++++++++++++++- crates/js/src/module.rs | 13 ++- starling/tests/console_output.rs | 86 +++++++++++++++++++ starling/tests/exit_status.rs | 16 ++++ tests/wpt-harness/run-wpt.mjs | 6 +- 12 files changed, 293 insertions(+), 53 deletions(-) create mode 100644 starling/tests/console_output.rs diff --git a/README.md b/README.md index 825491a9..a724013f 100644 --- a/README.md +++ b/README.md @@ -149,9 +149,17 @@ is `undefined` at the top level. **Legacy script mode** (`--legacy-script`) — sloppy mode, no `import`/`export`, `this` is the global object. -**Exit status** — `0` when the script and all the work it scheduled complete, `1` -when the script throws or an initializer, the event loop or the runtime fails (the -error is printed to stderr), and `2` for invalid command-line arguments. +**Output** — `console.log`, `console.info` and `console.debug` print to stdout, +and `console.warn` and `console.error` print to stderr with a `Warn:` or `Error:` +prefix. An exception a timer or other task throws is reported on stderr as +`Uncaught` with its message and stack, and a promise rejected with no handler as +`Uncaught (in promise)` with the reason's. + +**Exit status** — `0` when the script and all the work it scheduled complete, +including when a promise rejected with no handler. `1` when the script throws, +its top-level `await` rejects or never settles, or an initializer, the event loop +or the runtime fails, with the error printed to stderr. `2` for invalid +command-line arguments. --- diff --git a/crates/builtins/cpp-builtins/cpp/builtins/console.cpp b/crates/builtins/cpp-builtins/cpp/builtins/console.cpp index 4d3a9c69..33f9c1e9 100644 --- a/crates/builtins/cpp-builtins/cpp/builtins/console.cpp +++ b/crates/builtins/cpp-builtins/cpp/builtins/console.cpp @@ -405,33 +405,24 @@ JS::Result ToSource(JSContext *cx, std::string &sourceOut, JS::Hand namespace builtins::web::console { +// `log`, `info` and `debug` print the bare message on stdout. `warn` and `error` print it on +// stderr, prefixed with the level. __attribute__((weak)) void builtin_impl_console_log(Console::LogType log_ty, const char *msg) { - const char *prefix = nullptr; switch (log_ty) { - case Console::LogType::Log: - prefix = "Log"; - break; - case Console::LogType::Debug: - prefix = "Debug"; - break; - case Console::LogType::Info: - prefix = "Info"; - break; case Console::LogType::Warn: - prefix = "Warn"; - break; + fprintf(stderr, "Warn: %s\n", msg); + fflush(stderr); + return; case Console::LogType::Error: - prefix = "Error"; - break; + fprintf(stderr, "Error: %s\n", msg); + fflush(stderr); + return; default: - prefix = ""; - break; + fprintf(stdout, "%s\n", msg); + fflush(stdout); + return; } - MOZ_ASSERT(prefix); - - fprintf(stdout, "%s: %s\n", prefix, msg); - fflush(stdout); } template @@ -470,9 +461,9 @@ static bool console_out(JSContext *cx, unsigned argc, JS::Value *vp) { static bool assert_(JSContext *cx, unsigned argc, JS::Value *vp) { JS::CallArgs args = CallArgsFromVp(argc, vp); args.rval().setUndefined(); - auto condition = args.get(0).toBoolean(); + bool condition = JS::ToBoolean(args.get(0)); // 1. If condition is true, return. - if (!condition) { + if (condition) { return true; } @@ -486,11 +477,15 @@ static bool assert_(JSContext *cx, unsigned argc, JS::Value *vp) { // 4.3. Otherwise: // 4.3.1. Let concat be the concatenation of message, U+003A (:), U+0020 // SPACE, and first. 4.3.2. Set data[0] to concat. + // The data are the arguments after the condition. A string first datum is + // joined to the message with ": ", and any other one follows it after a space. auto length = args.length(); if (length > 1) { - message += ": "; + if (args.get(1).isString()) { + message += ":"; + } JS::RootedObjectVector visitedObjects(cx); - for (int i = 0; i < length; i++) { + for (unsigned i = 1; i < length; i++) { JS::HandleValue arg = args.get(i); std::string source; auto result = ToSource(cx, source, arg, &visitedObjects); @@ -498,15 +493,11 @@ static bool assert_(JSContext *cx, unsigned argc, JS::Value *vp) { return false; } // strip quotes for direct string logs - if (source[0] == '"' && source[source.length() - 1] == '"') { + if (source.length() >= 2 && source[0] == '"' && source[source.length() - 1] == '"') { source = source.substr(1, source.length() - 2); } - if (!message.empty()) { - message += " "; - message += source; - } else { - message += source; - } + message += " "; + message += source; } } diff --git a/crates/core-runtime/src/event_loop.rs b/crates/core-runtime/src/event_loop.rs index 8a4097ea..1f6aac13 100644 --- a/crates/core-runtime/src/event_loop.rs +++ b/crates/core-runtime/src/event_loop.rs @@ -978,11 +978,15 @@ pub async unsafe fn run_until( /// Handle and clear a pending JS exception. /// -/// Prints the exception to stderr and clears the pending state, -/// allowing the event loop to continue. This is called by [`EventLoop::step`] -/// when a task or microtask throws. +/// Prints the exception to stderr as `Uncaught` followed by its message, +/// location and stack, and clears the pending state, allowing the event loop to +/// continue. This is called by [`EventLoop::step`] when a task or microtask +/// throws. fn handle_and_clear_exception(scope: &Scope<'_>) { - js::exception::report_and_clear(scope, "event_loop"); + if js::exception::is_pending(scope) { + let captured = js::error::ExnThrown::capture(scope); + eprintln!("Uncaught {}", captured.to_string().trim_end()); + } } impl EventLoop { @@ -1088,7 +1092,8 @@ impl Default for EventLoop { /// then clears the weak-reference set for the current "turn". /// /// Any `FinalizationRegistry` cleanup a collection queued runs first, so a -/// callback's own microtasks are drained in the same turn. +/// callback's own microtasks are drained in the same turn. A promise rejected +/// with no handler attached by the end of the drain is reported on stderr. pub fn run_microtasks(scope: &Scope<'_>) { debug_assert!( !js::exception::is_pending(scope), @@ -1097,4 +1102,18 @@ pub fn run_microtasks(scope: &Scope<'_>) { crate::finalization::run_pending(scope); jobs::run_jobs(scope); // Weak-ref set is cleared by run_jobs. + report_unhandled_rejections(scope); +} + +/// Report each promise that was rejected with no handler attached, and still has none, on +/// stderr, with the rejection reason's message and stack. +pub fn report_unhandled_rejections(scope: &Scope<'_>) { + for promise in jobs::take_unhandled_rejections(scope) { + let Some(reason) = promise.result(scope) else { + continue; + }; + js::exception::set_pending(scope, reason, js::native::ExceptionStackBehavior::Capture); + let captured = js::error::ExnThrown::capture(scope); + eprintln!("Uncaught (in promise) {}", captured.to_string().trim_end()); + } } diff --git a/crates/core-runtime/src/module.rs b/crates/core-runtime/src/module.rs index 4b81a842..b48a6ade 100644 --- a/crates/core-runtime/src/module.rs +++ b/crates/core-runtime/src/module.rs @@ -253,10 +253,7 @@ pub(crate) fn rejection_message( return context.to_string(); }; js::exception::set_pending(scope, reason, js::native::ExceptionStackBehavior::Capture); - match ExnThrown::capture(scope).message { - Some(message) => format!("{context}: {message}"), - None => context.to_string(), - } + ExnThrown::capture(scope).with_context(context) } // ============================================================================ diff --git a/crates/core-runtime/src/runtime.rs b/crates/core-runtime/src/runtime.rs index 4b7f9bcf..22651e6b 100644 --- a/crates/core-runtime/src/runtime.rs +++ b/crates/core-runtime/src/runtime.rs @@ -290,6 +290,7 @@ impl Runtime { // Create the default global and register builtins. drop(rt.new_global()); + js::jobs::track_unhandled_rejections(&rt.default_global()); if config.pre_initialize { let scope = rt.default_global(); // SAFETY: the context lives as long as `rt`, whose `Drop` stops the recording. @@ -465,6 +466,8 @@ impl Drop for Runtime { fn drop(&mut self) { // SAFETY: stopping the recording has no precondition. unsafe { js::stack::record_origins(None) }; + // The tracked promises are rooted, so they go while the context is alive. + js::jobs::clear_unhandled_rejections(); // Futures left by loops that were never canceled, and those spawned with no loop active, // have to go while the context is alive. js::promise::cancel_all_pending_futures(); diff --git a/crates/js/src/error.rs b/crates/js/src/error.rs index 8e773baa..10fef11b 100644 --- a/crates/js/src/error.rs +++ b/crates/js/src/error.rs @@ -147,8 +147,23 @@ impl ExnThrown { } } - // No error report (a primitive throw like `throw "boom"`, or a non-`Error` object without engine error data): stringify the value itself. - // Any attached stack extracted above is kept. + // No error report (a primitive throw like `throw "boom"`, or a non-`Error` object + // without engine error data): stringify the value itself. + // If an attached stack was extracted above, it gets added to the captured error. + // Otherwise, the `stack` property is read from the object and and attached. + if stack.is_none() && exc_val.is_object() { + let exc_obj = Object::from_value(scope, exc_val).unwrap(); + match exc_obj.get_property(scope, c"stack") { + Ok(value) if value.is_string() => { + stack = crate::string::Str::from_value(scope, value) + .ok() + .and_then(|s| s.to_utf8(scope).ok()) + .filter(|s: &String| !s.is_empty()); + } + Ok(_) => {} + Err(ExnThrown) => crate::exception::clear(scope), + } + } let message = match crate::string::Str::from_value(scope, exc_val) { Ok(s) => s .to_utf8(scope) @@ -205,8 +220,17 @@ impl fmt::Display for CapturedError { } else { write!(f, "JavaScript exception")?; } + // The location is left out when the stack's first frame names it. if let Some(file) = &self.filename { - write!(f, " at {file}:{}:{}", self.lineno, self.column)?; + let location = format!("{file}:{}:{}", self.lineno, self.column); + let in_stack = self + .stack + .as_deref() + .and_then(|stack| stack.lines().next()) + .is_some_and(|frame| frame.ends_with(&location)); + if !in_stack { + write!(f, " at {location}")?; + } } if let Some(s) = &self.stack { write!(f, "\n{s}")?; @@ -217,6 +241,18 @@ impl fmt::Display for CapturedError { impl std::error::Error for CapturedError {} +impl CapturedError { + /// `context: error`, with the error as [`Display`](fmt::Display) formats it, or `context` + /// alone if nothing was captured. + pub fn with_context(&self, context: &str) -> String { + if self.message.is_none() && self.filename.is_none() && self.stack.is_none() { + return context.to_string(); + } + let error = self.to_string(); + format!("{context}: {}", error.trim_end()) + } +} + // --------------------------------------------------------------------------- // Throw helpers // --------------------------------------------------------------------------- diff --git a/crates/js/src/exception.rs b/crates/js/src/exception.rs index 7fe32cdf..ab6e6b2b 100644 --- a/crates/js/src/exception.rs +++ b/crates/js/src/exception.rs @@ -92,7 +92,10 @@ pub fn report_and_clear(scope: &Scope<'_>, context: &str) { } // `capture` clears the pending exception as part of extracting it. let captured = ExnThrown::capture(scope); - eprintln!("[{context}] Uncaught exception: {captured}"); + eprintln!( + "[{context}] Uncaught exception: {}", + captured.to_string().trim_end() + ); } /// Get the `JSErrorReport` from an Error object. diff --git a/crates/js/src/jobs.rs b/crates/js/src/jobs.rs index 75f64724..6a20c1e4 100644 --- a/crates/js/src/jobs.rs +++ b/crates/js/src/jobs.rs @@ -16,11 +16,22 @@ //! jobs::run_jobs(cx); //! ``` -use crate::gc::scope::Scope; +use std::cell::RefCell; + +use crate::gc::handle::RootedHeap; +use crate::gc::scope::{RootScope, Scope}; +use mozjs::jsapi::{JSContext as RawJSContext, PromiseRejectionHandlingState}; use mozjs::rust::wrappers2; use super::error::ExnThrown; +crate::instance_local! { + /// Promises rejected while no handler was attached, in rejection order, until + /// [`take_unhandled_rejections`] takes them. A promise that gets a handler first is removed. + static UNHANDLED_REJECTIONS: RefCell>> = + const { RefCell::new(Vec::new()) }; +} + /// Enable SpiderMonkey's built-in internal job queue. /// /// This is the simplest way to get Promise resolution working. Must be called @@ -74,3 +85,64 @@ pub fn stop_draining(scope: &Scope<'_>) { pub fn clear_kept_objects(scope: &Scope<'_>) { unsafe { wrappers2::ClearKeptObjects(scope.cx()) } } + +/// Track promises that are rejected while no handler is attached, for +/// [`take_unhandled_rejections`]. Call once per context. +pub fn track_unhandled_rejections(scope: &Scope<'_>) { + // SAFETY: `track_rejection` has the callback signature, and takes no data. + unsafe { + wrappers2::SetPromiseRejectionTrackerCallback( + scope.cx(), + Some(track_rejection), + std::ptr::null_mut(), + ) + } +} + +/// The engine's rejection tracker: records a promise rejected with no handler, and forgets it +/// again once a handler is attached. +unsafe extern "C" fn track_rejection( + cx: *mut RawJSContext, + _muted_errors: bool, + promise: mozjs::jsapi::HandleObject, + state: PromiseRejectionHandlingState, + _data: *mut std::ffi::c_void, +) { + // SAFETY: the engine calls the tracker with a valid context. A realm is entered whenever a + // promise is rejected or gets a handler, and the check covers any other call. + if unsafe { mozjs::jsapi::GetCurrentRealmOrNull(cx) }.is_null() { + return; + } + // SAFETY: a realm is entered, as checked above. + let scope = unsafe { RootScope::from_current_realm(cx) }; + // SAFETY: the engine passes a rooted handle to a live promise. + let Some(promise) = (unsafe { crate::Object::from_raw(&scope, promise.get()) }) + .and_then(|object| object.cast::().ok()) + else { + return; + }; + UNHANDLED_REJECTIONS.with(|list| { + let mut list = list.borrow_mut(); + match state { + PromiseRejectionHandlingState::Unhandled => list.push(RootedHeap::new(promise)), + PromiseRejectionHandlingState::Handled => list.retain(|tracked| *tracked != promise), + } + }); +} + +/// The promises that were rejected with no handler attached since the last call, and still have +/// none, in rejection order. +pub fn take_unhandled_rejections<'s>(scope: &'s Scope<'_>) -> Vec> { + let taken = UNHANDLED_REJECTIONS.with(|list| std::mem::take(&mut *list.borrow_mut())); + taken + .iter() + .map(|tracked| tracked.get(scope)) + .filter(|promise| promise.is_rejected() && !promise.is_handled()) + .collect() +} + +/// Forget every tracked rejection. The tracked promises are rooted, so this must run before the +/// context is destroyed. +pub fn clear_unhandled_rejections() { + UNHANDLED_REJECTIONS.with(|list| list.borrow_mut().clear()); +} diff --git a/crates/js/src/module.rs b/crates/js/src/module.rs index 39ea1919..d2e08f01 100644 --- a/crates/js/src/module.rs +++ b/crates/js/src/module.rs @@ -78,7 +78,9 @@ pub fn link(scope: &Scope<'_>, module_record: Object) -> Result<(), ExnThrown> { /// Evaluate a linked module. /// -/// Returns the evaluation result (typically a promise for top-level await). +/// Returns the evaluation result (typically a promise for top-level await). A +/// promise is marked handled, since the caller consumes its outcome, so its +/// rejection is not reported as an unhandled one. pub fn evaluate<'r>( scope: &'r Scope<'_>, module_record: Object, @@ -88,7 +90,14 @@ pub fn evaluate<'r>( wrappers2::ModuleEvaluate(scope.cx_mut(), module_record.handle(), rval.reborrow()) }; ExnThrown::check(ok)?; - Ok(rval.handle()) + let rval = rval.handle(); + if let Some(promise) = Object::from_value(scope, rval) + .ok() + .and_then(|object| object.cast::().ok()) + { + promise.set_any_is_handled(scope)?; + } + Ok(rval) } /// Throw if module evaluation failed. diff --git a/starling/tests/console_output.rs b/starling/tests/console_output.rs new file mode 100644 index 00000000..c5b084ba --- /dev/null +++ b/starling/tests/console_output.rs @@ -0,0 +1,86 @@ +// SPDX-License-Identifier: Apache-2.0-WITH-LLVM-exception + +//! End-to-end checks of what the `starling` binary writes to stdout and stderr. + +#![cfg(not(target_arch = "wasm32"))] + +use std::process::{Command, Output}; + +/// Run `source` as the module `main.mjs`, and return the process output. +fn run_module(source: &str) -> Output { + let dir = tempfile::tempdir().unwrap(); + let main = dir.path().join("main.mjs"); + std::fs::write(&main, source).unwrap(); + Command::new(env!("CARGO_BIN_EXE_starlingmonkey")) + .arg(&main) + .output() + .expect("failed to run starling") +} + +/// `log`, `info` and `debug` print the bare message on stdout. `warn` and +/// `error` print it on stderr, prefixed with the level. +#[test] +fn console_levels_write_to_their_streams() { + let out = run_module( + "console.log('a', 1); console.info('b'); console.debug('c');\n\ + console.warn('d'); console.error('e');", + ); + assert_eq!(out.status.code(), Some(0)); + assert_eq!(String::from_utf8_lossy(&out.stdout), "a 1\nb\nc\n"); + assert_eq!(String::from_utf8_lossy(&out.stderr), "Warn: d\nError: e\n"); +} + +/// `console.assert` prints nothing when its condition holds, and otherwise prints +/// "Assertion failed" and its data on stderr. +#[test] +fn console_assert_reports_failed_assertions() { + let out = run_module( + "console.assert(true, 'not printed');\n\ + console.assert(1, 'not printed either');\n\ + console.assert(false, 'failed', 1);\n\ + console.assert(0);\n\ + console.assert('', 2);", + ); + assert_eq!(out.status.code(), Some(0)); + assert_eq!( + String::from_utf8_lossy(&out.stderr), + "Error: Assertion failed: failed 1\nError: Assertion failed\nError: Assertion failed 2\n" + ); +} + +/// A promise rejected with no handler attached by the end of the microtask +/// drain is reported on stderr with its reason and location, and one that gets +/// a handler in the same turn is not. Neither changes the exit status. +#[test] +fn unhandled_rejections_are_reported() { + let out = run_module( + "Promise.reject(new Error('never handled'));\n\ + Promise.reject(new Error('handled')).catch(() => {});\n\ + setTimeout(async () => { throw new Error('from a timer'); }, 0);", + ); + assert_eq!(out.status.code(), Some(0)); + let stderr = String::from_utf8_lossy(&out.stderr); + assert!( + stderr.contains("Uncaught (in promise) never handled\n") && stderr.contains("main.mjs:1:"), + "{stderr}" + ); + assert!( + stderr.contains("Uncaught (in promise) from a timer\n") && stderr.contains("main.mjs:3:"), + "{stderr}" + ); + assert!( + !stderr.contains("Uncaught (in promise) handled"), + "{stderr}" + ); +} + +/// A top-level `await` that rejects fails the script once, as an evaluation +/// failure, and is not also reported as an unhandled rejection. +#[test] +fn top_level_rejection_is_reported_once() { + let out = run_module("await Promise.reject(new Error('top level'));"); + assert_eq!(out.status.code(), Some(1)); + let stderr = String::from_utf8_lossy(&out.stderr); + assert!(stderr.contains("top level"), "{stderr}"); + assert!(!stderr.contains("Uncaught (in promise)"), "{stderr}"); +} diff --git a/starling/tests/exit_status.rs b/starling/tests/exit_status.rs index ead3bba8..c803b263 100644 --- a/starling/tests/exit_status.rs +++ b/starling/tests/exit_status.rs @@ -108,6 +108,22 @@ fn throwing_script_exits_with_code_one() { ); } +/// A top-level throw in a module file reports where it was thrown. +#[test] +fn top_level_throw_reports_its_location() { + let dir = tempfile::tempdir().unwrap(); + let main = dir.path().join("main.mjs"); + std::fs::write(&main, "const x = 1;\nthrow new Error('located');").unwrap(); + let out = Command::new(env!("CARGO_BIN_EXE_starlingmonkey")) + .arg(&main) + .output() + .expect("failed to run starling"); + let stderr = String::from_utf8_lossy(&out.stderr); + assert_eq!(out.status.code(), Some(1), "stderr: {stderr}"); + assert!(stderr.contains("located"), "stderr: {stderr}"); + assert!(stderr.contains("main.mjs:2:"), "stderr: {stderr}"); +} + /// A top level that rejects once a timer has fired, after evaluation handed off to the event /// loop, reports the error and exits with 1. #[test] diff --git a/tests/wpt-harness/run-wpt.mjs b/tests/wpt-harness/run-wpt.mjs index 7e416131..6ccb1055 100644 --- a/tests/wpt-harness/run-wpt.mjs +++ b/tests/wpt-harness/run-wpt.mjs @@ -965,11 +965,11 @@ async function runSingleTest(testPath) { } // Parse results from stdout — look for the WPT_RESULTS_JSON marker. + const marker = "WPT_RESULTS_JSON:"; const lines = stdout.split("\n"); for (const line of lines) { - if (line.startsWith("Log: WPT_RESULTS_JSON:")) { - const json = line.slice("Log: WPT_RESULTS_JSON:".length); - return { results: JSON.parse(json), stdout, stderr }; + if (line.startsWith(marker)) { + return { results: JSON.parse(line.slice(marker.length)), stdout, stderr }; } } From 1f5408ac064e6a1b6a0c749ccd4b6e0be0a52b19 Mon Sep 17 00:00:00 2001 From: Till Schneidereit Date: Tue, 29 Sep 2026 15:43:13 +0200 Subject: [PATCH 6/9] Use workspace dependencies for heck and smallvec `heck` and `smallvec` are declared once in the workspace manifest, and `starling-macro` and `web-streams` refer to those declarations instead of naming versions of their own. --- Cargo.toml | 1 + crates/builtins/web-streams/Cargo.toml | 2 +- crates/starling-macro/Cargo.toml | 2 +- 3 files changed, 3 insertions(+), 2 deletions(-) diff --git a/Cargo.toml b/Cargo.toml index 6a9e3f04..060c7ce6 100644 --- a/Cargo.toml +++ b/Cargo.toml @@ -28,6 +28,7 @@ form_urlencoded = "1.2" futures-channel = "0.3" futures-lite = "2" futures-util = "0.3" +heck = "0.5" http = "1" http-body = "1" http-body-util = "0.1" diff --git a/crates/builtins/web-streams/Cargo.toml b/crates/builtins/web-streams/Cargo.toml index 678630cf..e0aa2df2 100644 --- a/crates/builtins/web-streams/Cargo.toml +++ b/crates/builtins/web-streams/Cargo.toml @@ -11,7 +11,7 @@ edition.workspace = true test = false [dependencies] -smallvec = "1.16" +smallvec.workspace = true core-runtime.workspace = true js.workspace = true web-globals.workspace = true diff --git a/crates/starling-macro/Cargo.toml b/crates/starling-macro/Cargo.toml index 13b2b5e5..5abeaa10 100644 --- a/crates/starling-macro/Cargo.toml +++ b/crates/starling-macro/Cargo.toml @@ -15,4 +15,4 @@ workspace = true syn = { version = "2", features = ["full", "extra-traits", "visit-mut"] } quote = "1" proc-macro2 = "1" -heck = "0.5" +heck.workspace = true From c9aeb38b7eb54bed74d0da4d6832106cb45dd7e7 Mon Sep 17 00:00:00 2001 From: Till Schneidereit Date: Tue, 29 Sep 2026 16:11:05 +0200 Subject: [PATCH 7/9] Read incoming wasi:http requests synchronously `platform::http::read_incoming_request` only calls sync host functions, so can be sync itself as well. --- crates/libstarling/src/serve_wasm.rs | 15 +++++++-------- crates/platform/src/http_wasm.rs | 2 +- 2 files changed, 8 insertions(+), 9 deletions(-) diff --git a/crates/libstarling/src/serve_wasm.rs b/crates/libstarling/src/serve_wasm.rs index 5c22feae..0e2f89af 100644 --- a/crates/libstarling/src/serve_wasm.rs +++ b/crates/libstarling/src/serve_wasm.rs @@ -385,14 +385,13 @@ async fn dispatch_request( timeouts: ServeTimeouts, ) -> Result { let clock = timeouts.start_clock(); - let (method, url, headers, body) = - match platform::http::read_incoming_request(wasi_request).await { - Ok(parts) => parts, - Err(e) => { - eprintln!("serve: the request's headers could not be read: {e:?}"); - return Ok(error_response(400, "Bad Request", clock.error_body_time_limit()).0); - } - }; + let (method, url, headers, body) = match platform::http::read_incoming_request(wasi_request) { + Ok(parts) => parts, + Err(e) => { + eprintln!("serve: the request's headers could not be read: {e:?}"); + return Ok(error_response(400, "Bad Request", clock.error_body_time_limit()).0); + } + }; // `wasi:http` always provides a body stream, even for a request that cannot have a body. In // that case, we drop the body, so `request.body` correctly returns `null`. let (has_body, content_length) = body_framing(&method, &headers); diff --git a/crates/platform/src/http_wasm.rs b/crates/platform/src/http_wasm.rs index 4bb35562..ad96e4b0 100644 --- a/crates/platform/src/http_wasm.rs +++ b/crates/platform/src/http_wasm.rs @@ -419,7 +419,7 @@ pub async fn send(request: Request) -> Result { /// Read an incoming `wasi:http` request into its parts (method, full URL, headers, body). /// /// Returns `Err` for a field the host accepted that `http` cannot represent. -pub async fn read_incoming_request( +pub fn read_incoming_request( request: WasiRequest, ) -> Result<(String, String, http::HeaderMap, IncomingBody), ErrorCode> { let method = string_of_method(request.get_method()); From 8ddc95502216be13389e9305e5470498c77ceb64 Mon Sep 17 00:00:00 2001 From: Till Schneidereit Date: Tue, 29 Sep 2026 16:17:27 +0200 Subject: [PATCH 8/9] Register the entry module under its file's path The entry module needs to be registered in the module registry same as any other module. Otherwise, importing it in other modules creates a second instance. This is also meaningfully simplifying module registration. --- crates/core-runtime/src/module.rs | 60 +++++++++++++++++++++---------- starling/tests/modules.rs | 43 ++++++++++++++++++++++ 2 files changed, 84 insertions(+), 19 deletions(-) create mode 100644 starling/tests/modules.rs diff --git a/crates/core-runtime/src/module.rs b/crates/core-runtime/src/module.rs index b48a6ade..72c17e8b 100644 --- a/crates/core-runtime/src/module.rs +++ b/crates/core-runtime/src/module.rs @@ -506,6 +506,37 @@ fn resolve_file_module<'r>( Ok(module) } +/// Register `module`, an entry module compiled from the source of the file at +/// `path`, as that file's module, the way [`resolve_file_module`] registers a +/// file it compiles: a module importing the file gets `module`, and `module`'s +/// relative imports resolve against the file's directory. A relative `path` is +/// resolved against the current directory. +pub fn register_entry_module( + scope: &Scope<'_>, + module: Object<'_>, + path: &Path, +) -> Result<(), ExnThrown> { + let path = if path.is_absolute() { + path.to_path_buf() + } else { + std::env::current_dir().unwrap_or_default().join(path) + }; + let canonical = std::fs::canonicalize(&path).unwrap_or_else(|_| lexically_normalize(path)); + let key = canonical.to_string_lossy().to_string(); + let path_str = js::JSString::from_str(scope, &key).map_err(|_| ExnThrown)?; + // SAFETY: `module` is a rooted module object, and `path_str` is rooted. + unsafe { SetModulePrivate(module.as_raw(), &path_str.as_value()) }; + registry(|reg| { + reg.borrow_mut().insert( + key, + ModuleEntry { + module_obj: Heap::from(module), + }, + ); + }); + Ok(()) +} + /// Why [`resolve_file_module`] could not produce a module. enum ModuleLoadError { /// Resolving or reading the module failed. The message describes why. @@ -881,26 +912,17 @@ pub unsafe fn evaluate_module<'s>( let module = unsafe { js::module::compile_module(scope, options.ptr, &mut src) } .map_err(|_| ExnThrown)?; - // If the filename is a real path, store its absolute form in the module - // private: the load hook reads it to resolve this entry's relative - // imports against the entry's own directory. Pathless entries (eval - // scripts, synthetic filenames) leave the private unset and fall back to - // the loader's base path. The empty-path guard prevents WASI from - // treating `Path::new("").exists()` as a valid root directory. + // If the filename is a path in an existing directory, or a file in the + // current directory, register the entry as that file's module. Other entries + // (eval scripts, synthetic filenames) stay unregistered and resolve their + // relative imports against the loader's base path. let path = Path::new(filename); - if let Some(parent) = path.parent() { - if !parent.as_os_str().is_empty() && parent.exists() { - let abs = if path.is_absolute() { - path.to_path_buf() - } else { - std::env::current_dir().unwrap_or_default().join(path) - }; - let path_str = - js::JSString::from_str(scope, &abs.to_string_lossy()).map_err(|_| ExnThrown)?; - // SAFETY: `module` is the rooted module object `compile_module` returned, and `path_str` is - // rooted. - unsafe { SetModulePrivate(module.as_raw(), &path_str.as_value()) }; - } + let is_path = match path.parent() { + Some(parent) if !parent.as_os_str().is_empty() => parent.exists(), + _ => path.is_file(), + }; + if is_path { + register_entry_module(scope, module, path)?; } js::module::load_requested_modules(scope, module).map_err(|_| ExnThrown)?; diff --git a/starling/tests/modules.rs b/starling/tests/modules.rs new file mode 100644 index 00000000..7c8f0022 --- /dev/null +++ b/starling/tests/modules.rs @@ -0,0 +1,43 @@ +// SPDX-License-Identifier: Apache-2.0-WITH-LLVM-exception + +//! End-to-end checks of module loading in the `starling` binary. + +#![cfg(not(target_arch = "wasm32"))] + +use std::process::Command; + +/// A module importing the entry module by path gets the entry module itself, so +/// the entry's top level runs once and the importer sees its bindings. That holds +/// for an absolute entry path and for a bare filename in the working directory. +#[test] +fn importing_the_entry_module_gets_the_entry_module() { + let dir = tempfile::tempdir().unwrap(); + std::fs::write( + dir.path().join("main.mjs"), + "import { markerType } from './dep.mjs';\n\ + export const marker = {};\n\ + globalThis.runs = (globalThis.runs ?? 0) + 1;\n\ + console.log(`runs ${globalThis.runs} ${markerType()}`);", + ) + .unwrap(); + std::fs::write( + dir.path().join("dep.mjs"), + "import * as main from './main.mjs';\n\ + export const markerType = () => typeof main.marker;", + ) + .unwrap(); + for entry in [dir.path().join("main.mjs"), "main.mjs".into()] { + let out = Command::new(env!("CARGO_BIN_EXE_starlingmonkey")) + .current_dir(dir.path()) + .arg(&entry) + .output() + .expect("failed to run starling"); + assert_eq!( + String::from_utf8_lossy(&out.stdout), + "runs 1 object\n", + "entry {}: stderr: {}", + entry.display(), + String::from_utf8_lossy(&out.stderr) + ); + } +} From cb0de22251dc8100d281ae7cddd706c251f1c1bc Mon Sep 17 00:00:00 2001 From: Till Schneidereit Date: Tue, 29 Sep 2026 16:18:23 +0200 Subject: [PATCH 9/9] Remove the unused register_source_module `core_runtime::module::register_source_module` was used by SpiderMonkey 147's module resolution hooks, but isn't used at all anymore. --- crates/core-runtime/src/module.rs | 51 ------------------------------- 1 file changed, 51 deletions(-) diff --git a/crates/core-runtime/src/module.rs b/crates/core-runtime/src/module.rs index 72c17e8b..acd5fa1c 100644 --- a/crates/core-runtime/src/module.rs +++ b/crates/core-runtime/src/module.rs @@ -835,57 +835,6 @@ fn link_evaluate_and_populate_values( Ok(()) } -/// Register a real ES-module source under an arbitrary specifier, making it -/// importable by name. -/// -/// Where [`register_synthetic_module`] populates `export var` bindings with -/// runtime values, this compiles genuine module `source` — with its own -/// `export`s and possibly its own `import`s — and registers it under `name`. The -/// component-model bootstrap uses it for the application's named modules (the -/// componentizer's `modules` list): these may `import` from the synthesized -/// import modules, so the bootstrap registers those first. -/// -/// The module is linked and evaluated immediately so its side effects run and -/// its exports are bound. On failure the half-registered entry is removed again, -/// matching [`register_synthetic_module`]. -/// -/// # Safety -/// -/// - [`init_module_loader`] must have been called first. -pub unsafe fn register_source_module( - scope: &Scope<'_>, - name: &str, - source: &str, -) -> Result<(), ExnThrown> { - let filename = CString::new(name).map_err(|_| ExnThrown)?; - let options = js::compile::options(scope, filename, 1); - let mut src = transform_str_to_source_text(source); - // SAFETY: `options` and `src` are live locals. - let module = unsafe { js::module::compile_module(scope, options.ptr, &mut src) }?; - - // Register before loading so the load hook (and the module's own imports) - // can find it by name. - registry(|reg| { - reg.borrow_mut().insert( - name.to_string(), - ModuleEntry { - module_obj: Heap::from(module), - }, - ); - }); - - let linked = js::module::load_requested_modules(scope, module) - .and_then(|()| js::module::link(scope, module)) - .and_then(|()| js::module::evaluate(scope, module).map(drop)); - if linked.is_err() { - registry(|reg| { - reg.borrow_mut().remove(name); - }); - return Err(ExnThrown); - } - Ok(()) -} - /// Evaluate a JS script as a module, with access to registered native modules. /// /// This compiles the given source as a module, loads its dependency graph (the