mirror of
https://github.com/colbymchenry/codegraph.git
synced 2026-10-28 22:52:00 +08:00
fix(daemon): a long sync of another project no longer gets the shared daemon killed (#2793)
Root cause: the daemon arms its liveness watchdog with its own project's index files (codegraph.db + -wal), and since PR #1231 a silent timeout defers the kill while those files advance. But the daemon also writes every other project a session opens with projectPath: the project lifecycle takes that project's writer lock, starts its watcher and runs the catch-up sync, all storing on the main thread. A long synchronous stretch of such a sync left the daemon's own files still, so the watchdog read it as a wedge and SIGKILLed a healthy daemon mid-sync; the next session reattached the project and the cycle repeated. A proxy serving in-process, armed with no files at all, had the same blind spot for its own project. Fix: addWatchdogProgressPaths() lets a process add files to its watchdog at runtime. The project lifecycle adds a project's db and -wal the moment this process becomes the project's writer, and drops them after it closes the project, so only indexes this process writes count, never one another process owns. Each change re-arms the watchdog with a new child whose argv carries the whole list; the old child is stopped after the new one starts. Added files take effect at once (a sync may block the loop right after); dropped ones wait a turn, so a server releasing its projects on the way out re-arms once or not at all. A message over the heartbeat pipe was tried first and dropped: Node's Writable holds a second write while one is still in flight, so the -wal line of a db + -wal registration never reached the child before a block that followed in the same tick (4 of 4 runs). Validation: - New __tests__/liveness-watchdog-projects.test.ts (11 cases, real watchdog child, lowered timeout): main's source 9 fail / 2 pass (the two guards), fix 11 pass. The lifecycle case arms the watchdog for project A, attaches B through the project lifecycle and stores into B's index on the main thread for 3s: killed at ~1s on main, runs to completion with the fix. - 8 mutations, each failed at least one case: no lifecycle registration, registration before the writer-lock check, no drop on release, no re-arm on add, no re-arm on drop, old child not stopped, drops re-armed at once, adds deferred. - Real daemon on Windows (watchdog timeout 5s, daemon keyed on a 2-file project, second project excalidraw 1d4dc2c with all 687 TS files edited, attached with codegraph_status projectPath), 3 interleaved rounds: main killed 3/3 about 13s after attaching B ("Main thread unresponsive for ~5s"), leaving B with 27,698 edges and 72,534 pending refs; the fix survived 3/3 through 7.8-9.3s main-thread blocks and finished B (61,120 edges, 0 pending), and 3/3 again on the final build (9.7-9.9s blocks). Fixes #2404 Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
This commit is contained in:
co-authored by
Claude Opus 5.5
parent
8651990abd
commit
b2a84873ed
+146
-28
@@ -30,10 +30,11 @@
|
||||
*
|
||||
* **Won't fire on real work.** Heavy parsing runs in the parse worker
|
||||
* (off-thread) and the daemon's indexing shells out to a child process, so the
|
||||
* daemon's main thread only ever does fast, bounded work. The default timeout
|
||||
* is ~300× the 5h #850 wedge shorter, yet far longer than any legitimate
|
||||
* main-thread block. Opt out with `CODEGRAPH_NO_WATCHDOG=1`; tune with
|
||||
* `CODEGRAPH_WATCHDOG_TIMEOUT_MS`.
|
||||
* daemon's main thread mostly does fast, bounded work; the exception, a sync
|
||||
* storing into SQLite, is what the disk-progress deferral below is for. The
|
||||
* default timeout is ~300× the 5h #850 wedge shorter, yet far longer than any
|
||||
* legitimate main-thread block. Opt out with `CODEGRAPH_NO_WATCHDOG=1`; tune
|
||||
* with `CODEGRAPH_WATCHDOG_TIMEOUT_MS`.
|
||||
*
|
||||
* **Disk-progress deferral (`progressPaths`).** The CLI `index`/`init` path is
|
||||
* different: it runs the SQLite store on this thread, and one long synchronous
|
||||
@@ -49,6 +50,18 @@
|
||||
* coinciding with unrelated file activity — or I/O hung beyond all reason —
|
||||
* still dies. A true wedge with no disk progress dies at the base timeout,
|
||||
* exactly as before.
|
||||
*
|
||||
* **Every index the process writes counts (#2404).** An MCP server syncs on
|
||||
* its main thread too, and not only the project it was armed for: the shared
|
||||
* daemon writes every project a session names by `projectPath`, and a proxy's
|
||||
* in-process fallback writes its own. {@link addWatchdogProgressPaths} adds a
|
||||
* project's index files the moment this process becomes its writer and drops
|
||||
* them when it lets go; each change re-arms the watchdog with a new child
|
||||
* whose argv carries the whole list. A new process gets its list the instant
|
||||
* it is spawned, whatever the parent does next: a message on the heartbeat
|
||||
* pipe could not promise that, since a pipe write queued behind one still in
|
||||
* flight waits for an event-loop turn, and a sync can block the loop right
|
||||
* after the project is added.
|
||||
*/
|
||||
import * as fs from 'fs';
|
||||
import * as os from 'os';
|
||||
@@ -174,27 +187,71 @@ export interface WatchdogOptions {
|
||||
/**
|
||||
* Files whose size/mtime advancing counts as forward progress (the SQLite
|
||||
* DB + `-wal` for an in-process indexer). With paths supplied, a silent
|
||||
* timeout only kills when the files did NOT advance — see the header. Omit
|
||||
* for pure heartbeat behavior (the daemon, whose main thread never runs
|
||||
* long synchronous work).
|
||||
* timeout only kills when the files did NOT advance — see the header. The
|
||||
* files of other indexes the process comes to write join at runtime through
|
||||
* {@link addWatchdogProgressPaths}; with neither, a silent timeout kills.
|
||||
*/
|
||||
progressPaths?: string[];
|
||||
}
|
||||
|
||||
/**
|
||||
* Install the main-thread liveness watchdog for a long-lived process. Returns a
|
||||
* handle to stop it, or `null` when disabled or when the child can't be spawned
|
||||
* (degraded, never throws — a missing watchdog must never keep a process from
|
||||
* starting).
|
||||
* Files added with {@link addWatchdogProgressPaths} and not yet dropped, with
|
||||
* how many registrations hold each.
|
||||
*/
|
||||
export function installMainThreadWatchdog(options: WatchdogOptions = {}): WatchdogHandle | null {
|
||||
if (isEnvTruthy(process.env.CODEGRAPH_NO_WATCHDOG)) return null;
|
||||
const addedProgressPaths = new Map<string, number>();
|
||||
|
||||
const timeoutMs = parseWatchdogTimeoutMs(process.env.CODEGRAPH_WATCHDOG_TIMEOUT_MS);
|
||||
const checkMs = deriveCheckIntervalMs(timeoutMs);
|
||||
const capMs = timeoutMs * PROGRESS_CAP_MULTIPLIER;
|
||||
const progressPaths = options.progressPaths ?? [];
|
||||
/**
|
||||
* Tells each armed watchdog the list of files changed: `added` when a file
|
||||
* joined it, false when files only left it.
|
||||
*/
|
||||
const armedWatchdogs = new Set<(added: boolean) => void>();
|
||||
|
||||
/**
|
||||
* Count `paths` advancing as forward progress for this process's liveness
|
||||
* watchdog, armed now or later, until the returned function is called
|
||||
* (#2404). For the index files of a project this process has just become the
|
||||
* writer of: its syncs store on the main thread, and without this a long one
|
||||
* reads as a wedge whenever the project is not the one the watchdog was armed
|
||||
* for. Registrations of one file nest, and the returned function is
|
||||
* idempotent.
|
||||
*/
|
||||
export function addWatchdogProgressPaths(paths: readonly string[]): () => void {
|
||||
let added = false;
|
||||
for (const file of paths) {
|
||||
const held = addedProgressPaths.get(file) ?? 0;
|
||||
addedProgressPaths.set(file, held + 1);
|
||||
if (held === 0) added = true;
|
||||
}
|
||||
if (added) for (const changed of armedWatchdogs) changed(true);
|
||||
let dropped = false;
|
||||
return () => {
|
||||
if (dropped) return;
|
||||
dropped = true;
|
||||
let removed = false;
|
||||
for (const file of paths) {
|
||||
const held = addedProgressPaths.get(file) ?? 0;
|
||||
if (held > 1) {
|
||||
addedProgressPaths.set(file, held - 1);
|
||||
} else if (held === 1) {
|
||||
addedProgressPaths.delete(file);
|
||||
removed = true;
|
||||
}
|
||||
}
|
||||
if (removed) for (const changed of armedWatchdogs) changed(false);
|
||||
};
|
||||
}
|
||||
|
||||
/** One watchdog child process, watching a fixed list of files. */
|
||||
interface WatchdogChild {
|
||||
paths: readonly string[];
|
||||
/** Write one heartbeat byte. */
|
||||
beat(): void;
|
||||
/** End the child: EOF, then a kill for good measure. */
|
||||
stop(): void;
|
||||
}
|
||||
|
||||
/** Spawn a watchdog child for `paths`, or null when it can't be spawned. */
|
||||
function spawnWatchdogChild(timeoutMs: number, checkMs: number, capMs: number, paths: readonly string[]): WatchdogChild | null {
|
||||
let child: ChildProcess;
|
||||
try {
|
||||
// No execArgv inheritance (unlike Worker), so the child carries none of our
|
||||
@@ -202,7 +259,7 @@ export function installMainThreadWatchdog(options: WatchdogOptions = {}): Watchd
|
||||
// fd 2 so the kill notice lands wherever the parent logs (daemon.log).
|
||||
child = spawn(
|
||||
process.execPath,
|
||||
['-e', CHILD_SOURCE, String(process.pid), String(timeoutMs), String(capMs), ...progressPaths],
|
||||
['-e', CHILD_SOURCE, String(process.pid), String(timeoutMs), String(capMs), ...paths],
|
||||
{
|
||||
stdio: ['pipe', 'ignore', 'inherit'],
|
||||
windowsHide: true,
|
||||
@@ -228,27 +285,88 @@ export function installMainThreadWatchdog(options: WatchdogOptions = {}): Watchd
|
||||
stdin.on('error', () => { /* child gone; heartbeat writes are best-effort */ });
|
||||
child.on('error', (err) => debug(`child error: ${err.message}`));
|
||||
|
||||
// Heartbeat: a byte per tick. When the main thread wedges, these stop and the
|
||||
// child's timeout fires. unref'd so it never keeps the process alive itself.
|
||||
const heartbeat = setInterval(() => {
|
||||
try { stdin.write('\n'); } catch { /* child gone */ }
|
||||
}, checkMs);
|
||||
heartbeat.unref();
|
||||
|
||||
// Neither the child nor its pipe should keep the parent alive past its work.
|
||||
child.unref();
|
||||
try { (stdin as unknown as { unref?: () => void }).unref?.(); } catch { /* ignore */ }
|
||||
|
||||
debug(`armed (child pid ${child.pid ?? '?'}): timeoutMs=${timeoutMs} checkMs=${checkMs} progressPaths=${progressPaths.length}`);
|
||||
debug(`armed (child pid ${child.pid ?? '?'}): timeoutMs=${timeoutMs} checkMs=${checkMs} progressPaths=${paths.length}`);
|
||||
|
||||
return {
|
||||
paths,
|
||||
beat(): void {
|
||||
try { stdin.write('\n'); } catch { /* child gone */ }
|
||||
},
|
||||
stop(): void {
|
||||
try { stdin.end(); } catch { /* ignore */ } // EOF -> child exits cleanly
|
||||
try { child.kill(); } catch { /* ignore */ } // belt-and-suspenders
|
||||
},
|
||||
};
|
||||
}
|
||||
|
||||
/**
|
||||
* Install the main-thread liveness watchdog for a long-lived process. Returns a
|
||||
* handle to stop it, or `null` when disabled or when the child can't be spawned
|
||||
* (degraded, never throws — a missing watchdog must never keep a process from
|
||||
* starting).
|
||||
*/
|
||||
export function installMainThreadWatchdog(options: WatchdogOptions = {}): WatchdogHandle | null {
|
||||
if (isEnvTruthy(process.env.CODEGRAPH_NO_WATCHDOG)) return null;
|
||||
|
||||
const timeoutMs = parseWatchdogTimeoutMs(process.env.CODEGRAPH_WATCHDOG_TIMEOUT_MS);
|
||||
const checkMs = deriveCheckIntervalMs(timeoutMs);
|
||||
const capMs = timeoutMs * PROGRESS_CAP_MULTIPLIER;
|
||||
const armedPaths = options.progressPaths ?? [];
|
||||
const watchedPaths = (): string[] => [...new Set([...armedPaths, ...addedProgressPaths.keys()])];
|
||||
|
||||
let current = spawnWatchdogChild(timeoutMs, checkMs, capMs, watchedPaths());
|
||||
if (!current) return null;
|
||||
|
||||
// Heartbeat: a byte per tick. When the main thread wedges, these stop and the
|
||||
// child's timeout fires. unref'd so it never keeps the process alive itself.
|
||||
const heartbeat = setInterval(() => current?.beat(), checkMs);
|
||||
heartbeat.unref();
|
||||
|
||||
// The list of files changed (#2404): a child for the new list takes over,
|
||||
// then the old one goes. Always from a turning event loop, so the old child,
|
||||
// its last heartbeat recent, has no reason to kill meanwhile; a spawn that
|
||||
// fails leaves it watching the old list. An added file takes effect at once,
|
||||
// since a sync may block the loop right after. Dropped files wait for the
|
||||
// next turn, so a process letting go of several projects, as one does on its
|
||||
// way out, re-arms once or not at all.
|
||||
let pendingRearm: NodeJS.Timeout | null = null;
|
||||
const rearm = (): void => {
|
||||
if (pendingRearm) clearTimeout(pendingRearm);
|
||||
pendingRearm = null;
|
||||
if (!current) return;
|
||||
const paths = watchedPaths();
|
||||
if (paths.length === current.paths.length && paths.every((p) => current!.paths.includes(p))) return;
|
||||
const next = spawnWatchdogChild(timeoutMs, checkMs, capMs, paths);
|
||||
if (!next) return;
|
||||
const previous = current;
|
||||
current = next;
|
||||
previous.stop();
|
||||
};
|
||||
const changed = (added: boolean): void => {
|
||||
if (added) {
|
||||
rearm();
|
||||
} else if (!pendingRearm) {
|
||||
pendingRearm = setTimeout(rearm, 0);
|
||||
pendingRearm.unref();
|
||||
}
|
||||
};
|
||||
armedWatchdogs.add(changed);
|
||||
|
||||
let stopped = false;
|
||||
return {
|
||||
stop(): void {
|
||||
if (stopped) return;
|
||||
stopped = true;
|
||||
armedWatchdogs.delete(changed);
|
||||
if (pendingRearm) clearTimeout(pendingRearm);
|
||||
pendingRearm = null;
|
||||
clearInterval(heartbeat);
|
||||
try { stdin.end(); } catch { /* ignore */ } // EOF -> child exits cleanly
|
||||
try { child.kill(); } catch { /* ignore */ } // belt-and-suspenders
|
||||
current?.stop();
|
||||
current = null;
|
||||
},
|
||||
};
|
||||
}
|
||||
|
||||
@@ -1,10 +1,12 @@
|
||||
/** Shared synchronization ownership for projects accessed by MCP engines (#1835). */
|
||||
import { realpathSync } from 'fs';
|
||||
import * as path from 'path';
|
||||
import type { Socket } from 'net';
|
||||
import type CodeGraph from '../index';
|
||||
import { canonicalProjectRoot, isInitialized } from '../directory';
|
||||
import { canonicalProjectRoot, getCodeGraphDir, isInitialized } from '../directory';
|
||||
import { LockUnavailableError, watchDisabledReason } from '../sync';
|
||||
import { getDaemonSocketCandidates } from './daemon-paths';
|
||||
import { addWatchdogProgressPaths } from './liveness-watchdog';
|
||||
import { connectWithHello } from './proxy';
|
||||
import { markWriterReady, readWriterLock, releaseWriterLock, tryAcquireWriterLock } from './writer-lock';
|
||||
|
||||
@@ -20,6 +22,8 @@ interface Project {
|
||||
socket: Socket | null;
|
||||
timer: NodeJS.Timeout;
|
||||
options: Parameters<CodeGraph['watch']>[0];
|
||||
/** Drops the index files from the liveness watchdog's progress; set while this process writes them (#2404). */
|
||||
dropProgress: (() => void) | null;
|
||||
}
|
||||
|
||||
export interface ProjectLease {
|
||||
@@ -42,7 +46,7 @@ export function acquireProject(
|
||||
if (!project) {
|
||||
const cg = open();
|
||||
project = { key, cg, refs: 0, owner: false, caughtUp: false, retirement: null, gate: null, socket: null, options,
|
||||
timer: setInterval(() => { void ready(root, project!); }, 1000) };
|
||||
dropProgress: null, timer: setInterval(() => { void ready(root, project!); }, 1000) };
|
||||
project.timer.unref();
|
||||
projects.set(key, project);
|
||||
}
|
||||
@@ -86,6 +90,9 @@ function retire(root: string, project: Project): Promise<void> {
|
||||
projects.delete(project.key);
|
||||
project.cg.close();
|
||||
if (project.owner) releaseWriterLock(root);
|
||||
// After the close, which may still checkpoint the WAL on this thread.
|
||||
project.dropProgress?.();
|
||||
project.dropProgress = null;
|
||||
resolve();
|
||||
};
|
||||
finish();
|
||||
@@ -144,6 +151,15 @@ async function activate(root: string, project: Project): Promise<void> {
|
||||
return;
|
||||
}
|
||||
project.owner = true;
|
||||
// This process writes the index from here on, in long synchronous stretches
|
||||
// on the main thread: the catch-up below and every watcher sync. The
|
||||
// liveness watchdog must count its files advancing as progress, as it does
|
||||
// for the project it was armed with; otherwise a long sync of any other
|
||||
// project reads as a wedge and the server is killed mid-sync (#2404).
|
||||
if (!project.dropProgress) {
|
||||
const db = path.join(getCodeGraphDir(root), 'codegraph.db');
|
||||
project.dropProgress = addWatchdogProgressPaths([db, `${db}-wal`]);
|
||||
}
|
||||
const disabled = watchDisabledReason(root);
|
||||
if (disabled) {
|
||||
process.stderr.write(`[CodeGraph MCP] File watcher disabled for ${root} — ${disabled}.\n`);
|
||||
|
||||
Reference in New Issue
Block a user