diff --git a/.changeset/quickjs-timeout-host-calls.md b/.changeset/quickjs-timeout-host-calls.md new file mode 100644 index 000000000..19024cd46 --- /dev/null +++ b/.changeset/quickjs-timeout-host-calls.md @@ -0,0 +1,22 @@ +--- +'@tanstack/ai-isolate-quickjs': patch +--- + +Enforce `timeout` as a wall-clock deadline that covers host-call suspensions. + +Previously the only deadline mechanism was QuickJS's `setInterruptHandler`, which +fires only while the sandbox is stepping bytecode. While the isolate is +asyncify-suspended awaiting a host binding's Promise, no interrupt fires, so the +configured `timeout` was silently ignored: a run with `timeout: 100` and an +800 ms host binding resolved successfully after ~800 ms, and a hung host binding +could hang `execute()` indefinitely. + +`execute()` now races the run against a wall-clock timer, so `timeout` bounds the +entire execution including time spent suspended in a host call. On the deadline +it returns a normalized `TimeoutError` immediately. The suspended host Promise +cannot be cancelled and the VM cannot be freed mid-asyncify, so disposal and the +internal execution-queue turn are deferred until the orphaned run unwinds (the +still-armed interrupt handler aborts it the moment the host Promise settles), +which prevents a late host result from resuming a dead execution and avoids +leaking the context. CPU-bound loops are still interrupted as before and leave +the context reusable. The public API is unchanged. diff --git a/packages/ai-isolate-quickjs/src/error-normalizer.ts b/packages/ai-isolate-quickjs/src/error-normalizer.ts index 2e025d3ba..2cee57022 100644 --- a/packages/ai-isolate-quickjs/src/error-normalizer.ts +++ b/packages/ai-isolate-quickjs/src/error-normalizer.ts @@ -2,6 +2,7 @@ import type { NormalizedError } from '@tanstack/ai-code-mode' const MEMORY_LIMIT_ERROR = 'MemoryLimitError' const STACK_OVERFLOW_ERROR = 'StackOverflowError' +const TIMEOUT_ERROR = 'TimeoutError' /** * Whether this normalized error indicates the QuickJS VM should not be reused @@ -21,6 +22,17 @@ export function normalizeError(error: unknown): NormalizedError { const msg = error.message const lower = msg.toLowerCase() + // QuickJS raises `InternalError: interrupted` when the interrupt handler + // trips the execution deadline. Surface it as a timeout so a CPU-bound + // deadline and a wall-clock (host-call) deadline report the same error. + if (lower.includes('interrupted')) { + return { + name: TIMEOUT_ERROR, + message: 'Code execution exceeded timeout', + stack: error.stack, + } + } + if ( lower.includes('out of memory') || lower.includes('memory alloc') || diff --git a/packages/ai-isolate-quickjs/src/isolate-context.ts b/packages/ai-isolate-quickjs/src/isolate-context.ts index e23bcb76a..a34279395 100644 --- a/packages/ai-isolate-quickjs/src/isolate-context.ts +++ b/packages/ai-isolate-quickjs/src/isolate-context.ts @@ -10,6 +10,21 @@ import type { ExecutionResult, IsolateContext } from '@tanstack/ai-code-mode' */ let globalExecQueue: Promise = Promise.resolve() +/** Sentinel resolved by the wall-clock deadline race. */ +const TIMED_OUT = Symbol('quickjs-isolate-timeout') + +/** + * Grace window granted, after the wall-clock timer fires, for the in-VM + * interrupt handler to settle a CPU-bound run before it is treated as a host + * call still suspended in asyncify. A CPU-bound loop trips the interrupt at + * (almost) the same instant the timer fires; the grace keeps that path on the + * normal, reusable-context branch instead of orphaning the VM. + */ +const TIMEOUT_INTERRUPT_GRACE_MS = 10 + +const delay = (ms: number): Promise => + new Promise((resolve) => setTimeout(resolve, ms)) + /** * IsolateContext implementation using QuickJS WASM */ @@ -19,6 +34,13 @@ export class QuickJSIsolateContext implements IsolateContext { private readonly timeout: number private disposed = false private executing = false + private timedOut = false + /** + * A run that timed out while a host call was suspended in asyncify. The VM + * cannot be disposed until this settles, so disposal and the global-queue + * turn are handed off to a continuation on it. `null` when no run is orphaned. + */ + private orphanedRun: Promise | null = null constructor(vm: QuickJSAsyncContext, logs: Array, timeout: number) { this.vm = vm @@ -28,21 +50,14 @@ export class QuickJSIsolateContext implements IsolateContext { async execute(code: string): Promise> { if (this.disposed) { - return { - success: false, - error: { - name: 'DisposedError', - message: 'Context has been disposed', - }, - logs: [], - } + return this.disposedResult() } // Serialize through the global queue to prevent concurrent // WASM asyncify suspensions across contexts. - let resolve!: () => void + let releaseTurn!: () => void const myTurn = new Promise((r) => { - resolve = r + releaseTurn = r }) const waitForPrev = globalExecQueue globalExecQueue = myTurn @@ -51,106 +66,161 @@ export class QuickJSIsolateContext implements IsolateContext { // eslint-disable-next-line @typescript-eslint/no-unnecessary-condition -- dispose() may be called concurrently while awaiting the queue if (this.disposed) { - resolve() - return { - success: false, - error: { - name: 'DisposedError', - message: 'Context has been disposed', - }, - logs: [], - } + releaseTurn() + return this.disposedResult() } this.executing = true this.logs.length = 0 - const releaseVmAfterFatalLimit = () => { - if (this.disposed) return - try { - this.vm.runtime.setInterruptHandler(() => false) - } catch { - // ignore if runtime is already torn down - } - this.disposed = true - this.vm.dispose() - } + // Set when a wall-clock timeout leaves a host call suspended in asyncify: + // interrupt-handler reset, VM disposal, and the queue-turn release are then + // owned by the orphaned-run continuation rather than the finally block. + let deferredToOrphan = false - const fail = (error: unknown) => { + const fail = (error: unknown): ExecutionResult => { const normalized = normalizeError(error) if (isFatalQuickJSLimitError(normalized)) { - releaseVmAfterFatalLimit() + this.disposeVm() } return { - success: false as const, + success: false, error: normalized, logs: [...this.logs], } } - try { - const wrappedCode = wrapCode(code) - - const deadline = Date.now() + this.timeout - this.vm.runtime.setInterruptHandler(() => { - return Date.now() > deadline - }) + // Drives the wrapped code to completion and marshals its result. On a + // deadline interrupt `evalCodeAsync` resolves with an error result, which + // `unwrapResult` throws and `fail` normalizes. + const runToSettled = async (): Promise> => { + const result = await this.vm.evalCodeAsync(wrapCode(code)) try { - const result = await this.vm.evalCodeAsync(wrappedCode) + const promiseHandle = this.vm.unwrapResult(result) + + // evalCodeAsync returns a Promise handle (our wrapper is an async IIFE). + // Use resolvePromise + executePendingJobs to properly await the + // QuickJS promise without re-entering the WASM asyncify state. + const nativePromise = this.vm.resolvePromise(promiseHandle) + promiseHandle.dispose() + this.vm.runtime.executePendingJobs() + const resolvedResult = await nativePromise + + const valueHandle = this.vm.unwrapResult(resolvedResult) + const dumpedResult = this.vm.dump(valueHandle) + valueHandle.dispose() let parsedResult: T - try { - const promiseHandle = this.vm.unwrapResult(result) - - // evalCodeAsync returns a Promise handle (our wrapper is an async IIFE). - // Use resolvePromise + executePendingJobs to properly await the - // QuickJS promise without re-entering the WASM asyncify state. - const nativePromise = this.vm.resolvePromise(promiseHandle) - promiseHandle.dispose() - this.vm.runtime.executePendingJobs() - const resolvedResult = await nativePromise - - const valueHandle = this.vm.unwrapResult(resolvedResult) - const dumpedResult = this.vm.dump(valueHandle) - valueHandle.dispose() - - if (typeof dumpedResult === 'string') { - try { - parsedResult = JSON.parse(dumpedResult) as T - } catch { - parsedResult = dumpedResult as T - } - } else { + if (typeof dumpedResult === 'string') { + try { + parsedResult = JSON.parse(dumpedResult) as T + } catch { parsedResult = dumpedResult as T } + } else { + parsedResult = dumpedResult as T + } - return { - success: true, - value: parsedResult, - logs: [...this.logs], - } - } catch (unwrapError) { - return fail(unwrapError) + // The wall-clock deadline may have fired while this run was suspended in + // a host call; the caller already received the timeout result, so a late + // success must not be surfaced as if the execution had finished in time. + if (this.timedOut) { + return this.timeoutResult() } - } finally { - // fail() may set disposed when releasing the VM after memory/stack limit errors - // eslint-disable-next-line @typescript-eslint/no-unnecessary-condition -- disposed set in fail() - if (!this.disposed) { - this.vm.runtime.setInterruptHandler(() => false) + + return { + success: true, + value: parsedResult, + logs: [...this.logs], } + } catch (unwrapError) { + return fail(unwrapError) + } + } + + try { + // The interrupt handler only fires while the VM is stepping bytecode, so + // it cannot preempt a host binding whose Promise is awaited via asyncify. + // Keep it for CPU-bound loops, but also race the run against a wall-clock + // timer so `timeout` bounds the whole execution, host suspensions included. + const deadline = Date.now() + this.timeout + this.vm.runtime.setInterruptHandler(() => Date.now() > deadline) + + const run = runToSettled() + + let timeoutTimer: ReturnType | undefined + const wallClock = new Promise((resolve) => { + timeoutTimer = setTimeout(() => resolve(TIMED_OUT), this.timeout) + }) + + const outcome = await Promise.race([run, wallClock]) + if (timeoutTimer !== undefined) { + clearTimeout(timeoutTimer) + } + + if (outcome !== TIMED_OUT) { + return outcome + } + + // The interrupt fires at (almost) the same instant for a CPU-bound loop; + // give the in-VM interrupt a brief window to settle the run before + // treating this as a host call still suspended in asyncify. + const graced = await Promise.race([ + run, + delay(TIMEOUT_INTERRUPT_GRACE_MS).then( + (): typeof TIMED_OUT => TIMED_OUT, + ), + ]) + if (graced !== TIMED_OUT) { + return graced } + + // Still suspended in a host call. The VM cannot be disposed now (freeing + // it mid-asyncify corrupts the stack the suspended call resumes into) and + // the host Promise cannot be cancelled. Leave the interrupt handler armed + // so the VM aborts the instant the host Promise resolves and it resumes + // stepping, then dispose the VM and release the queue turn once the + // orphaned run unwinds. This keeps the single-asyncify-suspension + // invariant that the global queue exists to protect. + this.timedOut = true + deferredToOrphan = true + this.orphanedRun = run + .catch(() => undefined) + .then(() => { + this.disposeVm() + releaseTurn() + }) + + return this.timeoutResult() } catch (error) { return fail(error) } finally { + // On the orphan path the continuation owns interrupt-handler reset, VM + // disposal, and the queue-turn release; the still-armed handler is what + // lets the suspended run abort when it resumes. + // eslint-disable-next-line @typescript-eslint/no-unnecessary-condition -- disposed may be set by fail() after a fatal limit error + if (!deferredToOrphan && !this.disposed) { + this.vm.runtime.setInterruptHandler(() => false) + } this.executing = false - resolve() + if (!deferredToOrphan) { + releaseTurn() + } } } async dispose(): Promise { if (this.disposed) return + // A timed-out execution may have left a host call suspended in asyncify. + // Its continuation disposes the VM once the call unwinds; wait for that + // rather than freeing the VM out from under the suspended call. + if (this.orphanedRun) { + await this.orphanedRun + return + } + // If an execution is in flight, wait for the global queue to drain // before disposing the VM. Otherwise the asyncified callback would // try to access a freed context. @@ -158,7 +228,44 @@ export class QuickJSIsolateContext implements IsolateContext { await globalExecQueue } + // eslint-disable-next-line @typescript-eslint/no-unnecessary-condition -- the orphaned continuation may have disposed while we awaited the queue + if (this.disposed) return + this.disposed = true this.vm.dispose() } + + /** Reset the interrupt handler and tear the VM down exactly once. */ + private disposeVm(): void { + if (this.disposed) return + try { + this.vm.runtime.setInterruptHandler(() => false) + } catch { + // ignore if the runtime is already torn down + } + this.disposed = true + this.vm.dispose() + } + + private disposedResult(): ExecutionResult { + return { + success: false, + error: { + name: 'DisposedError', + message: 'Context has been disposed', + }, + logs: [], + } + } + + private timeoutResult(): ExecutionResult { + return { + success: false, + error: { + name: 'TimeoutError', + message: `Code execution exceeded timeout of ${this.timeout}ms`, + }, + logs: [...this.logs], + } + } } diff --git a/packages/ai-isolate-quickjs/tests/isolate-driver.test.ts b/packages/ai-isolate-quickjs/tests/isolate-driver.test.ts index eb1506afb..bfd801415 100644 --- a/packages/ai-isolate-quickjs/tests/isolate-driver.test.ts +++ b/packages/ai-isolate-quickjs/tests/isolate-driver.test.ts @@ -152,6 +152,108 @@ describe('createQuickJSIsolateDriver', () => { }) }) + describe('execute - timeout across host calls', () => { + const sleep = (ms: number) => new Promise((r) => setTimeout(r, ms)) + + it('enforces the wall-clock timeout while a host call is suspended', async () => { + const hostDurationMs = 800 + const slow = makeBinding('slow', async () => { + await sleep(hostDurationMs) + return { done: true } + }) + const driver = createQuickJSIsolateDriver({ timeout: 100 }) + const context = await driver.createContext({ bindings: { slow } }) + + const startedAt = Date.now() + const result = await context.execute('return await slow({})') + const elapsedMs = Date.now() - startedAt + + expect(result.success).toBe(false) + expect(result.error?.name).toBe('TimeoutError') + // Returns at the deadline, not after the full host call completes. + expect(elapsedMs).toBeLessThan(hostDurationMs / 2) + }) + + it('still interrupts a CPU-bound loop and keeps the context reusable', async () => { + const driver = createQuickJSIsolateDriver({ timeout: 50 }) + const context = await driver.createContext({ bindings: {} }) + + const startedAt = Date.now() + const result = await context.execute(` + const start = Date.now(); + while (Date.now() - start < 5000) { + // spin + } + return 1; + `) + const elapsedMs = Date.now() - startedAt + + expect(result.success).toBe(false) + expect(result.error?.name).toBe('TimeoutError') + expect(elapsedMs).toBeLessThan(2000) + + // The in-VM interrupt path leaves the context usable, unlike a + // host-call timeout (which must dispose the suspended VM). + const reuse = await context.execute('return 7') + expect(reuse.success).toBe(true) + expect(reuse.value).toBe(7) + await context.dispose() + }) + + it('does not crash, resume, or unhandled-reject when the host call settles after the timeout', async () => { + const rejections: Array = [] + const onRejection = (reason: unknown) => rejections.push(reason) + process.on('unhandledRejection', onRejection) + + try { + let hostSettled = false + const slow = makeBinding('slow', async () => { + await sleep(300) + hostSettled = true + return { late: true } + }) + const driver = createQuickJSIsolateDriver({ timeout: 50 }) + const context = await driver.createContext({ bindings: { slow } }) + + const result = await context.execute('return await slow({})') + expect(result.success).toBe(false) + expect(result.error?.name).toBe('TimeoutError') + + // Let the orphaned host promise settle so the VM resumes and unwinds. + await sleep(500) + expect(hostSettled).toBe(true) + + // The late resumption disposed the VM; further execution is rejected + // cleanly rather than resuming the dead run. + const after = await context.execute('return 1') + expect(after.success).toBe(false) + expect(after.error?.name).toBe('DisposedError') + + await sleep(10) + } finally { + process.off('unhandledRejection', onRejection) + } + + expect(rejections).toEqual([]) + }) + + it('dispose after a host-call timeout waits for the orphaned run and resolves cleanly', async () => { + const slow = makeBinding('slow', async () => { + await sleep(300) + return { late: true } + }) + const driver = createQuickJSIsolateDriver({ timeout: 50 }) + const context = await driver.createContext({ bindings: { slow } }) + + const result = await context.execute('return await slow({})') + expect(result.error?.name).toBe('TimeoutError') + + // dispose() must wait for the suspended host call to unwind before + // freeing the VM; it resolves without throwing and leaks no context. + await expect(context.dispose()).resolves.toBeUndefined() + }) + }) + describe('execute - error handling', () => { it('returns error for syntax errors', async () => { const driver = createQuickJSIsolateDriver()