Skip to content

Commit 4944ed5

Browse files
committed
fix(sandbox): cap task timer bridges and dispose isolates on host timeout
1 parent 472399b commit 4944ed5

4 files changed

Lines changed: 106 additions & 0 deletions

File tree

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

Lines changed: 34 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

@@ -662,6 +670,7 @@ async function executeTask(request, executionId) {
662670
// sweep regardless of where execution throws.
663671
let nextTimerId = 1
664672
const timers = new Map()
673+
let activeIntervals = 0
665674
const cleanupTimers = () => {
666675
for (const entry of timers.values()) {
667676
try {
@@ -673,6 +682,7 @@ async function executeTask(request, executionId) {
673682
} catch {}
674683
}
675684
timers.clear()
685+
activeIntervals = 0
676686
}
677687

678688
// Phase timings (ms). Populated inline during execution; returned in
@@ -757,7 +767,20 @@ async function executeTask(request, executionId) {
757767
return v
758768
}
759769

770+
// Thrown into the isolate, so the stack must not carry worker host frames.
771+
const rejectTimer = (fnRef, message) => {
772+
try {
773+
fnRef.release()
774+
} catch {}
775+
const error = new RangeError(message)
776+
error.stack = `RangeError: ${message}`
777+
throw error
778+
}
779+
760780
const setTimeoutRef = new ivm.Reference((fnRef, msRef) => {
781+
if (timers.size >= MAX_PENDING_TIMERS) {
782+
rejectTimer(fnRef, `Too many pending timers (limit ${MAX_PENDING_TIMERS} per execution)`)
783+
}
761784
const id = nextTimerId++
762785
const delay = Math.max(0, Math.min(Number(unwrapPrimitive(msRef)) || 0, timeoutMs))
763786
const nodeTimer = setTimeout(() => {
@@ -792,11 +815,21 @@ async function executeTask(request, executionId) {
792815
entry.fnRef.release()
793816
} catch {}
794817
timers.delete(key)
818+
if (entry.recurring) activeIntervals--
795819
})
796820
releaseables.push(clearTimeoutRef)
797821
await jail.set('__clearTimeoutRef', clearTimeoutRef)
798822

799823
const setIntervalRef = new ivm.Reference((fnRef, msRef) => {
824+
if (timers.size >= MAX_PENDING_TIMERS) {
825+
rejectTimer(fnRef, `Too many pending timers (limit ${MAX_PENDING_TIMERS} per execution)`)
826+
}
827+
if (activeIntervals >= MAX_ACTIVE_INTERVALS) {
828+
rejectTimer(
829+
fnRef,
830+
`Too many active intervals (limit ${MAX_ACTIVE_INTERVALS} per execution)`
831+
)
832+
}
800833
const id = nextTimerId++
801834
const delay = Math.max(1, Math.min(Number(unwrapPrimitive(msRef)) || 1, timeoutMs))
802835
const nodeTimer = setInterval(() => {
@@ -810,6 +843,7 @@ async function executeTask(request, executionId) {
810843
}
811844
}, delay)
812845
timers.set(id, { nodeTimer, fnRef, recurring: true })
846+
activeIntervals++
813847
return id
814848
})
815849
releaseables.push(setIntervalRef)

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

Lines changed: 28 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -336,6 +336,34 @@ describe('isolated-vm scheduler', () => {
336336
expect(spawnMock).toHaveBeenCalledTimes(2)
337337
})
338338

339+
it('disposes the worker execution when the host timeout fires', async () => {
340+
const sent: Array<{ type?: string; executionId?: number }> = []
341+
const hungWorker = () => {
342+
const proc = createBaseProc()
343+
proc.send = (message: unknown) => {
344+
sent.push(message as { type?: string; executionId?: number })
345+
return true
346+
}
347+
setImmediate(() => proc.emit('message', { type: 'ready' }))
348+
return proc
349+
}
350+
const { executeInIsolatedVM } = await loadExecutionModule({ spawns: [hungWorker] })
351+
352+
const result = await executeInIsolatedVM({
353+
code: 'await new Promise(() => {})',
354+
params: {},
355+
envVars: {},
356+
contextVariables: {},
357+
timeoutMs: 10,
358+
requestId: 'req-hung',
359+
})
360+
361+
expect(result.termination).toBe('timeout')
362+
const executed = sent.find((message) => message.type === 'execute')
363+
expect(executed?.executionId).toBeDefined()
364+
expect(sent).toContainEqual({ type: 'cancel', executionId: executed?.executionId })
365+
})
366+
339367
it('spawns workers with an allowlisted env, never the parent process.env', async () => {
340368
const ALLOWED_WORKER_ENV_KEYS = new Set([
341369
'PATH',

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

Lines changed: 7 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -1309,6 +1309,13 @@ function dispatchToWorker(
13091309

13101310
const timeout = setTimeout(() => {
13111311
workerInfo.pendingExecutions.delete(execId)
1312+
// A never-settling promise escapes the isolate's own script timeout, so the worker
1313+
// would otherwise keep the isolate and its timers alive until the worker retires.
1314+
try {
1315+
workerInfo.process.send({ type: 'cancel', executionId: execId })
1316+
} catch (err) {
1317+
logger.warn('Failed to send timeout cancel to worker', { err, workerId: workerInfo.id })
1318+
}
13121319
workerInfo.activeExecutions--
13131320
totalActiveExecutions--
13141321
ownerState.activeExecutions = Math.max(0, ownerState.activeExecutions - 1)

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

Lines changed: 37 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -499,6 +499,43 @@ async function run() {
499499
assert.equal(next.result, 'still alive', next.error?.message)
500500
})
501501

502+
const runTask = (code: string) =>
503+
executeInIsolatedVM({
504+
code,
505+
params: {},
506+
envVars: {},
507+
contextVariables: {},
508+
timeoutMs: 10000,
509+
requestId: `e2e-${generateRandomHex(4)}`,
510+
ownerKey: 'e2e-owner',
511+
task: {
512+
id: 'e2e-timers',
513+
bundles: [],
514+
bootstrap: '',
515+
brokers: [],
516+
finalize: 'return new Uint8Array([1])',
517+
},
518+
})
519+
520+
await check('caps the timers a task may hold on the shared worker', async () => {
521+
const timeouts = await runTask(
522+
'const f = () => {}; for (let i = 0; i < 200000; i++) setTimeout(f, 60000)'
523+
)
524+
assert.match(timeouts.error?.message ?? '', /Too many pending timers/, timeouts.error?.message)
525+
const intervals = await runTask(
526+
'const f = () => {}; for (let i = 0; i < 200; i++) setInterval(f, 1000)'
527+
)
528+
assert.match(
529+
intervals.error?.message ?? '',
530+
/Too many active intervals/,
531+
intervals.error?.message
532+
)
533+
const reused = await runTask(
534+
'for (let round = 0; round < 3; round++) { const ids = []; for (let i = 0; i < 1000; i++) ids.push(setTimeout(() => {}, 60000)); ids.forEach(clearTimeout) }'
535+
)
536+
assert.equal(reused.error, undefined, reused.error?.message)
537+
})
538+
502539
await check('rejects a connection with the wrong token', async () => {
503540
const ws = await openSocket(probeHost, authFrame(generateRandomHex(32)))
504541
assert.equal(await closeCode(ws), 4401)

0 commit comments

Comments
 (0)