Skip to content

Commit 5a2f0e3

Browse files
committed
fix(execution): keep the over-limit abort path release-free
The abort guard fires for any non-acquired result, so an over-limit acquisition that was already aborted also released — contradicting the rule stated two lines below, where the same result returns without releasing because the script answers before its ZADD. Restrict the release to undetermined results and assert the no-release behavior in the existing over-limit abort test.
1 parent 2422e53 commit 5a2f0e3

2 files changed

Lines changed: 6 additions & 1 deletion

File tree

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

Lines changed: 4 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -704,6 +704,7 @@ describe('isolated-vm scheduler', () => {
704704
})
705705

706706
it('reports cancellation when abort races a rejected distributed lease', async () => {
707+
const scripts: string[] = []
707708
let resolveLease!: (value: number) => void
708709
let markLeaseRequested!: () => void
709710
const leaseResult = new Promise<number>((resolve) => {
@@ -719,6 +720,7 @@ describe('isolated-vm scheduler', () => {
719720
spawns: [() => createReadyProc('unused')],
720721
redisEvalImpl: (...args: unknown[]) => {
721722
const script = String(args[0] ?? '')
723+
scripts.push(script)
722724
if (script.includes('ZREMRANGEBYSCORE')) {
723725
markLeaseRequested()
724726
return leaseResult
@@ -748,6 +750,8 @@ describe('isolated-vm scheduler', () => {
748750
termination: 'cancelled',
749751
error: { name: 'AbortError' },
750752
})
753+
// Redis answered before its ZADD, so there is nothing to remove.
754+
expect(scripts.some((script) => script.includes("'ZREM'"))).toBe(false)
751755
expect(spawnMock).not.toHaveBeenCalled()
752756
})
753757

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

Lines changed: 2 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -1436,7 +1436,8 @@ export async function executeInIsolatedVM(
14361436
}
14371437

14381438
if (leaseAcquireResult !== 'acquired' && signal?.aborted) {
1439-
releaseLease()
1439+
// Only an undetermined result can have registered; see the over-limit branch below.
1440+
if (leaseAcquireResult === 'undetermined') releaseLease()
14401441
maybeCleanupOwner(ownerKey)
14411442
return {
14421443
result: null,

0 commit comments

Comments
 (0)