diff --git a/src/js-host-api/lib.js b/src/js-host-api/lib.js index 64281bf..79b210c 100644 --- a/src/js-host-api/lib.js +++ b/src/js-host-api/lib.js @@ -208,6 +208,30 @@ for (const method of [ SandboxBuilder.prototype[method] = wrapSync(orig); } +// setHostPrintFn needs a custom wrapper: the user's callback is wrapped in +// try/catch before it reaches the native layer, because exceptions thrown +// inside a Blocking ThreadsafeFunction escape as unhandled errors. +{ + const origSetHostPrintFn = SandboxBuilder.prototype.setHostPrintFn; + if (!origSetHostPrintFn) { + throw new Error('Cannot wrap missing method: SandboxBuilder.setHostPrintFn'); + } + SandboxBuilder.prototype.setHostPrintFn = wrapSync(function (callback) { + if (typeof callback !== 'function') { + throw new TypeError( + `SandboxBuilder.setHostPrintFn expects a function, received ${typeof callback}` + ); + } + return origSetHostPrintFn.call(this, (msg) => { + try { + callback(msg); + } catch (e) { + console.error('Host print callback threw:', e); + } + }); + }); +} + // ── Re-export ──────────────────────────────────────────────────────── module.exports = native; diff --git a/src/js-host-api/src/lib.rs b/src/js-host-api/src/lib.rs index b3c1e88..2a606c3 100644 --- a/src/js-host-api/src/lib.rs +++ b/src/js-host-api/src/lib.rs @@ -123,6 +123,8 @@ enum ErrorCode { Consumed, /// Internal / unexpected failure (lock poison, task join error, etc.). Internal, + /// Reentrant call detected — calling sandbox methods from a host callback. + Reentrant, } impl ErrorCode { @@ -135,6 +137,7 @@ impl ErrorCode { Self::InvalidArg => "ERR_INVALID_ARG", Self::Consumed => "ERR_CONSUMED", Self::Internal => "ERR_INTERNAL", + Self::Reentrant => "ERR_REENTRANT", } } } @@ -230,6 +233,15 @@ fn join_error(err: tokio::task::JoinError) -> napi::Error { ) } +/// Creates an error for reentrant calls detected at runtime. +fn reentrant_error() -> napi::Error { + hl_error( + ErrorCode::Reentrant, + "Cannot call sandbox methods from a host callback — the sandbox lock is held during \ + guest execution. Perform this operation after callHandler() resolves instead", + ) +} + // ── Snapshot ───────────────────────────────────────────────────────── /// A captured point-in-time state of a sandbox. @@ -401,6 +413,55 @@ impl SandboxBuilderWrapper { inner: Arc::new(Mutex::new(Some(proto_sandbox))), }) } + + /// Set a callback that receives guest `console.log` / `print` output. + /// + /// Without this, guest print output is silently discarded. The callback + /// receives each print message as a string. + /// + /// If the callback throws, the exception is caught by the JS wrapper + /// (`lib.js`) and logged to `console.error`. Guest execution continues. + /// + /// @param callback - `(message: string) => void` — called for each print + /// @returns this (for chaining) + /// @throws If the builder has already been consumed by `build()` + #[napi] + pub fn set_host_print_fn( + &self, + #[napi(ts_arg_type = "(message: string) => void")] callback: ThreadsafeFunction< + String, // Rust → JS argument type + (), // JS return type (void) + String, // JS → Rust argument type (same — identity mapping) + Status, // Error status type + false, // Not CallerHandled (napi manages errors) + false, // Not accepting unknown return types + >, + ) -> napi::Result<&Self> { + self.with_inner(|b| { + // Blocking mode ensures the TSFN call is queued even when the + // queue is full (it blocks until space is available), preventing + // silent message drops that NonBlocking mode would cause. + // + // The JS wrapper invokes the user callback synchronously in the + // TSFN handler — no microtask deferral. + // + // **Reentrancy note:** The print callback runs while the sandbox + // Mutex is held (inside `call_handler`'s `spawn_blocking`). + // If user code in the callback attempts to call methods that + // acquire the same lock (e.g. `snapshot()`, `restore()`, + // `unload()`, `callHandler()`), the `executing_flag` deadlock + // detection will return `ERR_REENTRANT` instead of hanging. + let print_fn = move |msg: String| -> i32 { + let status = callback.call(msg, ThreadsafeFunctionCallMode::Blocking); + if status == Status::Ok { + 0 + } else { + -1 + } + }; + b.with_host_print_fn(print_fn.into()) + }) + } } // ── ProtoJSSandbox ─────────────────────────────────────────────────── @@ -635,10 +696,10 @@ impl HostModuleWrapper { return Err(invalid_arg_error("Function name must not be empty")); } let wrapper = move |args: String| -> hyperlight_js::Result { - use ThreadsafeFunctionCallMode::NonBlocking; + use ThreadsafeFunctionCallMode::Blocking; let args: Vec> = serde_json::from_str(&args)?; let (tx, rx) = oneshot::channel(); - let status = func.call_with_return_value(Rest(args), NonBlocking, move |result, _| { + let status = func.call_with_return_value(Rest(args), Blocking, move |result, _| { let _ = tx.send(result); Ok(()) }); @@ -805,6 +866,7 @@ impl JSSandboxWrapper { poisoned_flag, last_call_stats: Arc::new(ArcSwapOption::empty()), disposed_flag: Arc::new(AtomicBool::new(false)), + executing_flag: Arc::new(AtomicBool::new(false)), }) } @@ -887,17 +949,37 @@ pub struct LoadedJSSandboxWrapper { /// `unload()`), for lock-free checks in sync getters that don't /// consult the inner Mutex. disposed_flag: Arc, + + /// Tracks whether guest code is currently executing inside `call_handler`. + /// + /// Used for deadlock detection: if a host callback (print fn, host function) + /// tries to call a method that needs the inner Mutex while this is `true`, + /// we return `ERR_REENTRANT` immediately instead of deadlocking. + executing_flag: Arc, } type LoadedJSSandboxGuard = OwnedMappedMutexGuard, LoadedJSSandbox>; impl LoadedJSSandboxWrapper { /// Borrow the inner value mutably via Mutex, or error if consumed. + /// + /// Performs deadlock detection: if `executing_flag` is set (meaning we're + /// inside a `call_handler` on a background thread), `try_lock` is attempted + /// first. If it fails, we know this is a reentrant call from a host callback + /// and return `ERR_REENTRANT` immediately instead of deadlocking. async fn with_inner( &self, f: impl AsyncFnOnce(LoadedJSSandboxGuard) -> napi::Result, ) -> napi::Result { - let sandbox = self.inner.clone().lock_owned().await; + let sandbox = if self.executing_flag.load(Ordering::Acquire) { + // We're inside a host callback — try_lock to detect reentrancy. + match self.inner.clone().try_lock_owned() { + Ok(guard) => guard, + Err(_) => return Err(reentrant_error()), + } + } else { + self.inner.clone().lock_owned().await + }; let sandbox = OwnedMutexGuard::try_map(sandbox, Option::as_mut) .map_err(|_| consumed_error("LoadedJSSandbox"))?; f(sandbox).await @@ -906,30 +988,42 @@ impl LoadedJSSandboxWrapper { /// Borrow the inner value mutably via Mutex, or error if consumed. /// The closure `f` will run using spawn_blocking, so it can perform long-running operations without /// blocking the Node.js event loop. This is the main way to interact with the inner `LoadedJSSandbox`. + /// + /// Sets `executing_flag` for the duration of the blocking closure so that + /// reentrant calls from host callbacks are detected instead of deadlocking. async fn with_blocking_inner( &self, f: impl FnOnce(LoadedJSSandboxGuard) -> napi::Result + Send + 'static, ) -> napi::Result { + let executing_flag = self.executing_flag.clone(); self.with_inner(async move |sandbox| { - tokio::task::spawn_blocking(move || f(sandbox)) + executing_flag.store(true, Ordering::Release); + let result = tokio::task::spawn_blocking(move || f(sandbox)) .await - .map_err(join_error)? + .map_err(join_error)?; + executing_flag.store(false, Ordering::Release); + result }) .await } /// Take ownership of the inner value, returning a consumed-state error if /// this instance has already been used. + /// + /// Performs the same deadlock detection as `with_inner`. async fn take_inner_with( &self, f: impl AsyncFnOnce(LoadedJSSandbox) -> napi::Result, ) -> napi::Result { - let sandbox = self - .inner - .lock() - .await - .take() - .ok_or_else(|| consumed_error("LoadedJSSandbox"))?; + let sandbox = if self.executing_flag.load(Ordering::Acquire) { + match self.inner.try_lock() { + Ok(mut guard) => guard.take(), + Err(_) => return Err(reentrant_error()), + } + } else { + self.inner.lock().await.take() + } + .ok_or_else(|| consumed_error("LoadedJSSandbox"))?; self.disposed_flag.store(true, Ordering::Release); f(sandbox).await } @@ -938,14 +1032,20 @@ impl LoadedJSSandboxWrapper { /// this instance has already been used. /// The closure `f` will run using spawn_blocking, so it can perform long-running operations without /// blocking the Node.js event loop. This is the main way to interact with the inner `LoadedJSSandbox`. + /// + /// Sets `executing_flag` for the duration of the blocking closure. async fn take_blocking_inner_with( &self, f: impl FnOnce(LoadedJSSandbox) -> napi::Result + Send + 'static, ) -> napi::Result { + let executing_flag = self.executing_flag.clone(); self.take_inner_with(async move |sandbox| { - tokio::task::spawn_blocking(move || f(sandbox)) + executing_flag.store(true, Ordering::Release); + let result = tokio::task::spawn_blocking(move || f(sandbox)) .await - .map_err(join_error)? + .map_err(join_error)?; + executing_flag.store(false, Ordering::Release); + result }) .await } diff --git a/src/js-host-api/tests/sandbox.test.js b/src/js-host-api/tests/sandbox.test.js index 396ee07..31c6969 100644 --- a/src/js-host-api/tests/sandbox.test.js +++ b/src/js-host-api/tests/sandbox.test.js @@ -1,5 +1,5 @@ // Basic sandbox functionality tests -import { describe, it, expect, beforeEach } from 'vitest'; +import { describe, it, expect, beforeEach, vi } from 'vitest'; import { SandboxBuilder } from '../lib.js'; import { expectThrowsWithCode, expectRejectsWithCode } from './test-helpers.js'; @@ -445,3 +445,114 @@ describe('LoadedJSSandbox.unload()', () => { expectThrowsWithCode(() => loaded.lastCallStats, 'ERR_CONSUMED'); }); }); + +// ── Host print function ────────────────────────────────────────────── + +describe('setHostPrintFn', () => { + it('should support method chaining', () => { + const builder = new SandboxBuilder(); + const returned = builder.setHostPrintFn(() => {}); + expect(returned).toBe(builder); + }); + + it('should receive console.log output from the guest', async () => { + const messages = []; + const builder = new SandboxBuilder().setHostPrintFn((msg) => { + messages.push(msg); + }); + const proto = await builder.build(); + const sandbox = await proto.loadRuntime(); + sandbox.addHandler( + 'handler', + `function handler(event) { + console.log("Hello from guest!"); + return event; + }` + ); + const loaded = await sandbox.getLoadedSandbox(); + await loaded.callHandler('handler', {}); + + expect(messages.join('')).toContain('Hello from guest!'); + }); + + it('should receive multiple console.log calls', async () => { + const messages = []; + const builder = new SandboxBuilder().setHostPrintFn((msg) => { + messages.push(msg); + }); + const proto = await builder.build(); + const sandbox = await proto.loadRuntime(); + sandbox.addHandler( + 'handler', + `function handler(event) { + console.log("first"); + console.log("second"); + console.log("third"); + return event; + }` + ); + const loaded = await sandbox.getLoadedSandbox(); + await loaded.callHandler('handler', {}); + + const combined = messages.join(''); + expect(combined).toContain('first'); + expect(combined).toContain('second'); + expect(combined).toContain('third'); + }); + + it('should use the last callback when set multiple times', async () => { + const firstMessages = []; + const secondMessages = []; + const builder = new SandboxBuilder() + .setHostPrintFn((msg) => firstMessages.push(msg)) + .setHostPrintFn((msg) => secondMessages.push(msg)); + const proto = await builder.build(); + const sandbox = await proto.loadRuntime(); + sandbox.addHandler( + 'handler', + `function handler(event) { + console.log("which callback?"); + return event; + }` + ); + const loaded = await sandbox.getLoadedSandbox(); + await loaded.callHandler('handler', {}); + + expect(firstMessages.length).toBe(0); + expect(secondMessages.join('')).toContain('which callback?'); + }); + + it('should continue guest execution when callback throws', async () => { + const builder = new SandboxBuilder().setHostPrintFn(() => { + throw new Error('print callback exploded'); + }); + const proto = await builder.build(); + const sandbox = await proto.loadRuntime(); + sandbox.addHandler( + 'handler', + `function handler(event) { + console.log("this will throw in the callback"); + return { survived: true }; + }` + ); + const loaded = await sandbox.getLoadedSandbox(); + + // Spy on console.error to suppress noise and verify the error is logged + const errorSpy = vi.spyOn(console, 'error').mockImplementation(() => {}); + try { + const result = await loaded.callHandler('handler', {}); + + // The JS wrapper catches the throw — guest continues normally + expect(result.survived).toBe(true); + expect(errorSpy).toHaveBeenCalledWith('Host print callback threw:', expect.any(Error)); + } finally { + errorSpy.mockRestore(); + } + }); + + it('should throw CONSUMED after build()', async () => { + const builder = new SandboxBuilder(); + await builder.build(); + expectThrowsWithCode(() => builder.setHostPrintFn(() => {}), 'ERR_CONSUMED'); + }); +});