Skip to content

Commit d330a42

Browse files
Bill LeoutsakosBill Leoutsakos
authored andcommitted
Serialize E2E signal cleanup ownership
Prevent opposite signals and normal finalization from racing detached cleanup or releasing its run lock.
1 parent c096f4e commit d330a42

3 files changed

Lines changed: 20 additions & 9 deletions

File tree

apps/sim/e2e/foundation/build-manifest.spec.ts

Lines changed: 3 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -124,12 +124,13 @@ test('orchestrator lock rejects live ownership and recovers stale descriptors',
124124
try {
125125
const lock = acquireE2eRunLock(lockPath)
126126
expect(() => acquireE2eRunLock(lockPath)).toThrow(/Another E2E orchestrator/)
127-
lock.transfer(process.pid)
127+
expect(lock.transfer(process.pid)).toBe(true)
128128
expect(() => acquireE2eRunLock(lockPath)).toThrow(/Another E2E orchestrator/)
129129
lock.retain('manual cleanup required')
130-
lock.transfer(process.pid)
130+
expect(lock.transfer(process.pid)).toBe(true)
131131
expect(() => acquireE2eRunLock(lockPath)).toThrow(/manual cleanup required/)
132132
lock.release()
133+
expect(lock.transfer(process.pid)).toBe(false)
133134

134135
mkdirSync(lockPath)
135136
expect(() => acquireE2eRunLock(lockPath)).toThrow(/is acquiring/)

apps/sim/e2e/scripts/run.ts

Lines changed: 12 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -111,6 +111,7 @@ async function main(): Promise<void> {
111111
let canaryCoverageComplete = true
112112
let diagnosticsRetained = true
113113
let failed = false
114+
let signalCleanupStarted = false
114115

115116
const cleanup = (): Promise<void> => {
116117
if (cleanupPromise) return cleanupPromise
@@ -163,6 +164,8 @@ async function main(): Promise<void> {
163164
}
164165

165166
const handleSignal = async (signal: NodeJS.Signals): Promise<void> => {
167+
if (signalCleanupStarted) return
168+
signalCleanupStarted = true
166169
failed = true
167170
const exitCode = signal === 'SIGINT' ? 130 : 143
168171
process.exitCode = exitCode
@@ -211,7 +214,9 @@ async function main(): Promise<void> {
211214
if (!cleanupProcess.pid) {
212215
throw new Error('Detached E2E cleanup supervisor started without a process ID')
213216
}
214-
runLock.transfer(cleanupProcess.pid)
217+
if (!runLock.transfer(cleanupProcess.pid)) {
218+
throw new Error('Detached E2E cleanup supervisor could not acquire run-lock ownership')
219+
}
215220
lockTransferred = true
216221
cleanupProcess.unref()
217222
console.error(`Detached E2E cleanup supervisor started as PID ${cleanupProcess.pid}`)
@@ -227,7 +232,8 @@ async function main(): Promise<void> {
227232
}
228233
process.exit(exitCode)
229234
}
230-
// `once` is intentional: a second signal uses the OS default force termination.
235+
// Both signal types share a synchronous single-flight guard so opposite signals cannot launch
236+
// competing supervisors while the first handler awaits child-process startup.
231237
const handleSigint = (): void => void handleSignal('SIGINT')
232238
const handleSigterm = (): void => void handleSignal('SIGTERM')
233239
process.once('SIGINT', handleSigint)
@@ -412,8 +418,10 @@ async function main(): Promise<void> {
412418
process.off('SIGINT', handleSigint)
413419
process.off('SIGTERM', handleSigterm)
414420
setManagedProcessGroupObserver(null)
415-
if (cleanupSucceeded) runLock.release()
416-
else runLock.retain('normal cleanup failed; inspect diagnostics and clean resources manually')
421+
if (!signalCleanupStarted) {
422+
if (cleanupSucceeded) runLock.release()
423+
else runLock.retain('normal cleanup failed; inspect diagnostics and clean resources manually')
424+
}
417425
console.info(
418426
diagnosticsRetained
419427
? `E2E ${failed ? 'failed' : 'completed'}; diagnostics: ${runDirectory}`

apps/sim/e2e/support/run-lock.ts

Lines changed: 5 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -32,7 +32,7 @@ export interface E2eRunLock {
3232
path: string
3333
token: string
3434
setProcessGroupIds(processGroupIds: number[]): void
35-
transfer(pid: number): void
35+
transfer(pid: number): boolean
3636
retain(reason: string): void
3737
release(): void
3838
}
@@ -74,14 +74,16 @@ export function acquireE2eRunLock(
7474
),
7575
})
7676
},
77-
transfer(pid: number): void {
77+
transfer(pid: number): boolean {
7878
const current = readDescriptor(lockPath)
79-
if (current?.token !== descriptor.token) return
79+
if (current?.token !== descriptor.token) return false
8080
writeDescriptor(lockPath, {
8181
...current,
8282
pid,
8383
processStartIdentity: readProcessStartIdentity(pid),
8484
})
85+
const transferred = readDescriptor(lockPath)
86+
return transferred?.token === descriptor.token && transferred.pid === pid
8587
},
8688
retain(reason: string): void {
8789
retainE2eRunLock(lockPath, descriptor.token, reason)

0 commit comments

Comments
 (0)