Skip to content

Commit 63487f7

Browse files
authored
fix(sandbox): cap task timer bridges and dispose isolates on host timeout (#9776)
* fix(sandbox): cap task timer bridges and dispose isolates on host timeout * improvement(sandbox): share timer admission and worker cancel helpers * improvement(sandbox): cover interval reuse in the timer cap e2e check
1 parent 83914e7 commit 63487f7

4 files changed

Lines changed: 115 additions & 9 deletions

File tree

‎apps/sim/lib/execution/isolated-vm-worker.cjs‎

Lines changed: 38 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -15,6 +15,14 @@ let brokerIdCounter = 0
1515
const FETCH_TIMEOUT_MS = 300000 // 5 minutes
1616
const BROKER_TIMEOUT_MS = 300000
1717
const MAX_STDOUT_CHARS = Number.parseInt(process.env.IVM_MAX_STDOUT_CHARS || '', 10) || 200000
18+
/**
19+
* Task-mode timer bridges keep a Node `Timeout`, a Map entry, and an `ivm.Reference`
20+
* per scheduled timer on the shared worker heap, outside the isolate's `memoryLimit`.
21+
* These ceilings charge a timer flood to the offending execution instead of every
22+
* co-tenant on the worker.
23+
*/
24+
const MAX_PENDING_TIMERS = 1000
25+
const MAX_ACTIVE_INTERVALS = 50
1826
const MAX_FETCH_OPTIONS_JSON_CHARS =
1927
Number.parseInt(process.env.IVM_MAX_FETCH_OPTIONS_JSON_CHARS || '', 10) || 256 * 1024
2028

@@ -757,7 +765,36 @@ async function executeTask(request, executionId) {
757765
return v
758766
}
759767

768+
// Thrown into the isolate, so the stack must not carry worker host frames.
769+
const timerLimitError = (fnRef, message) => {
770+
try {
771+
fnRef.release()
772+
} catch {}
773+
const error = new RangeError(message)
774+
error.stack = `RangeError: ${message}`
775+
return error
776+
}
777+
const admitTimer = (fnRef, recurring) => {
778+
if (timers.size >= MAX_PENDING_TIMERS) {
779+
throw timerLimitError(
780+
fnRef,
781+
`Too many pending timers (limit ${MAX_PENDING_TIMERS} per execution)`
782+
)
783+
}
784+
if (!recurring) return
785+
let activeIntervals = 0
786+
for (const entry of timers.values()) {
787+
if (entry.recurring && ++activeIntervals >= MAX_ACTIVE_INTERVALS) {
788+
throw timerLimitError(
789+
fnRef,
790+
`Too many active intervals (limit ${MAX_ACTIVE_INTERVALS} per execution)`
791+
)
792+
}
793+
}
794+
}
795+
760796
const setTimeoutRef = new ivm.Reference((fnRef, msRef) => {
797+
admitTimer(fnRef, false)
761798
const id = nextTimerId++
762799
const delay = Math.max(0, Math.min(Number(unwrapPrimitive(msRef)) || 0, timeoutMs))
763800
const nodeTimer = setTimeout(() => {
@@ -797,6 +834,7 @@ async function executeTask(request, executionId) {
797834
await jail.set('__clearTimeoutRef', clearTimeoutRef)
798835

799836
const setIntervalRef = new ivm.Reference((fnRef, msRef) => {
837+
admitTimer(fnRef, true)
800838
const id = nextTimerId++
801839
const delay = Math.max(1, Math.min(Number(unwrapPrimitive(msRef)) || 1, timeoutMs))
802840
const nodeTimer = setInterval(() => {

‎apps/sim/lib/execution/isolated-vm.test.ts‎

Lines changed: 24 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -87,6 +87,8 @@ type ControllableReadyProc = {
8787
spawn: SpawnFactory
8888
dispatched: Promise<void>
8989
release: (result?: unknown) => void
90+
/** Every message the pool sent the worker, in order. */
91+
sent: Array<{ type?: string; executionId?: number }>
9092
}
9193

9294
/**
@@ -104,11 +106,13 @@ function createControllableReadyProc(): ControllableReadyProc {
104106
})
105107
let proc: MockProc | undefined
106108
let lastExecutionId = 0
109+
const sent: ControllableReadyProc['sent'] = []
107110

108111
const spawn: SpawnFactory = () => {
109112
const current = createBaseProc()
110113
current.send = (message: unknown) => {
111114
const msg = message as { type?: string; executionId?: number }
115+
sent.push(msg)
112116
if (msg.type === 'execute') {
113117
lastExecutionId = msg.executionId ?? 0
114118
markDispatched()
@@ -128,7 +132,7 @@ function createControllableReadyProc(): ControllableReadyProc {
128132
})
129133
}
130134

131-
return { spawn, dispatched, release }
135+
return { spawn, dispatched, release, sent }
132136
}
133137

134138
function createReadyFetchProxyProc(fetchMessage: { url: string; optionsJson?: string }): MockProc {
@@ -336,6 +340,25 @@ describe('isolated-vm scheduler', () => {
336340
expect(spawnMock).toHaveBeenCalledTimes(2)
337341
})
338342

343+
it('disposes the worker execution when the host timeout fires', async () => {
344+
const { spawn, sent } = createControllableReadyProc()
345+
const { executeInIsolatedVM } = await loadExecutionModule({ spawns: [spawn] })
346+
347+
const result = await executeInIsolatedVM({
348+
code: 'await new Promise(() => {})',
349+
params: {},
350+
envVars: {},
351+
contextVariables: {},
352+
timeoutMs: 10,
353+
requestId: 'req-hung',
354+
})
355+
356+
expect(result.termination).toBe('timeout')
357+
const executed = sent.find((message) => message.type === 'execute')
358+
expect(executed?.executionId).toBeDefined()
359+
expect(sent).toContainEqual({ type: 'cancel', executionId: executed?.executionId })
360+
})
361+
339362
it('spawns workers with an allowlisted env, never the parent process.env', async () => {
340363
const ALLOWED_WORKER_ENV_KEYS = new Set([
341364
'PATH',

‎apps/sim/lib/execution/isolated-vm.ts‎

Lines changed: 13 additions & 7 deletions
Original file line numberDiff line numberDiff line change
@@ -1233,6 +1233,15 @@ function spawnWorker(): Promise<WorkerInfo> {
12331233
* Returns the ready worker with the fewest active executions that still
12341234
* has capacity, or null if none available.
12351235
*/
1236+
/** Asks the worker to dispose an execution's isolate, sweeping its timers and pending calls. */
1237+
function sendCancel(workerInfo: WorkerInfo, executionId: number) {
1238+
try {
1239+
workerInfo.process.send({ type: 'cancel', executionId })
1240+
} catch (err) {
1241+
logger.warn('Failed to send cancel to worker', { err, workerId: workerInfo.id })
1242+
}
1243+
}
1244+
12361245
function selectWorker(): WorkerInfo | null {
12371246
let best: WorkerInfo | null = null
12381247
for (const w of workers.values()) {
@@ -1309,6 +1318,9 @@ function dispatchToWorker(
13091318

13101319
const timeout = setTimeout(() => {
13111320
workerInfo.pendingExecutions.delete(execId)
1321+
// A never-settling promise escapes the isolate's own script timeout, so the worker
1322+
// would otherwise keep the isolate and its timers alive until the worker retires.
1323+
sendCancel(workerInfo, execId)
13121324
workerInfo.activeExecutions--
13131325
totalActiveExecutions--
13141326
ownerState.activeExecutions = Math.max(0, ownerState.activeExecutions - 1)
@@ -1682,13 +1694,7 @@ export async function executeInIsolatedVM(
16821694
const wi = workers.get(state.workerId)
16831695
const pending = wi?.pendingExecutions.get(state.execId)
16841696
if (pending) pending.cancelled = true
1685-
if (wi) {
1686-
try {
1687-
wi.process.send({ type: 'cancel', executionId: state.execId })
1688-
} catch (err) {
1689-
logger.warn('Failed to send cancel to worker', { err, workerId: state.workerId })
1690-
}
1691-
}
1697+
if (wi) sendCancel(wi, state.execId)
16921698
}
16931699
resolveWithRelease({
16941700
result: null,

‎apps/sim/scripts/test-isolated-vm-remote-e2e.ts‎

Lines changed: 40 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -9,6 +9,7 @@ import { createLogger } from '@sim/logger'
99
import { getErrorMessage } from '@sim/utils/errors'
1010
import { sleep } from '@sim/utils/helpers'
1111
import { generateRandomBytes, generateRandomHex } from '@sim/utils/random'
12+
import type { IsolatedVMExecutionRequest } from '@/lib/execution/isolated-vm'
1213

1314
/**
1415
* Runs the isolated-vm pool against a real `isolated-vm-host.cjs` over real
@@ -383,7 +384,14 @@ async function run() {
383384
const slowFetch = (ms: number) =>
384385
`const r = await fetch('http://127.0.0.1:${slowAddress.port}/?ms=${ms}'); return await r.text()`
385386

386-
const execute = (code: string, extra: { timeoutMs?: number; signal?: AbortSignal } = {}) =>
387+
const execute = (
388+
code: string,
389+
extra: {
390+
timeoutMs?: number
391+
signal?: AbortSignal
392+
task?: IsolatedVMExecutionRequest['task']
393+
} = {}
394+
) =>
387395
executeInIsolatedVM(
388396
{
389397
code,
@@ -393,6 +401,7 @@ async function run() {
393401
timeoutMs: extra.timeoutMs ?? 10000,
394402
requestId: `e2e-${generateRandomHex(4)}`,
395403
ownerKey: 'e2e-owner',
404+
task: extra.task,
396405
},
397406
{ signal: extra.signal }
398407
)
@@ -499,6 +508,36 @@ async function run() {
499508
assert.equal(next.result, 'still alive', next.error?.message)
500509
})
501510

511+
const runTask = (code: string) =>
512+
execute(code, {
513+
task: {
514+
id: 'e2e-timers',
515+
bundles: [],
516+
bootstrap: '',
517+
brokers: [],
518+
finalize: 'return new Uint8Array([1])',
519+
},
520+
})
521+
522+
await check('caps the timers a task may hold on the shared worker', async () => {
523+
const timeouts = await runTask(
524+
'const f = () => {}; for (let i = 0; i < 200000; i++) setTimeout(f, 60000)'
525+
)
526+
assert.match(timeouts.error?.message ?? '', /Too many pending timers/, timeouts.error?.message)
527+
const intervals = await runTask(
528+
'const f = () => {}; for (let i = 0; i < 200; i++) setInterval(f, 1000)'
529+
)
530+
assert.match(
531+
intervals.error?.message ?? '',
532+
/Too many active intervals/,
533+
intervals.error?.message
534+
)
535+
const reused = await runTask(
536+
'for (let round = 0; round < 3; round++) { const ids = []; for (let i = 0; i < 1000; i++) ids.push(setTimeout(() => {}, 60000)); ids.forEach(clearTimeout); const intervals = []; for (let i = 0; i < 50; i++) intervals.push(setInterval(() => {}, 60000)); intervals.forEach(clearInterval) }'
537+
)
538+
assert.equal(reused.error, undefined, reused.error?.message)
539+
})
540+
502541
await check('rejects a connection with the wrong token', async () => {
503542
const ws = await openSocket(probeHost, authFrame(generateRandomHex(32)))
504543
assert.equal(await closeCode(ws), 4401)

0 commit comments

Comments
 (0)