From 1c6b1a1dc27b97f1b7e351696ff6b1ffe66c1aa4 Mon Sep 17 00:00:00 2001 From: Guy Bedford Date: Mon, 24 Aug 2026 13:47:00 -0700 Subject: [PATCH 1/4] rust/jsg: add typed jsg::Function and fix function-call FFI hazards Adds jsg::Function, a persistent GC-traced handle to a JS function with a typed Rust call signature (the counterpart of unwrapping a C++ jsg::Function from a JS value). It is FromJS/ToJS/Traced, so it works directly as a #[jsg_method] parameter or resource field, with arguments converted via ToJS tuples and returns via FromJS. A FromJS impl for () supports discard-result callbacks. Two safety fixes uncovered while building on the existing call path: * local_function_call wrapped fn->Call in jsg::check(), so a JS throw from the callee unwound JsExceptionThrown across the nounwind FFI frame. The shim now tunnels callee exceptions through a kj::Exception (shared checkTunneled helper, hoisted from the unwrappers), returning Result to Rust with the JS error type and message preserved. * Global was Send: dropping one on a foreign thread would call global_reset without the isolate lock and corrupt the V8 heap. A raw-pointer phantom now makes it !Send + !Sync. --- src/rust/jsg-test/tests/function.rs | 125 +++++++++++++++++++- src/rust/jsg/README.md | 18 +++ src/rust/jsg/ffi.c++ | 43 +++---- src/rust/jsg/ffi.h | 1 + src/rust/jsg/function.rs | 175 ++++++++++++++++++++++++++++ src/rust/jsg/lib.rs | 3 + src/rust/jsg/v8.rs | 14 ++- src/rust/jsg/wrappable.rs | 10 ++ 8 files changed, 365 insertions(+), 24 deletions(-) create mode 100644 src/rust/jsg/function.rs diff --git a/src/rust/jsg-test/tests/function.rs b/src/rust/jsg-test/tests/function.rs index 42b47652965..e133f252494 100644 --- a/src/rust/jsg-test/tests/function.rs +++ b/src/rust/jsg-test/tests/function.rs @@ -2,8 +2,12 @@ // Licensed under the Apache 2.0 license found in the LICENSE file or at: // https://opensource.org/licenses/Apache-2.0 -//! Tests for `Local::call()`, `As` trait, and `impl_local_cast!` conversions. +//! Tests for `Local::call()`, `jsg::Function`, `As` trait, and +//! `impl_local_cast!` conversions. +use jsg::ExceptionType; +use jsg::FromJS; +use jsg::Function; use jsg::Number; use jsg::ToJS; use jsg::v8; @@ -154,6 +158,125 @@ fn call_with_object_receiver_via_into() { }); } +/// A throwing callee surfaces as an `Err` preserving the JS error type and +/// message — it must not unwind across the FFI boundary. +#[test] +fn call_throw_returns_error() { + let harness = crate::Harness::new(); + harness.run_in_context(|lock, ctx| { + let value = ctx + .eval_raw("(() => { throw new TypeError('boom') })") + .unwrap(); + let func = value.try_as::().unwrap(); + let err = func + .call::(lock, None::>, &[]) + .unwrap_err(); + assert_eq!(err.name, ExceptionType::TypeError); + assert_eq!(err.message, "boom"); + Ok(()) + }); +} + +/// A non-Error throw (e.g. a string) still comes back as an `Err`. +#[test] +fn call_throw_non_error_value() { + let harness = crate::Harness::new(); + harness.run_in_context(|lock, ctx| { + let value = ctx.eval_raw("(() => { throw 'plain string' })").unwrap(); + let func = value.try_as::().unwrap(); + let result = func.call::(lock, None::>, &[]); + assert!(result.is_err()); + Ok(()) + }); +} + +/// `jsg::Function` converts from a JS function value and calls with typed args. +#[test] +fn typed_function_from_js_and_call() { + let harness = crate::Harness::new(); + harness.run_in_context(|lock, ctx| { + let value = ctx.eval_raw("((a, b) => a + b)").unwrap(); + let func: Function<(Number, Number), Number> = Function::from_js(lock, value)?; + let result = func.call(lock, (Number::new(10.0), Number::new(32.0)))?; + assert!((result.value() - 42.0).abs() < f64::EPSILON); + Ok(()) + }); +} + +/// `jsg::Function<(), ()>` — no arguments, result discarded. +#[test] +fn typed_function_unit_signature() { + let harness = crate::Harness::new(); + harness.run_in_context(|lock, ctx| { + let value = ctx + .eval_raw("(() => { globalThis.sideEffect = 7; })") + .unwrap(); + let func: Function = Function::from_js(lock, value)?; + func.call(lock, ())?; + let observed: Number = ctx.eval(lock, "globalThis.sideEffect").unwrap(); + assert!((observed.value() - 7.0).abs() < f64::EPSILON); + Ok(()) + }); +} + +/// `jsg::Function::from_js` rejects non-function values with a TypeError. +#[test] +fn typed_function_rejects_non_function() { + let harness = crate::Harness::new(); + harness.run_in_context(|lock, ctx| { + let value = ctx.eval_raw("42").unwrap(); + let err = Function::<(), ()>::from_js(lock, value).unwrap_err(); + assert_eq!(err.name, ExceptionType::TypeError); + assert_eq!(err.message, "expected function, got number"); + Ok(()) + }); +} + +/// A throwing callee through `jsg::Function` preserves the JS error type. +#[test] +fn typed_function_throw_preserves_error_type() { + let harness = crate::Harness::new(); + harness.run_in_context(|lock, ctx| { + let value = ctx + .eval_raw("(() => { throw new RangeError('out of range') })") + .unwrap(); + let func: Function = Function::from_js(lock, value)?; + let err = func.call(lock, ()).unwrap_err(); + assert_eq!(err.name, ExceptionType::RangeError); + assert_eq!(err.message, "out of range"); + Ok(()) + }); +} + +/// `jsg::Function::call_with_receiver` passes `this`. +#[test] +fn typed_function_with_receiver() { + let harness = crate::Harness::new(); + harness.run_in_context(|lock, ctx| { + let obj = ctx.eval_raw("({x: 100})").unwrap(); + let value = ctx.eval_raw("(function() { return this.x; })").unwrap(); + let func: Function<(), Number> = Function::from_js(lock, value)?; + let result = func.call_with_receiver(lock, Some(obj), ())?; + assert!((result.value() - 100.0).abs() < f64::EPSILON); + Ok(()) + }); +} + +/// A clone remains callable after the original is dropped (independent +/// persistent handles). +#[test] +fn typed_function_clone_independent() { + let harness = crate::Harness::new(); + harness.run_in_context(|lock, ctx| { + let value = ctx.eval_raw("(() => 'alive')").unwrap(); + let func: Function<(), String> = Function::from_js(lock, value)?; + let cloned = func.clone(lock); + drop(func); + assert_eq!(cloned.call(lock, ())?, "alive"); + Ok(()) + }); +} + /// `impl_local_cast!` conversions: `Local` → `Local` via `Into`. #[test] fn local_object_into_value() { diff --git a/src/rust/jsg/README.md b/src/rust/jsg/README.md index a6391a50784..fa45a830617 100644 --- a/src/rust/jsg/README.md +++ b/src/rust/jsg/README.md @@ -143,6 +143,24 @@ impl EventEmitter { Without tracing, storing a `Global` back to the resource's own wrapper creates an unbreakable reference cycle that leaks until the worker is torn down. With `visit_global` tracing (generated automatically by `#[jsg_resource]`), the cycle is collected by the next full GC after all strong Rust `Rc`s are dropped. +### `Function` + +A persistent, GC-traced handle to a JavaScript function with a typed Rust call signature — the counterpart of unwrapping a C++ `jsg::Function`. `Args` is a tuple of `ToJS` argument types; `R` is the `FromJS` return type (`()` discards the result). It is `FromJS`/`ToJS`/`Traced`, so it can be taken directly as a `#[jsg_method]` parameter and stored in a resource field: + +```rust +#[jsg_method] +fn on_event(&self, callback: jsg::Function<(String,), ()>) { + self.callback.set(Some(callback)); +} + +// later, under a lock: +if let Some(cb) = self.callback.take() { + cb.call(lock, ("ready".to_owned(),))?; +} +``` + +A JS exception thrown by the callee is returned as a `jsg::Error` preserving the JS error type and message. Use `call_with_receiver` to pass an explicit `this`. + ## Union Types To accept JavaScript values that can be one of several types, define an enum with `#[jsg_oneof]`: diff --git a/src/rust/jsg/ffi.c++ b/src/rust/jsg/ffi.c++ index a2f904a59aa..0a4dc84cd4a 100644 --- a/src/rust/jsg/ffi.c++ +++ b/src/rust/jsg/ffi.c++ @@ -77,6 +77,22 @@ static v8::Local makeInternedStr(v8::Isolate* isolate, const Name& n v8::String::NewFromUtf8(isolate, name.data(), v8::NewStringType::kInternalized, name.size())); } +// Runs `fn` (returning a v8::MaybeLocal) and, on failure, converts the pending +// JS exception into a `jsg.:`-prefixed kj::Exception and throws it, so +// workerd-cxx's Result::run catches it in C++ and hands Rust a catchable error +// instead of aborting. Termination exceptions are re-thrown unchanged. The +// TryCatch and termination handling are delegated to jsg::Lock::tryCatch. +template +static v8::Local checkTunneled(v8::Isolate* isolate, Func&& fn) { + auto& js = ::workerd::jsg::Lock::from(isolate); + JSG_TRY(js) { + return ::workerd::jsg::check(fn()); + } + JSG_CATCH(error) { + kj::throwFatalException(::workerd::jsg::createTunneledException(isolate, error.getHandle(js))); + }; +} + // Wrappable implementation - calls into Rust via CXX bridge Wrappable::~Wrappable() { wrappable_invoke_drop(*this); @@ -405,6 +421,8 @@ MaybeLocal local_symbol_description(Isolate* isolate, const Local& value) { } // Local + +// Fallible: tunnels any JS exception thrown by the callee (see checkTunneled). Local local_function_call( Isolate* isolate, const Local& function, const Local& recv, ::rust::Slice args) { auto context = isolate->GetCurrentContext(); @@ -416,7 +434,8 @@ Local local_function_call( v8Args[i] = local_as_ref_from_ffi(args[i]); } - return to_ffi(::workerd::jsg::check(fn->Call(context, receiver, v8Args.size(), v8Args.data()))); + return to_ffi(checkTunneled( + isolate, [&] { return fn->Call(context, receiver, v8Args.size(), v8Args.data()); })); } // Local @@ -642,22 +661,6 @@ void wrappable_attach_wrapper(kj::Rc wrappable, FunctionCallbackInfo& // Unwrappers -// Runs `fn` (returning a v8::MaybeLocal) and, on failure, converts the pending -// JS exception into a `jsg.:`-prefixed kj::Exception and throws it, so -// workerd-cxx's Result::run catches it in C++ and hands Rust a catchable error -// instead of aborting. Termination exceptions are re-thrown unchanged. The -// TryCatch and termination handling are delegated to jsg::Lock::tryCatch. -template -static v8::Local unwrapCoerce(v8::Isolate* isolate, Func&& fn) { - auto& js = ::workerd::jsg::Lock::from(isolate); - JSG_TRY(js) { - return ::workerd::jsg::check(fn()); - } - JSG_CATCH(error) { - kj::throwFatalException(::workerd::jsg::createTunneledException(isolate, error.getHandle(js))); - }; -} - ::rust::String unwrap_string(Isolate* isolate, Local value) { auto v8Value = local_from_ffi(kj::mv(value)); // Fast path: a string primitive needs no coercion and can't throw, so skip @@ -667,7 +670,7 @@ static v8::Local unwrapCoerce(v8::Isolate* isolate, Func&& fn) { v8Str = v8Value.As(); } else { auto context = isolate->GetCurrentContext(); - v8Str = unwrapCoerce(isolate, [&] { return v8Value->ToString(context); }); + v8Str = checkTunneled(isolate, [&] { return v8Value->ToString(context); }); } v8::String::ValueView view(isolate, v8Str); if (!view.is_one_byte()) { @@ -689,7 +692,7 @@ double unwrap_number(Isolate* isolate, Local value) { } auto context = isolate->GetCurrentContext(); v8::Local number = - unwrapCoerce(isolate, [&] { return v8Value->ToNumber(context); }); + checkTunneled(isolate, [&] { return v8Value->ToNumber(context); }); return number->Value(); } @@ -732,7 +735,7 @@ DEFINE_TYPED_ARRAY_UNWRAP(biguint64_array, BigUint64Array, uint64_t) // // Iterate() can fail with a *pending* V8 exception rather than a KJ one -- e.g. a // throwing Proxy trap or getter on one of the array's elements. That must be tunneled -// through the same `jsg.:`-prefixed kj::Exception mechanism as `unwrapCoerce` +// through the same `jsg.:`-prefixed kj::Exception mechanism as `checkTunneled` // uses (see above), rather than discarded in favor of a generic KJ_REQUIRE failure, // or the real error is lost and the pending exception is left dangling on the isolate. ::rust::Vec local_array_iterate(Isolate* isolate, Local value) { diff --git a/src/rust/jsg/ffi.h b/src/rust/jsg/ffi.h index 16f56170280..98bbd7a22b0 100644 --- a/src/rust/jsg/ffi.h +++ b/src/rust/jsg/ffi.h @@ -180,6 +180,7 @@ Local local_symbol_new_with_description(Isolate* isolate, Local description); MaybeLocal local_symbol_description(Isolate* isolate, const Local& value); // Local +// Fallible: tunnels any JS exception thrown by the callee. Local local_function_call( Isolate* isolate, const Local& function, const Local& recv, ::rust::Slice args); diff --git a/src/rust/jsg/function.rs b/src/rust/jsg/function.rs new file mode 100644 index 00000000000..bc963d4788e --- /dev/null +++ b/src/rust/jsg/function.rs @@ -0,0 +1,175 @@ +// Copyright (c) 2026 Cloudflare, Inc. +// Licensed under the Apache 2.0 license found in the LICENSE file or at: +// https://opensource.org/licenses/Apache-2.0 + +//! Typed persistent handles to JavaScript functions. + +use std::marker::PhantomData; + +use crate::Error; +use crate::FromJS; +use crate::Lock; +use crate::ToJS; +use crate::Traced; +use crate::Type; +use crate::v8; + +/// A persistent, GC-traced handle to a JavaScript function with a typed Rust +/// call signature — the Rust counterpart of unwrapping a C++ +/// `jsg::Function` from a JS function value. +/// +/// `Args` is a tuple of [`ToJS`] argument types (use `()` for none, `(T,)` for +/// one); `R` is the [`FromJS`] return type (`()` discards the result). +/// +/// Obtained via [`FromJS`], e.g. as a `#[jsg_method]` parameter or a +/// `#[jsg_struct]` field. The handle is persistent — safe to store in a +/// resource and call later — but must be traced for GC; `#[jsg_resource]` +/// fields get this automatically via the [`Traced`] impl. Like all V8 handles +/// it is bound to its isolate's thread (`!Send + !Sync`). +/// +/// # Example +/// +/// ```ignore +/// fn each(&self, lock: &mut Lock, callback: Function<(Number,), Number>) -> Result<(), Error> { +/// let doubled = callback.call(lock, (Number::new(21.0),))?; +/// ... +/// } +/// ``` +pub struct Function { + handle: v8::Global, + /// `fn`-pointer phantom: no drop obligations and no `Send`/`Sync` + /// inherited from `Args`/`R` (thread affinity is already enforced by the + /// raw-pointer phantom inside [`v8::Global`]). + _signature: PhantomData R>, +} + +impl Function { + /// Returns the underlying function handle in the current `HandleScope`. + pub fn as_local<'a>(&self, lock: &mut Lock) -> v8::Local<'a, v8::Function> { + self.handle.as_local(lock) + } + + /// Creates an independent persistent handle to the same JS function. + /// + /// Not the std `Clone` trait because cloning a V8 persistent handle + /// requires the isolate (see [`v8::Global::clone`]). + #[must_use] + pub fn clone(&self, lock: &mut Lock) -> Self { + Self { + handle: self.handle.clone(lock), + _signature: PhantomData, + } + } +} + +impl Function { + /// Calls the function with `undefined` as the receiver, converting + /// arguments via [`ToJS`] and the result via [`FromJS`]. + /// + /// If the callee throws, the exception is returned as an [`Error`] + /// preserving the JS error type and message. + pub fn call(&self, lock: &mut Lock, args: Args) -> Result { + self.call_with_receiver(lock, None::>, args) + } + + /// Like [`Function::call`] with an explicit `this` receiver. + pub fn call_with_receiver<'a, Recv: Into>>( + &self, + lock: &mut Lock, + receiver: Option, + args: Args, + ) -> Result { + let func = self.handle.as_local(lock); + let arg_locals = args.to_js_args(lock); + func.call::(lock, receiver, &arg_locals) + } +} + +impl std::fmt::Debug for Function { + fn fmt(&self, f: &mut std::fmt::Formatter<'_>) -> std::fmt::Result { + f.write_str("Function") + } +} + +impl Type for Function { + fn class_name() -> &'static str { + "function" + } + + fn is_exact(value: &v8::Local) -> bool { + value.is_function() + } +} + +impl FromJS for Function { + type ResultType = Self; + + fn from_js(_lock: &mut Lock, value: v8::Local) -> Result { + value.clone().try_as::().map_or_else( + || { + Err(Error::new_type_error(format!( + "expected function, got {}", + value.type_of() + ))) + }, + |func| { + Ok(Self { + handle: func.into(), + _signature: PhantomData, + }) + }, + ) + } +} + +impl ToJS for Function { + fn to_js<'a, 'b>(self, lock: &'a mut Lock) -> v8::Local<'b, v8::Value> + where + 'b: 'a, + { + self.handle.as_local(lock).into() + } +} + +/// Delegates to the inner [`v8::Global`] — a strong handle that participates +/// in cycle collection once the owning resource is only reachable from JS. +impl Traced for Function { + fn trace(&self, visitor: &mut v8::GcVisitor) { + self.handle.trace(visitor); + } +} + +/// Argument tuples accepted by [`Function::call`]. +/// +/// Implemented for tuples of [`ToJS`] types up to 8 elements. +pub trait FunctionArgs { + /// Converts each element to a JS value in the current `HandleScope`. + fn to_js_args<'a>(self, lock: &mut Lock) -> Vec>; +} + +impl FunctionArgs for () { + fn to_js_args<'a>(self, _lock: &mut Lock) -> Vec> { + Vec::new() + } +} + +macro_rules! impl_function_args { + ($($arg:ident),+) => { + impl<$($arg: ToJS),+> FunctionArgs for ($($arg,)+) { + #[expect(non_snake_case)] + fn to_js_args<'a>(self, lock: &mut Lock) -> Vec> { + let ($($arg,)+) = self; + vec![$($arg.to_js(lock)),+] + } + } + }; +} + +impl_function_args!(A1); +impl_function_args!(A1, A2); +impl_function_args!(A1, A2, A3); +impl_function_args!(A1, A2, A3, A4); +impl_function_args!(A1, A2, A3, A4, A5); +impl_function_args!(A1, A2, A3, A4, A5, A6); +impl_function_args!(A1, A2, A3, A4, A5, A6, A7); +impl_function_args!(A1, A2, A3, A4, A5, A6, A7, A8); diff --git a/src/rust/jsg/lib.rs b/src/rust/jsg/lib.rs index 7bb9a115d1a..e9f6b2c3ee2 100644 --- a/src/rust/jsg/lib.rs +++ b/src/rust/jsg/lib.rs @@ -7,6 +7,7 @@ use std::num::ParseIntError; use std::ops::Deref; pub mod feature_flags; +pub mod function; pub mod macros; pub mod modules; pub mod nullable; @@ -15,6 +16,8 @@ pub mod v8; mod wrappable; pub use feature_flags::FeatureFlags; +pub use function::Function; +pub use function::FunctionArgs; pub use nullable::Nullable; pub use resource::Rc; pub use resource::Resource; diff --git a/src/rust/jsg/v8.rs b/src/rust/jsg/v8.rs index f6c55ae49bd..92d0f309c37 100644 --- a/src/rust/jsg/v8.rs +++ b/src/rust/jsg/v8.rs @@ -360,12 +360,14 @@ pub mod ffi { pub unsafe fn local_symbol_description(isolate: *mut Isolate, value: &Local) -> MaybeLocal; // Local + // Fallible: tunnels any JS exception thrown by the callee (including + // exceptions from user code invoked transitively). See "Unwrappers" below. pub unsafe fn local_function_call( isolate: *mut Isolate, function: &Local, recv: &Local, args: &[Local], - ) -> Local; + ) -> Result; // Local pub unsafe fn local_object_set_property( @@ -1452,6 +1454,9 @@ impl Local<'_, Function> { /// `args` is a slice of `Local` — use [`ToJS::to_js`] to convert Rust /// values, or `.into()` for other `Local` handles. /// + /// If the callee throws, the exception is returned as an [`Error`] preserving + /// the JS error type and message. + /// /// # Example /// /// ```ignore @@ -1482,7 +1487,7 @@ impl Local<'_, Function> { &self.handle, recv.as_ffi(), &ffi_args, - ), + )?, ) }; R::from_js(lock, result) @@ -2900,7 +2905,10 @@ pub struct Global { /// This is sound because GC tracing is always single-threaded within a V8 /// isolate and `trace` is never re-entrant on the same object. traced: UnsafeCell, - _marker: PhantomData, + /// `*mut ()` makes `Global` `!Send + !Sync`: the handle is bound to its + /// isolate's thread, and even `Drop` mutates V8 state (`global_reset`), so + /// moving one across threads would corrupt the V8 heap. + _marker: PhantomData<(T, *mut ())>, } // Common implementations for all Global diff --git a/src/rust/jsg/wrappable.rs b/src/rust/jsg/wrappable.rs index 58c76187b8e..b02742dd1c5 100644 --- a/src/rust/jsg/wrappable.rs +++ b/src/rust/jsg/wrappable.rs @@ -282,6 +282,16 @@ pub trait FromJS: Sized { // Primitive type implementations // ============================================================================= +/// `()` accepts any JS value and discards it — the return type for callbacks +/// whose result is ignored, mirroring C++ `jsg::Function`. +impl FromJS for () { + type ResultType = Self; + + fn from_js(_lock: &mut Lock, _value: v8::Local) -> Result { + Ok(()) + } +} + // Boolean implementation for JavaScript booleans. impl Type for bool { fn class_name() -> &'static str { From f759d72e678d320fb6a1ae2552c8964eb635b738 Mon Sep 17 00:00:00 2001 From: Guy Bedford Date: Mon, 24 Aug 2026 16:42:14 -0700 Subject: [PATCH 2/4] rust/jsg: distinguish isolate termination from JS exceptions at the FFI Isolate termination during a JS function call previously surfaced to Rust as a generic internal error (the JsExceptionThrown re-thrown by jsg::Lock::tryCatch was swallowed by workerd-cxx's catch-all), so a Rust caller could not tell it apart from a callee throw and might keep calling into JS. The FFI shims now convert an escaping JsExceptionThrown into a kj::Exception with the unforgeable description "jsg-internal.Terminated: ..." (user exceptions tunnel under the plain jsg. prefix). Rust parses it into a jsg::Error with is_termination() set, and Lock::throw_exception() re-raises such errors by re-arming terminate_execution() instead of scheduling a catchable JS throw. The termination flag remains pending on the isolate throughout, so even a caller that ignores the error cannot resume JS execution. Also fills the review-noted test gaps for jsg::Function: coverage for the #[jsg_method] parameter pipeline, #[jsg_struct] fields (which needed a ToLocalValue impl), GC tracing of a stored callback in traced mode, and cycle collection of resource -> callback -> wrapper loops. --- src/rust/jsg-test/tests/function.rs | 15 +++ src/rust/jsg-test/tests/function_resource.rs | 131 +++++++++++++++++++ src/rust/jsg-test/tests/jsg_struct.rs | 19 +++ src/rust/jsg-test/tests/mod.rs | 1 + src/rust/jsg/README.md | 2 +- src/rust/jsg/ffi.c++ | 40 +++++- src/rust/jsg/function.rs | 22 +++- src/rust/jsg/lib.rs | 64 +++++++++ 8 files changed, 280 insertions(+), 14 deletions(-) create mode 100644 src/rust/jsg-test/tests/function_resource.rs diff --git a/src/rust/jsg-test/tests/function.rs b/src/rust/jsg-test/tests/function.rs index e133f252494..d64a0c18923 100644 --- a/src/rust/jsg-test/tests/function.rs +++ b/src/rust/jsg-test/tests/function.rs @@ -262,6 +262,21 @@ fn typed_function_with_receiver() { }); } +/// Isolate termination surfaces as an `Error` with `is_termination()` set — +/// distinguishable from a JS throw, and not process-fatal. +#[test] +fn call_after_terminate_returns_termination_error() { + let harness = crate::Harness::new(); + harness.run_in_context(|lock, ctx| { + let value = ctx.eval_raw("(() => 1)").unwrap(); + let func: Function<(), Number> = Function::from_js(lock, value)?; + lock.terminate_execution(); + let err = func.call(lock, ()).unwrap_err(); + assert!(err.is_termination()); + Ok(()) + }); +} + /// A clone remains callable after the original is dropped (independent /// persistent handles). #[test] diff --git a/src/rust/jsg-test/tests/function_resource.rs b/src/rust/jsg-test/tests/function_resource.rs new file mode 100644 index 00000000000..871656f2cf0 --- /dev/null +++ b/src/rust/jsg-test/tests/function_resource.rs @@ -0,0 +1,131 @@ +// Copyright (c) 2026 Cloudflare, Inc. +// Licensed under the Apache 2.0 license found in the LICENSE file or at: +// https://opensource.org/licenses/Apache-2.0 + +//! Tests for `jsg::Function` held in resources: the `#[jsg_method]` parameter +//! pipeline, GC tracing of stored callbacks, and cycle collection. + +use std::cell::Cell; +use std::sync::atomic::AtomicUsize; +use std::sync::atomic::Ordering; + +use jsg::Number; +use jsg::ToJS; +use jsg::v8; +use jsg_macros::jsg_method; +use jsg_macros::jsg_resource; + +static HOLDER_DROPS: AtomicUsize = AtomicUsize::new(0); + +#[jsg_resource] +struct CallbackHolder { + pub callback: Cell>>, +} + +impl Drop for CallbackHolder { + fn drop(&mut self) { + HOLDER_DROPS.fetch_add(1, Ordering::SeqCst); + } +} + +#[jsg_resource] +impl CallbackHolder { + #[jsg_method] + fn set_callback(&self, callback: jsg::Function<(), Number>) { + self.callback.set(Some(callback)); + } + + #[jsg_method] + fn invoke(&self, lock: &mut jsg::Lock) -> Result { + let callback = self + .callback + .take() + .ok_or_else(|| jsg::Error::new_error("no callback set"))?; + let result = callback.call(lock, ()); + self.callback.set(Some(callback)); + result + } +} + +/// `Function` as a `#[jsg_method]` parameter: JS registers a callback through +/// the macro pipeline and Rust invokes it. +#[test] +fn function_as_method_parameter() { + let harness = crate::Harness::new(); + harness.run_in_context(|lock, ctx| { + let holder = jsg::Rc::new(CallbackHolder { + callback: Cell::new(None), + }); + let wrapped = holder.to_js(lock); + ctx.set_global("holder", wrapped); + + ctx.eval_raw("holder.setCallback(() => 42)").unwrap(); + let result: Number = ctx.eval(lock, "holder.invoke()").unwrap(); + assert!((result.value() - 42.0).abs() < f64::EPSILON); + Ok(()) + }); +} + +/// A stored callback survives a full GC while the resource is only reachable +/// from JS (traced mode): if `Traced` tracing of the inner `Global` were +/// broken, the traced handle would dangle and this would crash or fail. +#[test] +fn stored_callback_survives_gc_in_traced_mode() { + let harness = crate::Harness::new(); + harness.run_in_context(|lock, ctx| { + let holder = jsg::Rc::new(CallbackHolder { + callback: Cell::new(None), + }); + let wrapped = holder.clone().to_js(lock); + ctx.set_global("holder", wrapped); + ctx.eval_raw("holder.setCallback(() => 7)").unwrap(); + + // Drop the strong Rust ref so the resource (and its stored Global) + // downgrades to traced mode, then force a full GC. + std::mem::drop(holder); + crate::Harness::request_gc(lock); + + let result: Number = ctx.eval(lock, "holder.invoke()").unwrap(); + assert!((result.value() - 7.0).abs() < f64::EPSILON); + Ok(()) + }); +} + +/// A cycle between a resource and its stored callback (the JS closure captures +/// the resource's own wrapper) is collected once nothing else references it. +#[test] +fn callback_cycle_collected() { + HOLDER_DROPS.store(0, Ordering::SeqCst); + + let harness = crate::Harness::new(); + harness.run_in_context(|lock, ctx| { + let holder = jsg::Rc::new(CallbackHolder { + callback: Cell::new(None), + }); + let wrapped = holder.clone().to_js(lock); + + // Build the cycle wrapper -> Function -> closure -> wrapper without + // leaving any global variable referencing the holder. + let setter = ctx + .eval_raw("(h => { h.setCallback(() => { h; return 1; }); })") + .unwrap(); + setter.try_as::().unwrap().call::<(), _>( + lock, + None::>, + &[wrapped], + )?; + + std::mem::drop(holder); + crate::Harness::request_gc(lock); + // Still alive: the Local from to_js pins the wrapper for this scope. + assert_eq!(HOLDER_DROPS.load(Ordering::SeqCst), 0); + Ok(()) + }); + + // New context: the old scope's Locals are gone; only the cycle remains. + harness.run_in_context(|lock, _ctx| { + crate::Harness::request_gc(lock); + assert_eq!(HOLDER_DROPS.load(Ordering::SeqCst), 1); + Ok(()) + }); +} diff --git a/src/rust/jsg-test/tests/jsg_struct.rs b/src/rust/jsg-test/tests/jsg_struct.rs index 5bfa854a29e..44fde6f2331 100644 --- a/src/rust/jsg-test/tests/jsg_struct.rs +++ b/src/rust/jsg-test/tests/jsg_struct.rs @@ -207,3 +207,22 @@ fn type_of_returns_correct_js_types() { Ok(()) }); } + +#[jsg_struct] +struct HandlerOptions { + pub handler: jsg::Function<(), jsg::Number>, +} + +/// A `jsg::Function` field converts through `#[jsg_struct]` and remains +/// callable from the unwrapped struct. +#[test] +fn struct_with_function_field() { + let harness = crate::Harness::new(); + harness.run_in_context(|lock, ctx| { + let value = ctx.eval_raw("({handler: () => 7})").unwrap(); + let options = ::from_js(lock, value)?; + let result = options.handler.call(lock, ())?; + assert!((result.value() - 7.0).abs() < f64::EPSILON); + Ok(()) + }); +} diff --git a/src/rust/jsg-test/tests/mod.rs b/src/rust/jsg-test/tests/mod.rs index e5e149ea5cb..4e7a1bef4e0 100644 --- a/src/rust/jsg-test/tests/mod.rs +++ b/src/rust/jsg-test/tests/mod.rs @@ -8,6 +8,7 @@ mod coercion_safety; mod collections_gc; mod eval; mod function; +mod function_resource; mod gc; mod jsg_oneof; mod jsg_struct; diff --git a/src/rust/jsg/README.md b/src/rust/jsg/README.md index fa45a830617..fb25487761e 100644 --- a/src/rust/jsg/README.md +++ b/src/rust/jsg/README.md @@ -159,7 +159,7 @@ if let Some(cb) = self.callback.take() { } ``` -A JS exception thrown by the callee is returned as a `jsg::Error` preserving the JS error type and message. Use `call_with_receiver` to pass an explicit `this`. +A JS exception thrown by the callee is returned as a `jsg::Error` preserving the JS error type and message. Isolate termination during the call is returned as a `jsg::Error` with `is_termination()` set; stop calling into JS when you see it — throwing it via `Lock::throw_exception()` re-arms termination rather than scheduling a catchable JS exception. Use `call_with_receiver` to pass an explicit `this`. ## Union Types diff --git a/src/rust/jsg/ffi.c++ b/src/rust/jsg/ffi.c++ index 0a4dc84cd4a..18c476293aa 100644 --- a/src/rust/jsg/ffi.c++ +++ b/src/rust/jsg/ffi.c++ @@ -77,20 +77,46 @@ static v8::Local makeInternedStr(v8::Isolate* isolate, const Name& n v8::String::NewFromUtf8(isolate, name.data(), v8::NewStringType::kInternalized, name.size())); } +// Description prefix for isolate termination surfaced across the FFI. Rust's +// `jsg::Error::from_kj_description` recognizes it and marks the error as +// termination, which `Lock::throw_exception` re-raises by re-arming +// TerminateExecution() instead of scheduling a JS throw. The `jsg-internal.` +// tunneling namespace makes it unforgeable from guest JS: user exceptions +// tunnel with the plain `jsg.` prefix. +static constexpr kj::StringPtr TERMINATED_DESCRIPTION = + "jsg-internal.Terminated: JavaScript execution terminated"_kj; + // Runs `fn` (returning a v8::MaybeLocal) and, on failure, converts the pending // JS exception into a `jsg.:`-prefixed kj::Exception and throws it, so // workerd-cxx's Result::run catches it in C++ and hands Rust a catchable error -// instead of aborting. Termination exceptions are re-thrown unchanged. The -// TryCatch and termination handling are delegated to jsg::Lock::tryCatch. +// instead of aborting. +// +// Isolate termination (which jsg::Lock::tryCatch re-throws as JsExceptionThrown, +// expecting C++ callers to keep unwinding) cannot unwind across the nounwind FFI +// frame either, so it is converted to a TERMINATED_DESCRIPTION kj::Exception. +// The termination flag remains pending on the isolate regardless, so even a Rust +// caller that swallows the error cannot resume JS execution. template static v8::Local checkTunneled(v8::Isolate* isolate, Func&& fn) { auto& js = ::workerd::jsg::Lock::from(isolate); - JSG_TRY(js) { - return ::workerd::jsg::check(fn()); + try { + JSG_TRY(js) { + return ::workerd::jsg::check(fn()); + } + JSG_CATCH(error) { + kj::throwFatalException( + ::workerd::jsg::createTunneledException(isolate, error.getHandle(js))); + }; + } catch (::workerd::jsg::JsExceptionThrown&) { + // Lock::tryCatch also re-throws JsExceptionThrown when no exception was + // actually scheduled (e.g. V8 already cleared the termination flag after + // unwinding all JS frames); it conflates that case with termination, and so + // do we. + // Constructed directly (not via KJ_EXCEPTION) so the description is exactly + // TERMINATED_DESCRIPTION, which Rust matches on. + kj::throwFatalException(kj::Exception( + kj::Exception::Type::FAILED, __FILE__, __LINE__, kj::str(TERMINATED_DESCRIPTION))); } - JSG_CATCH(error) { - kj::throwFatalException(::workerd::jsg::createTunneledException(isolate, error.getHandle(js))); - }; } // Wrappable implementation - calls into Rust via CXX bridge diff --git a/src/rust/jsg/function.rs b/src/rust/jsg/function.rs index bc963d4788e..c2a0a497fca 100644 --- a/src/rust/jsg/function.rs +++ b/src/rust/jsg/function.rs @@ -21,11 +21,12 @@ use crate::v8; /// `Args` is a tuple of [`ToJS`] argument types (use `()` for none, `(T,)` for /// one); `R` is the [`FromJS`] return type (`()` discards the result). /// -/// Obtained via [`FromJS`], e.g. as a `#[jsg_method]` parameter or a -/// `#[jsg_struct]` field. The handle is persistent — safe to store in a -/// resource and call later — but must be traced for GC; `#[jsg_resource]` -/// fields get this automatically via the [`Traced`] impl. Like all V8 handles -/// it is bound to its isolate's thread (`!Send + !Sync`). +/// Obtained via [`FromJS`], e.g. as a `#[jsg_method]` parameter, a +/// `#[jsg_struct]` field, or a `#[jsg_resource]` field. The handle is +/// persistent — safe to store in a resource and call later — but must be +/// traced for GC; resource fields get this automatically via the [`Traced`] +/// impl. Like all V8 handles it is bound to its isolate's thread +/// (`!Send + !Sync`). /// /// # Example /// @@ -67,7 +68,9 @@ impl Function { /// arguments via [`ToJS`] and the result via [`FromJS`]. /// /// If the callee throws, the exception is returned as an [`Error`] - /// preserving the JS error type and message. + /// preserving the JS error type and message. Isolate termination during the + /// call is returned as an [`Error`] with [`Error::is_termination`] set — + /// stop calling into JS when you see it (see the accessor's docs). pub fn call(&self, lock: &mut Lock, args: Args) -> Result { self.call_with_receiver(lock, None::>, args) } @@ -131,6 +134,13 @@ impl ToJS for Function { } } +/// By-ref conversion, used by `#[jsg_struct]` field wrapping. +impl v8::ToLocalValue for Function { + fn to_local<'a>(&self, lock: &mut Lock) -> v8::Local<'a, v8::Value> { + self.handle.as_local(lock).into() + } +} + /// Delegates to the inner [`v8::Global`] — a strong handle that participates /// in cycle collection once the owning resource is only reachable from JS. impl Traced for Function { diff --git a/src/rust/jsg/lib.rs b/src/rust/jsg/lib.rs index e9f6b2c3ee2..3e5ab7c16a8 100644 --- a/src/rust/jsg/lib.rs +++ b/src/rust/jsg/lib.rs @@ -104,6 +104,14 @@ pub struct Error { /// `impl_error_constructors!` macro) always set this to `false`; only /// `from_kj_description()` sets it to `true`. is_internal: bool, + /// If `true`, this error represents isolate termination surfaced across the + /// FFI (the C++ shims' `TERMINATED_DESCRIPTION` tunnel, see + /// `src/rust/jsg/ffi.c++`), not a catchable JS exception. + /// `Lock::throw_exception()` re-raises it by re-arming + /// `terminate_execution()` instead of scheduling a JS throw. Only + /// `from_kj_description()` sets it; guest JS cannot forge it because user + /// exceptions tunnel under the `jsg.` prefix, not `jsg-internal.`. + is_termination: bool, } impl std::fmt::Display for Error { @@ -123,6 +131,7 @@ macro_rules! impl_error_constructors { name: ExceptionType::$variant, message: message.into(), is_internal: false, + is_termination: false, } } )* @@ -173,6 +182,7 @@ impl FromJS for Error { name: name.map_or(ExceptionType::Error, |n| ExceptionType::from(n.as_str())), message, is_internal: false, + is_termination: false, }) } else { Err(Self::new_type_error("Unknown error")) @@ -186,6 +196,7 @@ impl Error { name: ExceptionType::from(name), message: message.to_owned(), is_internal: false, + is_termination: false, } } @@ -202,6 +213,7 @@ impl Error { name, message: message.into(), is_internal: false, + is_termination: false, } } @@ -325,6 +337,17 @@ impl Error { None => (msg.strip_prefix("jsg-internal.")?, true), }; + // Isolate termination tunneled by the FFI shims (TERMINATED_DESCRIPTION in + // ffi.c++). Only recognized under the unforgeable `jsg-internal.` prefix. + if is_internal && let Some(message) = body.strip_prefix("Terminated: ") { + return Some(Self { + name: ExceptionType::Error, + message: message.to_owned(), + is_internal: true, + is_termination: true, + }); + } + if let Some(rest) = body.strip_prefix("DOMException(") && let Some((name, message)) = rest.split_once("): ") { @@ -332,6 +355,7 @@ impl Error { name: ExceptionType::from(name), message: message.to_owned(), is_internal, + is_termination: false, }); } @@ -340,9 +364,21 @@ impl Error { name: ExceptionType::from(ty), message: message.to_owned(), is_internal, + is_termination: false, }) } + /// Returns `true` if this error represents isolate termination rather than a + /// catchable JS exception. + /// + /// Callers looping over JS callbacks should treat this as a signal to stop + /// immediately: the termination flag is pending on the isolate, so further JS + /// entry only fails again. Throwing the error via `Lock::throw_exception()` + /// re-arms termination rather than scheduling a JS throw. + pub fn is_termination(&self) -> bool { + self.is_termination + } + /// Like `new_type_error()`, but marks the resulting `Error` `is_internal`, so /// `Lock::throw_exception()` redacts `message` instead of exposing it to guest JS. /// The JS-visible error type ends up as plain `Error` regardless of the type set @@ -354,6 +390,7 @@ impl Error { name: ExceptionType::TypeError, message: message.into(), is_internal: true, + is_termination: false, } } } @@ -374,6 +411,23 @@ mod tunneled_error_tests { assert!(!err.is_internal, "jsg. errors must not be redacted"); } + #[test] + fn termination_tunnel() { + let err = from_description("jsg-internal.Terminated: JavaScript execution terminated"); + assert!(err.is_termination()); + assert!(err.is_internal); + assert_eq!(err.message, "JavaScript execution terminated"); + } + + #[test] + fn termination_not_forgeable_from_guest_prefix() { + // A user error named "Terminated" tunnels under `jsg.`, which must not + // be treated as termination. + let err = from_description("jsg.Terminated: fake"); + assert!(!err.is_termination()); + assert!(!err.is_internal); + } + #[test] fn jsg_require_wraps_with_expected_prefix() { // What JSG_REQUIRE(cond, TypeError, "boom") actually produces via KJ_REQUIRE. @@ -851,10 +905,20 @@ impl Lock { /// Throws an error as a V8 exception. /// + /// If `err.is_termination` is set, no JS exception is scheduled; termination + /// is re-armed instead so V8 unwinds all JS frames (a termination "throw" + /// must never be catchable by guest JS). + /// /// If `err.is_internal` is set, the message is redacted (via /// `throw_internal_error()`) rather than thrown verbatim, matching how /// C++ `decodeTunneledException()` handles `isInternal` KJ exceptions. pub fn throw_exception(&mut self, err: &Error) { + if err.is_termination { + // Idempotent if termination is already pending; covers the edge case + // where V8 cleared the flag after unwinding all JS frames. + self.terminate_execution(); + return; + } if err.is_internal { self.throw_internal_error(&err.message); return; From b4bc686e43e231b4edd84ceb635e5c8c91b4fdc4 Mon Sep 17 00:00:00 2001 From: Guy Bedford Date: Mon, 24 Aug 2026 16:59:35 -0700 Subject: [PATCH 3/4] rust/jsg: fix doc-markdown lint in function tests --- src/rust/jsg-test/tests/function.rs | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/src/rust/jsg-test/tests/function.rs b/src/rust/jsg-test/tests/function.rs index d64a0c18923..f9f742b4f66 100644 --- a/src/rust/jsg-test/tests/function.rs +++ b/src/rust/jsg-test/tests/function.rs @@ -219,7 +219,7 @@ fn typed_function_unit_signature() { }); } -/// `jsg::Function::from_js` rejects non-function values with a TypeError. +/// `jsg::Function::from_js` rejects non-function values with a `TypeError`. #[test] fn typed_function_rejects_non_function() { let harness = crate::Harness::new(); From eea48099e455044a497ca2986fcff8e65365304f Mon Sep 17 00:00:00 2001 From: Guy Bedford Date: Mon, 24 Aug 2026 17:16:21 -0700 Subject: [PATCH 4/4] rust/jsg-test: cancel termination before returning control to the harness Debug V8 DCHECKs (!isolate_->has_exception()) on API entry while the termination exception is still scheduled. Production dispatch paths return into V8 with the exception pending (the normal throw protocol), but the test harness keeps making embedder calls after the test closure returns, so a test that deliberately terminates must cancel first. --- src/rust/jsg-test/ffi.c++ | 4 ++++ src/rust/jsg-test/ffi.h | 1 + src/rust/jsg-test/lib.rs | 10 ++++++++++ src/rust/jsg-test/tests/function.rs | 1 + 4 files changed, 16 insertions(+) diff --git a/src/rust/jsg-test/ffi.c++ b/src/rust/jsg-test/ffi.c++ index 96aa5865d86..5e1d361f3fc 100644 --- a/src/rust/jsg-test/ffi.c++ +++ b/src/rust/jsg-test/ffi.c++ @@ -114,6 +114,10 @@ void TestHarness::run_in_context( }); } +void cancel_termination(Isolate* isolate) { + isolate->CancelTerminateExecution(); +} + void request_gc(Isolate* isolate, GcType gc_type) { switch (gc_type) { case GcType::Full: diff --git a/src/rust/jsg-test/ffi.h b/src/rust/jsg-test/ffi.h index 58c74ab9b7e..0634d1f3899 100644 --- a/src/rust/jsg-test/ffi.h +++ b/src/rust/jsg-test/ffi.h @@ -58,6 +58,7 @@ kj::Own create_test_harness(); enum class GcType : uint8_t; // Triggers garbage collection for testing purposes. +void cancel_termination(Isolate* isolate); void request_gc(Isolate* isolate, GcType gc_type); // Creates a V8 object with the C++ WORKERD_WRAPPABLE_TAG set in its internal fields. diff --git a/src/rust/jsg-test/lib.rs b/src/rust/jsg-test/lib.rs index ba8604173f6..d62071ac3bd 100644 --- a/src/rust/jsg-test/lib.rs +++ b/src/rust/jsg-test/lib.rs @@ -53,6 +53,7 @@ mod ffi { /// current HandleScope. #[expect(clippy::allow_attributes)] // Only used in tests, but #[expect(dead_code)] fails during test builds #[allow(dead_code)] + pub unsafe fn cancel_termination(isolate: *mut Isolate); pub unsafe fn request_gc(isolate: *mut Isolate, gc_type: GcType); /// Creates a V8 object with the C++ `WORKERD_WRAPPABLE_TAG` in its internal fields. @@ -215,6 +216,15 @@ impl Harness { } } + /// Cancels a pending `terminate_execution()` so the harness can keep using + /// the isolate after a test deliberately terminates it. Debug V8 DCHECKs + /// (`!isolate_->has_exception()`) on API entry if the termination exception + /// is still scheduled when the test returns control to the harness. + pub fn cancel_termination(lock: &mut jsg::Lock) { + // SAFETY: isolate is valid and locked (guaranteed by Lock). + unsafe { ffi::cancel_termination(lock.isolate().as_ffi()) }; + } + pub fn request_gc(lock: &mut jsg::Lock) { // SAFETY: isolate is valid and locked (guaranteed by Lock). unsafe { ffi::request_gc(lock.isolate().as_ffi(), ffi::GcType::Full) }; diff --git a/src/rust/jsg-test/tests/function.rs b/src/rust/jsg-test/tests/function.rs index f9f742b4f66..556ee0a3f96 100644 --- a/src/rust/jsg-test/tests/function.rs +++ b/src/rust/jsg-test/tests/function.rs @@ -273,6 +273,7 @@ fn call_after_terminate_returns_termination_error() { lock.terminate_execution(); let err = func.call(lock, ()).unwrap_err(); assert!(err.is_termination()); + crate::Harness::cancel_termination(lock); Ok(()) }); }