GithubStar/codegraph
1
0
Fork 0
mirror of https://github.com/colbymchenry/codegraph.git synced 2026-10-22 01:07:18 +08:00
Code Issues Packages Projects Releases Wiki Activity

fix(mcp): wait for a stopping daemon, and for the proxy's fallback engine (#2311) (#2316)

Browse Source 
* fix(mcp): wait for a stopping daemon, and for the proxy's fallback engine (#2311)

Now that QueryPool.destroy() waits up to 15 s for a query worker still
starting up (#2307), two callers gave up too early:

- stopDaemonAt() gave a daemon 3 s after SIGTERM, then re-probed its
  identity and reported it still running once the daemon had closed its
  socket, so a daemon waiting on a starting worker made `codegraph index`
  abort. A daemon that has closed its socket but still holds its lock is
  now given 17 s more to exit; nothing is signalled during that wait.
- The proxy's in-process fallback called engine.stop() and exited on the
  next line. It now exits once its engine, and any engine still being
  retired after a handback to the daemon, has stopped, with a 20 s
  backstop. Once shutdown starts it ignores client input and starts no
  new engine.

Extracted from bompus/codegraph@cc87ec34da,
a fork merge commit that cannot be cherry-picked whole; only these four
files are taken, unchanged.

Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>

* fix(mcp): a stopping daemon is waited for whether it closed its socket or released its lock (#2311)

Builds on the previous commit:

- stopDaemonAt: one rule after the first 3 s. If the daemon's identity can
  be re-proven (same lock, socket answers), escalate to SIGKILL as before.
  Otherwise, whether it has stopped answering its socket or already let go
  of its lock, it is partway through its own shutdown: signal nothing more
  and wait, bounded, for the PID to exit. The previous commit reported a
  daemon that had released its lock but not yet exited as still running,
  a race at the end of every slow shutdown. The cleanup that follows
  compares the lock with the one signalled, so a successor's is kept. On
  Windows, where SIGTERM is TerminateProcess, the same wait covers a
  termination still settling.
- The grace is WORKER_START_SETTLE_MS + 2 s and the proxy's backstop
  WORKER_START_SETTLE_MS + 5 s, so they move with the settle wait they
  cover. stopDaemonAt takes `shutdownGraceMs` so a test can bound it.
- The proxy says so on stderr when its backstop fires.

Tests: the daemon fixture's setup and cleanup are shared, and the fixture
process is always killed on cleanup (a failing run used to leak it). New:
a daemon that released its lock mid-shutdown is waited for; a daemon that
never finishes stopping is still reported after the grace and never
SIGKILLed; the proxy waits for an engine being retired at shutdown; and
client input arriving once shutdown has begun starts no engine.

Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>

---------

Co-authored-by: Aaron Queen <bompus@users.noreply.github.com>
Co-authored-by: Claude Opus 5.5 <noreply@anthropic.com>
This commit is contained in:
Colby Mchenry
2026-10-03 05:05:29 +00:00
committed by GitHub
parent ad60dccd04
commit dbb3d63cb1
6 changed files with 357 additions and 14 deletions
Download Patch File Download Diff File Expand all files Collapse all files
src/mcp/daemon-registry.ts
+40 -9
View File
@@ -32,6 +32,7 @@ import {
type DaemonLockInfo,
} from './daemon-paths';
import { readWriterLock, releaseWriterLock, tryAcquireWriterLock } from './writer-lock';
import { WORKER_START_SETTLE_MS } from '../worker-teardown';
export interface DaemonRecord {
/** Realpath'd project root the daemon serves. */
@@ -213,6 +214,18 @@ export async function clearStaleDaemonArtifacts(root: string): Promise<boolean>
const sleep = (ms: number): Promise<void> => new Promise((r) => setTimeout(r, ms));
/** How long `stopDaemonAt` gives a daemon to exit on SIGTERM before looking closer. */
const DAEMON_TERM_WAIT_MS = 3_000;
/**
* How much longer `stopDaemonAt` waits for a daemon that is partway through its
* own shutdown: it has stopped answering its socket (or let go of its lock) but
* not yet exited. That shutdown waits up to {@link WORKER_START_SETTLE_MS} for a
* query worker still starting up before it exits, so this covers that plus the
* rest of the shutdown (#2311).
*/
const DAEMON_SHUTDOWN_GRACE_MS = WORKER_START_SETTLE_MS + 2_000;
async function waitForDeath(pid: number, timeoutMs: number): Promise<boolean> {
const deadline = Date.now() + timeoutMs;
while (Date.now() < deadline) {
@@ -235,8 +248,14 @@ export interface StopResult {
* keys its socket/lockfile). Resolves the pid from the authoritative lockfile,
* falling back to the registry. Rebuild callers preserve unverified live locks
* instead of interpreting a failed probe as permission to discard the database.
* A daemon still finishing its own shutdown is waited for, up to
* `shutdownGraceMs` more (default {@link DAEMON_SHUTDOWN_GRACE_MS}; tests
* shorten it), before it is reported `still-running`.
*/
export async function stopDaemonAt(root: string, options: { preserveUnverified?: boolean } = {}): Promise<StopResult> {
export async function stopDaemonAt(
root: string,
options: { preserveUnverified?: boolean; shutdownGraceMs?: number } = {},
): Promise<StopResult> {
let pid: number | null = null;
let identity: DaemonLockInfo | null = null;
let lockContents: string | null = null;
@@ -285,17 +304,29 @@ export async function stopDaemonAt(root: string, options: { preserveUnverified?:
// (no graceful path), so we always sweep artifacts ourselves below.
try { process.kill(pid, 'SIGTERM'); } catch { /* raced to exit */ }
let outcome: StopResult['outcome'] = 'term';
if (!(await waitForDeath(pid, 3000))) {
if (!(await waitForDeath(pid, DAEMON_TERM_WAIT_MS))) {
// Re-prove identity before escalating; the old PID may have been reused.
if (!sameLock() || !await probeDaemonIdentity(identity) || !sameLock()) {
return { root, pid, outcome: 'still-running' };
if (sameLock() && await probeDaemonIdentity(identity) && sameLock()) {
try { process.kill(pid, 'SIGKILL'); } catch { /* raced to exit */ }
if (!(await waitForDeath(pid, 2000))) {
return { root, pid, outcome: 'still-running' };
}
outcome = 'kill';
} else {
// No longer answering its socket, or no longer holding the lock it held
// when it was signalled: a daemon partway through its own shutdown, which
// closes the socket first and can then wait on a query worker still
// starting up before it releases the lock and exits (#2311). On Windows,
// where SIGTERM is TerminateProcess, this is a termination still settling.
// Its identity can't be re-proven without the socket, so signal nothing
// more; just wait, bounded, for the PID to go. One that outlives the
// wait is reported, as before.
if (!(await waitForDeath(pid, options.shutdownGraceMs ?? DAEMON_SHUTDOWN_GRACE_MS))) {
return { root, pid, outcome: 'still-running' };
}
}
try { process.kill(pid, 'SIGKILL'); } catch { /* raced to exit */ }
if (!(await waitForDeath(pid, 2000))) {
return { root, pid, outcome: 'still-running' };
}
outcome = 'kill';
}
// Compares the lock with the one we signalled, so a successor's is kept.
cleanupDaemonArtifacts(root, lockContents);
return { root, pid, outcome };
}
src/mcp/proxy.ts
+35 -4
View File
@@ -35,6 +35,7 @@ import { ExploreSessionState } from './explore-session-state';
import { getTelemetry, ClientInfo } from '../telemetry';
import { installMainThreadWatchdog, WatchdogHandle } from './liveness-watchdog';
import type { MCPEngine } from './engine';
import { WORKER_START_SETTLE_MS } from '../worker-teardown';
/** Default poll cadence for the PPID watchdog (same as the direct server). */
const DEFAULT_PPID_POLL_MS = 5000;
@@ -48,6 +49,15 @@ const DEFAULT_PPID_POLL_MS = 5000;
const DEFAULT_DAEMON_RETRY_MS = 5_000;
const DEFAULT_DAEMON_RETRY_MAX_MS = 300_000;
/**
* Longest the proxy waits, once it is shutting down, for its in-process engines
* to stop before exiting anyway. An engine's stop waits up to
* {@link WORKER_START_SETTLE_MS} for a query worker still starting up (exiting
* mid-start can crash the process on Windows); this bounds a stop that never
* settles (#2311).
*/
const SHUTDOWN_BACKSTOP_MS = WORKER_START_SETTLE_MS + 5_000;
/**
* Env var that opts INTO the "attached to shared daemon" log line. Off by
* default: the line is benign INFO, but MCP hosts render any server stderr at
@@ -250,6 +260,8 @@ export async function runLocalHandshakeProxy(deps: LocalHandshakeDeps): Promise<
let engineReady: Promise<void> | null = null;
// Calls being served in-process; the engine is only ever stopped after they finish.
const localCalls = new Set<Promise<void>>();
// Engines retired but not yet stopped; shutdown() waits for them too.
const retiring = new Set<MCPEngine>();
let shuttingDown = false;
const retryBaseMs = parseDelayMs(process.env.CODEGRAPH_DAEMON_RETRY_MS, DEFAULT_DAEMON_RETRY_MS);
const retryMaxMs = Math.max(retryBaseMs, parseDelayMs(process.env.CODEGRAPH_DAEMON_RETRY_MAX_MS, DEFAULT_DAEMON_RETRY_MAX_MS));
@@ -282,12 +294,23 @@ export async function runLocalHandshakeProxy(deps: LocalHandshakeDeps): Promise<
try { livenessWatchdog?.stop(); } catch { /* ignore */ }
if (retryTimer) clearTimeout(retryTimer);
try { daemonSocket?.destroy(); } catch { /* ignore */ }
try { engine?.stop(); } catch { /* ignore */ }
process.exit(0);
// Exit once every engine has stopped, a retiring one included: exiting while
// a query worker is still starting up can crash the process on Windows (see
// QueryPool.destroy), and stopping releases the writer lock. The backstop
// bounds a stop that never settles; it stays ref'd so the process lives
// until one of the two exits it.
setTimeout(() => {
process.stderr.write(`[CodeGraph MCP] In-process engine did not stop within ${SHUTDOWN_BACKSTOP_MS}ms; exiting anyway.\n`);
process.exit(0);
}, SHUTDOWN_BACKSTOP_MS);
const stopping = [engine, ...retiring].map((e) => Promise.resolve().then(() => e?.stop()));
void Promise.allSettled(stopping).then(() => process.exit(0));
};
// Resolves the engine a call was started on, so a call in flight while the
// engine is being retired still finishes on it.
const ensureEngine = async (): Promise<MCPEngine> => {
// shutdown() stops only the engines it can see; never start one after it.
if (!engine && shuttingDown) throw new Error('codegraph is shutting down');
if (!engine) {
engine = deps.makeEngine();
engineReady = engine.ensureInitialized(deps.root).catch(() => { /* degraded */ });
@@ -302,8 +325,13 @@ export async function runLocalHandshakeProxy(deps: LocalHandshakeDeps): Promise<
const retired = engine;
engine = null;
engineReady = null;
await Promise.allSettled([...localCalls]);
try { await retired?.stop(); } catch { /* best-effort */ }
if (retired) retiring.add(retired);
try {
await Promise.allSettled([...localCalls]);
try { await retired?.stop(); } catch { /* best-effort */ }
} finally {
if (retired) retiring.delete(retired);
}
};
// Daemon-unavailable fallback: serve a client message in-process.
const handleLocally = (line: string): Promise<void> => {
@@ -352,6 +380,9 @@ export async function runLocalHandshakeProxy(deps: LocalHandshakeDeps): Promise<
let stdinBuf = '';
process.stdin.setEncoding('utf8');
process.stdin.on('data', (chunk: string) => {
// Nothing is served once shutdown starts: a call now could start an engine
// shutdown() never sees, and the process exits as soon as the others stop.
if (shuttingDown) return;
stdinBuf += chunk;
let idx: number;
while ((idx = stdinBuf.indexOf('\n')) !== -1) {