Skip to content

Commit 6ab5280

Browse files
committed
Add signal handlers to release lock on SIGINT/SIGTERM/SIGHUP
This prevents stale lockfiles from being left behind when the build process is interrupted by a signal (Ctrl+C, kill, terminal close). The signal handlers: - Register after the lock is acquired - Release the lock before re-emitting the signal to allow normal termination - Are cleaned up after normal build completion to avoid memory leaks Also adds a test to verify signal handlers are properly registered and removed during the build lifecycle.
1 parent 2d6f655 commit 6ab5280

File tree

2 files changed

+74
-1
lines changed

2 files changed

+74
-1
lines changed

packages/app/src/cli/services/build/extension.test.ts

Lines changed: 32 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -388,4 +388,36 @@ describe('buildFunctionExtension', () => {
388388
expect(lockfile.lock).toHaveBeenCalled()
389389
expect(releaseLock).toHaveBeenCalled()
390390
})
391+
392+
test('registers signal handlers after acquiring lock and removes them after release', async () => {
393+
// Given
394+
const processOnSpy = vi.spyOn(process, 'on')
395+
const processRemoveListenerSpy = vi.spyOn(process, 'removeListener')
396+
397+
// When
398+
await expect(
399+
buildFunctionExtension(extension, {
400+
stdout,
401+
stderr,
402+
signal,
403+
app,
404+
environment: 'production',
405+
}),
406+
).resolves.toBeUndefined()
407+
408+
// Then
409+
// Verify signal handlers were registered for SIGINT, SIGTERM, SIGHUP
410+
expect(processOnSpy).toHaveBeenCalledWith('SIGINT', expect.any(Function))
411+
expect(processOnSpy).toHaveBeenCalledWith('SIGTERM', expect.any(Function))
412+
expect(processOnSpy).toHaveBeenCalledWith('SIGHUP', expect.any(Function))
413+
414+
// Verify signal handlers were removed after build completed
415+
expect(processRemoveListenerSpy).toHaveBeenCalledWith('SIGINT', expect.any(Function))
416+
expect(processRemoveListenerSpy).toHaveBeenCalledWith('SIGTERM', expect.any(Function))
417+
expect(processRemoveListenerSpy).toHaveBeenCalledWith('SIGHUP', expect.any(Function))
418+
419+
// Cleanup spies
420+
processOnSpy.mockRestore()
421+
processRemoveListenerSpy.mockRestore()
422+
})
391423
})

packages/app/src/cli/services/build/extension.ts

Lines changed: 42 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -130,6 +130,42 @@ type BuildFunctionExtensionOptions = ExtensionBuildOptions
130130
// This should be long enough to allow for slow builds but short enough to recover from crashes
131131
const LOCK_STALE_MS = 10000
132132

133+
// Signals that should trigger lock cleanup
134+
const CLEANUP_SIGNALS: NodeJS.Signals[] = ['SIGINT', 'SIGTERM', 'SIGHUP']
135+
136+
/**
137+
* Registers signal handlers to release the lock on abnormal termination.
138+
* Returns a cleanup function to remove the handlers when the lock is released normally.
139+
*/
140+
function registerLockCleanupHandlers(releaseLock: () => Promise<void>): () => void {
141+
const handlers: Map<NodeJS.Signals, NodeJS.SignalsListener> = new Map()
142+
143+
for (const signal of CLEANUP_SIGNALS) {
144+
const handler: NodeJS.SignalsListener = () => {
145+
outputDebug(`Received ${signal}, releasing build lock...`)
146+
// Use sync-style cleanup since we're in a signal handler
147+
// The lock release is async but we need to at least initiate it
148+
releaseLock()
149+
.catch((err) => outputDebug(`Failed to release lock on ${signal}: ${err}`))
150+
.finally(() => {
151+
// Re-emit the signal after cleanup so the process can terminate normally
152+
// Remove our handler first to avoid infinite loop
153+
process.removeListener(signal, handler)
154+
process.kill(process.pid, signal)
155+
})
156+
}
157+
handlers.set(signal, handler)
158+
process.on(signal, handler)
159+
}
160+
161+
// Return cleanup function to remove handlers
162+
return () => {
163+
for (const [signal, handler] of handlers) {
164+
process.removeListener(signal, handler)
165+
}
166+
}
167+
}
168+
133169
/**
134170
* Removes a stale lockfile if it exists and is not actively held.
135171
* This handles cases where a previous build process crashed without releasing the lock.
@@ -170,7 +206,8 @@ export async function buildFunctionExtension(
170206
// Clean up any stale locks from crashed builds before attempting to acquire
171207
await cleanupStaleLock(lockfilePath)
172208

173-
let releaseLock
209+
let releaseLock: () => Promise<void>
210+
let removeSignalHandlers: () => void
174211
try {
175212
releaseLock = await lockfile.lock(extension.directory, {
176213
retries: {
@@ -181,6 +218,8 @@ export async function buildFunctionExtension(
181218
stale: LOCK_STALE_MS,
182219
lockfilePath,
183220
})
221+
// Register signal handlers to release lock on SIGINT, SIGTERM, SIGHUP
222+
removeSignalHandlers = registerLockCleanupHandlers(releaseLock)
184223
// eslint-disable-next-line @typescript-eslint/no-explicit-any
185224
} catch (error: any) {
186225
outputDebug(`Failed to acquire function build lock: ${error.message}`)
@@ -231,6 +270,8 @@ export async function buildFunctionExtension(
231270
newError.errors = error.errors
232271
throw newError
233272
} finally {
273+
// Remove signal handlers before releasing lock (normal termination path)
274+
removeSignalHandlers()
234275
await releaseLock()
235276
}
236277
}

0 commit comments

Comments
 (0)