Skip to content

Commit 9fda6e6

Browse files
authored
double SIGINT prevention (#125)
Includes a fix in lib/start.js that adds an inner SIGINT handler to prevent race conditions during graceful shutdown. Add test case to verify that multiple SIGINT signals don't cause double shutdown. The test confirms that: - Only one shutdown occurs despite multiple signals - SIGINT handlers ignore subsequent signals during shutdown - All SIGINT listeners are properly cleaned up after shutdown
1 parent 730dad2 commit 9fda6e6

File tree

2 files changed

+63
-0
lines changed

2 files changed

+63
-0
lines changed

lib/start.js

Lines changed: 4 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -67,9 +67,13 @@ export async function start (client, selectedRuntime) {
6767
}
6868

6969
process.once('SIGINT', async () => {
70+
process.on('SIGINT', () => {
71+
// we ignore future signals to avoid double shutdown
72+
})
7073
clearTimeout(recordTimeout)
7174
await recordMetrics(entrypointUrl)
7275
await shutdown()
76+
process.removeAllListeners('SIGINT')
7377
})
7478

7579
const { statusCode, body } = await requestRecord('start')

test/start.test.ts

Lines changed: 59 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -241,4 +241,63 @@ describe('start', () => {
241241
globalThis.setTimeout = originalSetTimeout
242242
}
243243
})
244+
245+
it('should prevent double shutdown when multiple SIGINT signals are received', async () => {
246+
// Set parseArgs result to enable recording
247+
parseArgsResult = { values: { record: true } }
248+
249+
const pid = 'test-runtime-multi-sigint'
250+
251+
// Track shutdown calls
252+
let shutdownCount = 0
253+
const originalClose = mockServer.close
254+
mockServer.close = mock.fn(async () => {
255+
shutdownCount++
256+
await originalClose.call(mockServer)
257+
})
258+
259+
// Mock setTimeout to prevent actual 10-minute timeout
260+
const originalSetTimeout = globalThis.setTimeout
261+
globalThis.setTimeout = mock.fn((callback: () => void, delay: number) => {
262+
if (delay === 600000) {
263+
return {} as NodeJS.Timeout
264+
}
265+
return originalSetTimeout(callback, delay)
266+
}) as typeof setTimeout
267+
268+
try {
269+
const { start } = await import('../lib/start.js')
270+
await start(mockClient, pid)
271+
272+
// Verify SIGINT listener is registered
273+
assert.strictEqual(process.listenerCount('SIGINT'), 1, 'Should have one SIGINT listener')
274+
275+
// First SIGINT - should trigger shutdown
276+
process.emit('SIGINT')
277+
await new Promise(resolve => setImmediate(resolve))
278+
279+
// During shutdown, there should be an ignoring handler
280+
const listenerCountDuringShutdown = process.listenerCount('SIGINT')
281+
282+
// Send second SIGINT during shutdown
283+
process.emit('SIGINT')
284+
await new Promise(resolve => setImmediate(resolve))
285+
286+
// Send third SIGINT to be sure
287+
process.emit('SIGINT')
288+
await new Promise(resolve => setImmediate(resolve))
289+
await new Promise(resolve => setImmediate(resolve))
290+
291+
// Verify shutdown was only called once
292+
const closeMock = mockServer.close as MockFunction<() => Promise<void>>
293+
assert.strictEqual(closeMock.mock.calls.length, 1, 'Server close should only be called once despite multiple SIGINT signals')
294+
assert.strictEqual(shutdownCount, 1, 'Shutdown should only happen once')
295+
296+
// Verify all SIGINT listeners are cleaned up
297+
assert.strictEqual(process.listenerCount('SIGINT'), 0, 'All SIGINT listeners should be removed after shutdown')
298+
} finally {
299+
globalThis.setTimeout = originalSetTimeout
300+
mockServer.close = originalClose
301+
}
302+
})
244303
})

0 commit comments

Comments
 (0)