fix(auto): register SIGHUP/SIGINT handlers to clean lock files on crash (#1821)
When VSCode crashes or the parent process dies, only SIGTERM was handled, leaving stranded .gsd.lock/ and auto.lock files. Now registers handlers on SIGTERM, SIGHUP, and SIGINT, and calls both clearLock() and releaseSessionLock() for complete cleanup. Fixes #1797 Co-authored-by: Claude Opus 4.6 (1M context) <noreply@anthropic.com>
This commit is contained in:
parent
e94bda817f
commit
d2be9f34ce
2 changed files with 121 additions and 7 deletions
|
|
@ -1,16 +1,24 @@
|
|||
/**
|
||||
* Auto-mode Supervisor — SIGTERM handling and working-tree activity detection.
|
||||
* Auto-mode Supervisor — signal handling and working-tree activity detection.
|
||||
*
|
||||
* Pure functions — no module-level globals or AutoContext dependency.
|
||||
*/
|
||||
|
||||
import { clearLock } from "./crash-recovery.js";
|
||||
import { releaseSessionLock } from "./session-lock.js";
|
||||
import { nativeHasChanges } from "./native-git-bridge.js";
|
||||
|
||||
// ─── SIGTERM Handling ─────────────────────────────────────────────────────────
|
||||
// ─── Signal Handling ─────────────────────────────────────────────────────────
|
||||
|
||||
/** Signals that should trigger lock cleanup on process termination. */
|
||||
const CLEANUP_SIGNALS: NodeJS.Signals[] = ["SIGTERM", "SIGHUP", "SIGINT"];
|
||||
|
||||
/**
|
||||
* Register a SIGTERM handler that clears the lock file and exits cleanly.
|
||||
* Register signal handlers that clear lock files and exit cleanly.
|
||||
* Installs handlers on SIGTERM, SIGHUP, and SIGINT so that lock files
|
||||
* are cleaned up regardless of how the process is terminated (normal kill,
|
||||
* parent process death, or Ctrl+C).
|
||||
*
|
||||
* Captures the active base path at registration time so the handler
|
||||
* always references the correct path even if the module variable changes.
|
||||
* Removes any previously registered handler before installing the new one.
|
||||
|
|
@ -21,19 +29,22 @@ export function registerSigtermHandler(
|
|||
currentBasePath: string,
|
||||
previousHandler: (() => void) | null,
|
||||
): () => void {
|
||||
if (previousHandler) process.off("SIGTERM", previousHandler);
|
||||
if (previousHandler) {
|
||||
for (const sig of CLEANUP_SIGNALS) process.off(sig, previousHandler);
|
||||
}
|
||||
const handler = () => {
|
||||
clearLock(currentBasePath);
|
||||
releaseSessionLock(currentBasePath);
|
||||
process.exit(0);
|
||||
};
|
||||
process.on("SIGTERM", handler);
|
||||
for (const sig of CLEANUP_SIGNALS) process.on(sig, handler);
|
||||
return handler;
|
||||
}
|
||||
|
||||
/** Deregister the SIGTERM handler (called on stop/pause). */
|
||||
/** Deregister signal handlers from all cleanup signals (called on stop/pause). */
|
||||
export function deregisterSigtermHandler(handler: (() => void) | null): void {
|
||||
if (handler) {
|
||||
process.off("SIGTERM", handler);
|
||||
for (const sig of CLEANUP_SIGNALS) process.off(sig, handler);
|
||||
}
|
||||
}
|
||||
|
||||
|
|
|
|||
103
src/resources/extensions/gsd/tests/signal-handlers.test.ts
Normal file
103
src/resources/extensions/gsd/tests/signal-handlers.test.ts
Normal file
|
|
@ -0,0 +1,103 @@
|
|||
import test from "node:test";
|
||||
import assert from "node:assert/strict";
|
||||
import {
|
||||
registerSigtermHandler,
|
||||
deregisterSigtermHandler,
|
||||
} from "../auto-supervisor.ts";
|
||||
|
||||
/**
|
||||
* Tests for signal handler registration (SIGTERM, SIGHUP, SIGINT).
|
||||
*
|
||||
* Validates that registerSigtermHandler installs handlers on all three
|
||||
* signals and deregisterSigtermHandler removes them from all three.
|
||||
* Fixes #1797 — stranded lock files on VSCode crash due to missing
|
||||
* SIGHUP and SIGINT handlers.
|
||||
*/
|
||||
|
||||
test("registerSigtermHandler installs handlers on SIGTERM, SIGHUP, and SIGINT", () => {
|
||||
const before = {
|
||||
SIGTERM: process.listenerCount("SIGTERM"),
|
||||
SIGHUP: process.listenerCount("SIGHUP"),
|
||||
SIGINT: process.listenerCount("SIGINT"),
|
||||
};
|
||||
|
||||
const handler = registerSigtermHandler("/tmp/test-signal-handlers", null);
|
||||
|
||||
assert.equal(
|
||||
process.listenerCount("SIGTERM"),
|
||||
before.SIGTERM + 1,
|
||||
"SIGTERM listener should be added",
|
||||
);
|
||||
assert.equal(
|
||||
process.listenerCount("SIGHUP"),
|
||||
before.SIGHUP + 1,
|
||||
"SIGHUP listener should be added",
|
||||
);
|
||||
assert.equal(
|
||||
process.listenerCount("SIGINT"),
|
||||
before.SIGINT + 1,
|
||||
"SIGINT listener should be added",
|
||||
);
|
||||
|
||||
// Clean up
|
||||
deregisterSigtermHandler(handler);
|
||||
});
|
||||
|
||||
test("deregisterSigtermHandler removes handlers from all three signals", () => {
|
||||
const handler = registerSigtermHandler("/tmp/test-signal-handlers", null);
|
||||
|
||||
const during = {
|
||||
SIGTERM: process.listenerCount("SIGTERM"),
|
||||
SIGHUP: process.listenerCount("SIGHUP"),
|
||||
SIGINT: process.listenerCount("SIGINT"),
|
||||
};
|
||||
|
||||
deregisterSigtermHandler(handler);
|
||||
|
||||
assert.equal(
|
||||
process.listenerCount("SIGTERM"),
|
||||
during.SIGTERM - 1,
|
||||
"SIGTERM listener should be removed",
|
||||
);
|
||||
assert.equal(
|
||||
process.listenerCount("SIGHUP"),
|
||||
during.SIGHUP - 1,
|
||||
"SIGHUP listener should be removed",
|
||||
);
|
||||
assert.equal(
|
||||
process.listenerCount("SIGINT"),
|
||||
during.SIGINT - 1,
|
||||
"SIGINT listener should be removed",
|
||||
);
|
||||
});
|
||||
|
||||
test("registerSigtermHandler deregisters previous handler from all signals", () => {
|
||||
const before = {
|
||||
SIGTERM: process.listenerCount("SIGTERM"),
|
||||
SIGHUP: process.listenerCount("SIGHUP"),
|
||||
SIGINT: process.listenerCount("SIGINT"),
|
||||
};
|
||||
|
||||
const handler1 = registerSigtermHandler("/tmp/test-signal-handlers", null);
|
||||
const handler2 = registerSigtermHandler("/tmp/test-signal-handlers-2", handler1);
|
||||
|
||||
// Should still only have one extra listener per signal (old one removed, new one added)
|
||||
assert.equal(
|
||||
process.listenerCount("SIGTERM"),
|
||||
before.SIGTERM + 1,
|
||||
"SIGTERM should have exactly one handler after re-registration",
|
||||
);
|
||||
assert.equal(
|
||||
process.listenerCount("SIGHUP"),
|
||||
before.SIGHUP + 1,
|
||||
"SIGHUP should have exactly one handler after re-registration",
|
||||
);
|
||||
assert.equal(
|
||||
process.listenerCount("SIGINT"),
|
||||
before.SIGINT + 1,
|
||||
"SIGINT should have exactly one handler after re-registration",
|
||||
);
|
||||
|
||||
// Clean up
|
||||
deregisterSigtermHandler(handler2);
|
||||
});
|
||||
Loading…
Add table
Reference in a new issue