Skip to content

Commit 5927f1c

Browse files
claudeclaude
committed
Address PR #24 review: lock release on early failure, hint, stale window
- Wrap post-preflight body of main() in try/catch so the lock is released if any later setup step (SessionStore.load, getPort, runManager.init, router setup) throws before the SIGINT/SIGTERM handlers are wired up. - Update the `instance-running` hint to reflect that the lock is a directory now, and to also mention the `.lock.meta.json` sidecar so manual cleanup actually clears all state. - Bump LOCK_STALE_MS from 30s to 5min: a debugger pause, long event-loop stall, or short system sleep should not let a second invocation consider the lock stale and race the original. Manual reclaim is still documented in the error hint.
1 parent 5458899 commit 5927f1c

2 files changed

Lines changed: 85 additions & 73 deletions

File tree

‎src/server/index.ts‎

Lines changed: 79 additions & 71 deletions
Original file line numberDiff line numberDiff line change
@@ -28,85 +28,93 @@ async function main(): Promise<void> {
2828
process.exit(1);
2929
}
3030

31-
const sessionStore = await SessionStore.load(preflight.storageRoot, cwd);
32-
const port = await getPort({ port: [DEFAULT_PREF_PORT, 6801, 6802, 6803, 6804, 0] });
33-
const auth = makeAuthContext(port);
31+
// The lock is owned from this point forward; release it on any failure so
32+
// a botched startup doesn't leave a stale lock blocking subsequent runs
33+
// until the staleness window expires.
34+
try {
35+
const sessionStore = await SessionStore.load(preflight.storageRoot, cwd);
36+
const port = await getPort({ port: [DEFAULT_PREF_PORT, 6801, 6802, 6803, 6804, 0] });
37+
const auth = makeAuthContext(port);
3438

35-
const runManager = new RunManager({
36-
claudeBin,
37-
cwd,
38-
storageRoot: preflight.storageRoot,
39-
session: sessionStore,
40-
});
41-
await runManager.init();
39+
const runManager = new RunManager({
40+
claudeBin,
41+
cwd,
42+
storageRoot: preflight.storageRoot,
43+
session: sessionStore,
44+
});
45+
await runManager.init();
4246

43-
const webRoot = resolveWebRoot();
44-
const handler = createRouter({ auth, session: sessionStore, runManager, webRoot, cwd });
47+
const webRoot = resolveWebRoot();
48+
const handler = createRouter({ auth, session: sessionStore, runManager, webRoot, cwd });
4549

46-
const server = createServer((req, res) => {
47-
Promise.resolve(handler(req, res)).catch((err) => {
48-
log.error('http.handler-rejected', { error: (err as Error).message });
49-
try {
50-
res.statusCode = 500;
51-
res.end();
52-
} catch {
53-
/* */
54-
}
50+
const server = createServer((req, res) => {
51+
Promise.resolve(handler(req, res)).catch((err) => {
52+
log.error('http.handler-rejected', { error: (err as Error).message });
53+
try {
54+
res.statusCode = 500;
55+
res.end();
56+
} catch {
57+
/* */
58+
}
59+
});
5560
});
56-
});
5761

58-
server.listen(port, '127.0.0.1', async () => {
59-
await writeLockMeta(preflight.lockFilePath, process.pid, port);
60-
const url = `${auth.origin}/?t=${auth.token}`;
61-
log.info('server.listening', { url, pid: process.pid });
62-
console.log(`mdredd listening at ${url}`);
63-
if (shouldOpen) {
64-
open(url).catch((err) => {
65-
console.log(`(could not open browser automatically: ${err.message})`);
66-
});
67-
}
68-
});
62+
server.listen(port, '127.0.0.1', async () => {
63+
await writeLockMeta(preflight.lockFilePath, process.pid, port);
64+
const url = `${auth.origin}/?t=${auth.token}`;
65+
log.info('server.listening', { url, pid: process.pid });
66+
console.log(`mdredd listening at ${url}`);
67+
if (shouldOpen) {
68+
open(url).catch((err) => {
69+
console.log(`(could not open browser automatically: ${err.message})`);
70+
});
71+
}
72+
});
6973

70-
// 5s for runners to drain (each runner self-bounds at SIGTERM+2s SIGKILL),
71-
// plus 3s slack for lockfile / FS work. Anything still hanging past the hard
72-
// timer is force-exited so a stuck child can never wedge the server.
73-
const STOP_RUNNERS_TIMEOUT_MS = 5_000;
74-
const HARD_SHUTDOWN_TIMEOUT_MS = 8_000;
74+
// 5s for runners to drain (each runner self-bounds at SIGTERM+2s SIGKILL),
75+
// plus 3s slack for lockfile / FS work. Anything still hanging past the hard
76+
// timer is force-exited so a stuck child can never wedge the server.
77+
const STOP_RUNNERS_TIMEOUT_MS = 5_000;
78+
const HARD_SHUTDOWN_TIMEOUT_MS = 8_000;
7579

76-
let shuttingDown = false;
77-
const shutdown = async (sig: string): Promise<void> => {
78-
if (shuttingDown) return;
79-
shuttingDown = true;
80-
log.info('server.shutdown', { signal: sig, activeRuns: runManager.activeCount() });
81-
const hardTimer = setTimeout(() => {
82-
log.error('server.shutdown-forced-exit', { reason: 'hard timeout exceeded' });
83-
process.exit(1);
84-
}, HARD_SHUTDOWN_TIMEOUT_MS);
85-
hardTimer.unref();
86-
try {
87-
// Stop accepting new HTTP traffic and drop existing keep-alive/SSE
88-
// connections so the server's `listening` socket and the SSE keepers
89-
// don't hold the event loop open.
90-
server.close();
91-
server.closeAllConnections?.();
92-
const result = await runManager.stopAll(STOP_RUNNERS_TIMEOUT_MS);
93-
if (result.timedOut) {
94-
log.warn('server.shutdown.stopAll-timeout', { stopped: result.stopped });
80+
let shuttingDown = false;
81+
const shutdown = async (sig: string): Promise<void> => {
82+
if (shuttingDown) return;
83+
shuttingDown = true;
84+
log.info('server.shutdown', { signal: sig, activeRuns: runManager.activeCount() });
85+
const hardTimer = setTimeout(() => {
86+
log.error('server.shutdown-forced-exit', { reason: 'hard timeout exceeded' });
87+
process.exit(1);
88+
}, HARD_SHUTDOWN_TIMEOUT_MS);
89+
hardTimer.unref();
90+
try {
91+
// Stop accepting new HTTP traffic and drop existing keep-alive/SSE
92+
// connections so the server's `listening` socket and the SSE keepers
93+
// don't hold the event loop open.
94+
server.close();
95+
server.closeAllConnections?.();
96+
const result = await runManager.stopAll(STOP_RUNNERS_TIMEOUT_MS);
97+
if (result.timedOut) {
98+
log.warn('server.shutdown.stopAll-timeout', { stopped: result.stopped });
99+
}
100+
await preflight.releaseLock();
101+
} catch (err) {
102+
log.error('server.shutdown-error', { error: (err as Error).message });
103+
} finally {
104+
clearTimeout(hardTimer);
95105
}
96-
await preflight.releaseLock();
97-
} catch (err) {
98-
log.error('server.shutdown-error', { error: (err as Error).message });
99-
} finally {
100-
clearTimeout(hardTimer);
101-
}
102-
process.exit(0);
103-
};
104-
process.on('SIGINT', () => {
105-
void shutdown('SIGINT');
106-
});
107-
process.on('SIGTERM', () => {
108-
void shutdown('SIGTERM');
109-
});
106+
process.exit(0);
107+
};
108+
process.on('SIGINT', () => {
109+
void shutdown('SIGINT');
110+
});
111+
process.on('SIGTERM', () => {
112+
void shutdown('SIGTERM');
113+
});
114+
} catch (err) {
115+
await preflight.releaseLock().catch(() => undefined);
116+
throw err;
117+
}
110118
}
111119

112120
function resolveWebRoot(): string {

‎src/server/preflight.ts‎

Lines changed: 6 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -25,7 +25,11 @@ export interface PreflightResult {
2525
// Threshold (ms) past which a lockfile is considered stale and may be
2626
// reclaimed. proper-lockfile refreshes the lockfile's mtime every stale/2
2727
// while the holder is alive; after a crash it is reclaimable after `stale` ms.
28-
const LOCK_STALE_MS = 30_000;
28+
// Set generously: a long event-loop stall, debugger pause, or brief system
29+
// sleep should not cause a second invocation to consider the lock stale and
30+
// race with the original — that would defeat the single-instance guarantee.
31+
// Users can always reclaim manually via the hint in `instance-running`.
32+
const LOCK_STALE_MS = 5 * 60_000;
2933

3034
export class PreflightError extends Error {
3135
code: string;
@@ -287,7 +291,7 @@ async function acquireLock(
287291
throw new PreflightError(
288292
'instance-running',
289293
`Another mdredd instance appears to be running${info}.`,
290-
`Close it first, or remove ${lockFilePath} if you are sure it is stale.`,
294+
`Close it first, or, if you are sure it is stale, remove the lock directory and its sidecar: rm -rf ${lockFilePath} ${metaPath}`,
291295
);
292296
}
293297
throw err;

0 commit comments

Comments
 (0)