From de54b9a87071db4e56f4e7adc0d0ddd123b959c9 Mon Sep 17 00:00:00 2001 From: Waleed Latif Date: Sat, 7 Nov 2026 14:03:14 -0800 Subject: [PATCH 1/4] fix(sandbox): drop in-flight fetches once the pool gives up or the time budget passes --- apps/sim/lib/execution/isolated-vm.ts | 44 +++++++--- .../scripts/test-isolated-vm-remote-e2e.ts | 82 +++++++++++++++++++ 2 files changed, 113 insertions(+), 13 deletions(-) diff --git a/apps/sim/lib/execution/isolated-vm.ts b/apps/sim/lib/execution/isolated-vm.ts index 31a1d6ed253..dfcb93f32ad 100644 --- a/apps/sim/lib/execution/isolated-vm.ts +++ b/apps/sim/lib/execution/isolated-vm.ts @@ -190,8 +190,14 @@ interface PendingExecution { timeout: ReturnType ownerKey: string brokers?: Record - /** Set when the caller aborts. Broker dispatches and the final result stop resolving the promise. */ - cancelled: boolean + /** + * Aborted once the pool gives up on the execution (caller abort, timeout, worker loss). Broker + * dispatches and the final result stop resolving the promise, and in-flight `fetch` calls are + * dropped, since their responses can no longer reach the isolate. + */ + abort: AbortController + /** How long the pool waits for a result before giving up; also the longest any one `fetch` may run. */ + hostTimeoutMs: number /** Number of broker calls made so far for this execution. */ brokerCallCount: number } @@ -387,7 +393,8 @@ function normalizeFetchOptions(options?: IsolatedFetchOptions): SecureFetchOptio async function secureFetch( requestId: string, url: string, - options?: IsolatedFetchOptions + options: IsolatedFetchOptions | undefined, + signal: AbortSignal ): Promise { if (url.length > MAX_FETCH_URL_LENGTH) { return JSON.stringify({ @@ -398,7 +405,7 @@ async function secureFetch( try { const response = await secureFetchWithValidation( url, - normalizeFetchOptions(options), + { ...normalizeFetchOptions(options), signal }, 'fetchUrl' ) const bodyResult = truncateString(await response.text(), MAX_FETCH_RESPONSE_CHARS) @@ -758,7 +765,7 @@ function handleBrokerMessage( return } - if (pending.cancelled) { + if (pending.abort.signal.aborted) { sendResponse({ error: 'Execution cancelled' }) return } @@ -801,7 +808,7 @@ function handleBrokerMessage( Promise.resolve() .then(() => pending.runInOutboundScope(() => handler(args, brokerId))) .then((resultValue) => { - if (pending.cancelled) { + if (pending.abort.signal.aborted) { sendResponse({ error: 'Execution cancelled' }) return } @@ -864,7 +871,7 @@ function handleWorkerMessage(workerId: number, message: unknown) { // If the caller aborted, the outer Promise is already resolved with // AbortError. Still run all the bookkeeping above so pool counters stay // accurate; just skip re-resolving. - if (!pending.cancelled) { + if (!pending.abort.signal.aborted) { pending.resolve(msg.result as IsolatedVMExecutionResult) } drainQueue() @@ -895,7 +902,7 @@ function handleWorkerMessage(workerId: number, message: unknown) { typeof msg.executionId === 'number' ? workerInfo?.pendingExecutions.get(msg.executionId) : undefined - if (!pending || pending.cancelled) { + if (!pending || pending.abort.signal.aborted) { sendFetchResponse(JSON.stringify({ error: 'Execution no longer active' })) return } @@ -926,8 +933,14 @@ function handleWorkerMessage(workerId: number, message: unknown) { return } } + // A fetch the script left running after returning is cut off once the execution's own time + // budget has passed: nothing can read its response. + const signal = AbortSignal.any([ + pending.abort.signal, + AbortSignal.timeout(pending.hostTimeoutMs), + ]) pending - .runInOutboundScope(() => secureFetch(requestId, url, options)) + .runInOutboundScope(() => secureFetch(requestId, url, options, signal)) .then(sendFetchResponse) .catch((err) => { sendFetchResponse(JSON.stringify({ error: getErrorMessage(err, 'Fetch failed') })) @@ -971,6 +984,7 @@ function cleanupWorker(workerId: number, interruptedMessage?: string) { for (const [id, pending] of workerInfo.pendingExecutions) { clearTimeout(pending.timeout) + pending.abort.abort() totalActiveExecutions-- const owner = ownerStates.get(pending.ownerKey) if (owner) { @@ -1316,8 +1330,11 @@ function dispatchToWorker( workerInfo.idleTimeout = null } + const abort = new AbortController() + const hostTimeoutMs = req.timeoutMs + 1000 const timeout = setTimeout(() => { workerInfo.pendingExecutions.delete(execId) + abort.abort() // A never-settling promise escapes the isolate's own script timeout, so the worker // would otherwise keep the isolate and its timers alive until the worker retires. sendCancel(workerInfo, execId) @@ -1345,7 +1362,7 @@ function dispatchToWorker( resetWorkerIdleTimeout(workerInfo.id) } drainQueue() - }, req.timeoutMs + 1000) + }, hostTimeoutMs) workerInfo.pendingExecutions.set(execId, { runInOutboundScope: state.runInOutboundScope, @@ -1353,7 +1370,8 @@ function dispatchToWorker( timeout, ownerKey: ownerState.ownerKey, brokers, - cancelled: false, + abort, + hostTimeoutMs, brokerCallCount: 0, }) workerInfo.activeExecutions++ @@ -1687,13 +1705,13 @@ export async function executeInIsolatedVM( if (removed) clearTimeout(removed.queueTimeout) state.queueId = undefined } - // If dispatched, mark the pending entry cancelled and ask the worker to + // If dispatched, abort the pending entry and ask the worker to // dispose its isolate so the pool slot can be released. The worker will // emit a `result` shortly after, which runs the normal counter cleanup. if (state.workerId !== undefined && state.execId !== undefined) { const wi = workers.get(state.workerId) const pending = wi?.pendingExecutions.get(state.execId) - if (pending) pending.cancelled = true + pending?.abort.abort() if (wi) sendCancel(wi, state.execId) } resolveWithRelease({ diff --git a/apps/sim/scripts/test-isolated-vm-remote-e2e.ts b/apps/sim/scripts/test-isolated-vm-remote-e2e.ts index d09fe1e54d9..a6aceb24f12 100644 --- a/apps/sim/scripts/test-isolated-vm-remote-e2e.ts +++ b/apps/sim/scripts/test-isolated-vm-remote-e2e.ts @@ -508,6 +508,88 @@ async function run() { assert.equal(next.result, 'still alive', next.error?.message) }) + /** A server that never answers, so a fetch to it stays in flight until the app drops it. */ + const stalledServer = async () => { + const reached = Promise.withResolvers() + const dropped = Promise.withResolvers() + const server = createHttpServer((req) => { + req.socket.once('close', () => dropped.resolve()) + reached.resolve() + }) + await new Promise((resolve) => server.listen(0, '127.0.0.1', resolve)) + const address = server.address() + assert(address && typeof address === 'object') + return { + url: `http://127.0.0.1:${address.port}/`, + reached: reached.promise, + /** Whether the app closed the request within `withinMs`. */ + droppedSoon: (withinMs = 2000) => + Promise.race([dropped.promise.then(() => true), sleep(withinMs).then(() => false)]), + close: () => { + server.closeAllConnections() + server.close() + }, + } + } + type StalledServer = Awaited> + + const checkFetchDropped = ( + name: string, + endExecution: (server: StalledServer) => Promise + ) => + check(name, async () => { + const server = await stalledServer() + try { + await endExecution(server) + assert(await server.droppedSoon(), 'The app kept the fetch open') + } finally { + server.close() + } + }) + + await check('drops the in-flight fetch of a timed-out execution at the timeout', async () => { + const server = await stalledServer() + try { + // Started late in the run, so only dropping it at the timeout, not at its own time + // budget, closes it promptly. + const result = await execute( + `await fetch('http://127.0.0.1:${slowAddress.port}/?ms=800'); await fetch('${server.url}')`, + { timeoutMs: 1000 } + ) + assert.equal(result.termination, 'timeout', result.error?.message) + assert(await server.droppedSoon(400), 'The timed-out execution kept its fetch open') + } finally { + server.close() + } + }) + + await checkFetchDropped('drops the in-flight fetch of a cancelled execution', async (server) => { + const controller = new AbortController() + const pending = execute(`await fetch('${server.url}')`, { signal: controller.signal }) + await server.reached + controller.abort() + assert.equal((await pending).termination, 'cancelled') + }) + + await checkFetchDropped('drops the in-flight fetch when its worker dies', async (server) => { + const pending = execute(`await fetch('${server.url}')`) + await server.reached + for (const pid of workerPidsOf(host.process.pid ?? -1)) process.kill(pid, 'SIGKILL') + assert.equal((await pending).error?.isSystemError, true) + }) + + await checkFetchDropped( + 'sends a fetch the script left running, then drops it after the time budget', + async (server) => { + const result = await execute( + `fetch('${server.url}', { method: 'POST', body: 'ping' }); return 1`, + { timeoutMs: 500 } + ) + assert.equal(result.result, 1, result.error?.message) + await server.reached + } + ) + const runTask = (code: string) => execute(code, { task: { From 248c12b322b36e0059e98dd1f99e7a2cd9746ddb Mon Sep 17 00:00:00 2001 From: Waleed Latif Date: Sat, 7 Nov 2026 14:13:28 -0800 Subject: [PATCH 2/4] fix(sandbox): use manual resolvers in the stalled-server e2e helper --- .../sim/scripts/test-isolated-vm-remote-e2e.ts | 18 ++++++++++++------ 1 file changed, 12 insertions(+), 6 deletions(-) diff --git a/apps/sim/scripts/test-isolated-vm-remote-e2e.ts b/apps/sim/scripts/test-isolated-vm-remote-e2e.ts index a6aceb24f12..598057dcf0d 100644 --- a/apps/sim/scripts/test-isolated-vm-remote-e2e.ts +++ b/apps/sim/scripts/test-isolated-vm-remote-e2e.ts @@ -510,21 +510,27 @@ async function run() { /** A server that never answers, so a fetch to it stays in flight until the app drops it. */ const stalledServer = async () => { - const reached = Promise.withResolvers() - const dropped = Promise.withResolvers() + let markReached = () => {} + let markDropped = () => {} + const reached = new Promise((resolve) => { + markReached = resolve + }) + const dropped = new Promise((resolve) => { + markDropped = resolve + }) const server = createHttpServer((req) => { - req.socket.once('close', () => dropped.resolve()) - reached.resolve() + req.socket.once('close', markDropped) + markReached() }) await new Promise((resolve) => server.listen(0, '127.0.0.1', resolve)) const address = server.address() assert(address && typeof address === 'object') return { url: `http://127.0.0.1:${address.port}/`, - reached: reached.promise, + reached, /** Whether the app closed the request within `withinMs`. */ droppedSoon: (withinMs = 2000) => - Promise.race([dropped.promise.then(() => true), sleep(withinMs).then(() => false)]), + Promise.race([dropped.then(() => true), sleep(withinMs).then(() => false)]), close: () => { server.closeAllConnections() server.close() From 1bfe3f3d67bf822b9e0127f54e09f01a73814070 Mon Sep 17 00:00:00 2001 From: Waleed Latif Date: Sat, 7 Nov 2026 14:22:25 -0800 Subject: [PATCH 3/4] fix(sandbox): drop in-flight fetches when the isolate reports a timeout --- apps/sim/lib/execution/isolated-vm.ts | 5 ++- .../scripts/test-isolated-vm-remote-e2e.ts | 33 ++++++++++++------- 2 files changed, 25 insertions(+), 13 deletions(-) diff --git a/apps/sim/lib/execution/isolated-vm.ts b/apps/sim/lib/execution/isolated-vm.ts index dfcb93f32ad..edd1a0ce5da 100644 --- a/apps/sim/lib/execution/isolated-vm.ts +++ b/apps/sim/lib/execution/isolated-vm.ts @@ -871,9 +871,12 @@ function handleWorkerMessage(workerId: number, message: unknown) { // If the caller aborted, the outer Promise is already resolved with // AbortError. Still run all the bookkeeping above so pool counters stay // accurate; just skip re-resolving. + const result = msg.result as IsolatedVMExecutionResult if (!pending.abort.signal.aborted) { - pending.resolve(msg.result as IsolatedVMExecutionResult) + pending.resolve(result) } + // The isolate's own timeout gives up on the execution just as the pool's does. + if (result.termination === 'timeout') pending.abort.abort() drainQueue() } return diff --git a/apps/sim/scripts/test-isolated-vm-remote-e2e.ts b/apps/sim/scripts/test-isolated-vm-remote-e2e.ts index 598057dcf0d..0513a9d647d 100644 --- a/apps/sim/scripts/test-isolated-vm-remote-e2e.ts +++ b/apps/sim/scripts/test-isolated-vm-remote-e2e.ts @@ -541,33 +541,42 @@ async function run() { const checkFetchDropped = ( name: string, - endExecution: (server: StalledServer) => Promise + endExecution: (server: StalledServer) => Promise, + withinMs?: number ) => check(name, async () => { const server = await stalledServer() try { await endExecution(server) - assert(await server.droppedSoon(), 'The app kept the fetch open') + assert(await server.droppedSoon(withinMs), 'The app kept the fetch open') } finally { server.close() } }) - await check('drops the in-flight fetch of a timed-out execution at the timeout', async () => { - const server = await stalledServer() - try { + await checkFetchDropped( + 'drops the in-flight fetch of an execution the app timed out', + async (server) => { // Started late in the run, so only dropping it at the timeout, not at its own time - // budget, closes it promptly. + // budget, closes it within the grace period. const result = await execute( - `await fetch('http://127.0.0.1:${slowAddress.port}/?ms=800'); await fetch('${server.url}')`, + `await fetch('http://127.0.0.1:${slowAddress.port}/?ms=800'); fetch('${server.url}'); await new Promise(() => {})`, { timeoutMs: 1000 } ) assert.equal(result.termination, 'timeout', result.error?.message) - assert(await server.droppedSoon(400), 'The timed-out execution kept its fetch open') - } finally { - server.close() - } - }) + }, + 400 + ) + + await checkFetchDropped( + 'drops the in-flight fetch of an execution the isolate timed out', + async (server) => { + // The isolate's own timeout ends a synchronous spin a second before the pool would. + const result = await execute(`fetch('${server.url}'); for (;;) {}`, { timeoutMs: 1000 }) + assert.equal(result.termination, 'timeout', result.error?.message) + }, + 400 + ) await checkFetchDropped('drops the in-flight fetch of a cancelled execution', async (server) => { const controller = new AbortController() From 88442a212ea0fc3c0f173e1851123ec05edee0f4 Mon Sep 17 00:00:00 2001 From: Waleed Latif Date: Sat, 7 Nov 2026 14:29:14 -0800 Subject: [PATCH 4/4] test(sandbox): pin that a fire-and-forget fetch survives a normal return --- apps/sim/scripts/test-isolated-vm-remote-e2e.ts | 5 +++++ 1 file changed, 5 insertions(+) diff --git a/apps/sim/scripts/test-isolated-vm-remote-e2e.ts b/apps/sim/scripts/test-isolated-vm-remote-e2e.ts index 0513a9d647d..e4ea4cc309f 100644 --- a/apps/sim/scripts/test-isolated-vm-remote-e2e.ts +++ b/apps/sim/scripts/test-isolated-vm-remote-e2e.ts @@ -602,6 +602,11 @@ async function run() { ) assert.equal(result.result, 1, result.error?.message) await server.reached + assert.equal( + await server.droppedSoon(300), + false, + 'The app dropped the fetch as soon as the script returned' + ) } )