diff --git a/CHANGELOG.md b/CHANGELOG.md index f3dc0e2e0c..ea0df9d058 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -413,6 +413,8 @@ and adheres to [Semantic Versioning](https://semver.org/spec/v2.0.0.html). - On Windows, a project opened with a different drive-letter or folder-name case, like `d:\work\app` and `D:\Work\App`, now joins the CodeGraph background server that is already running for it. Before, each spelling started a server of its own that exited at once, so the session quietly served the graph by itself with no shared file watching or auto-sync, and `codegraph list` showed the project twice. Thanks @greatjackzhou. (#2278) - An agent session that lost its connection to CodeGraph's shared background server, or couldn't reach it at startup, no longer stops that server from running for the rest of the session. It reconnects on its own once the server can start, so other sessions on the same project get auto-sync back instead of running read-only. Thanks @ijbranch for the report. (#2277) - On Windows, `codegraph upgrade` no longer breaks the install while an agent session is using CodeGraph. It used to stop partway and leave CodeGraph unable to start; it now swaps the new files in with CodeGraph running and puts the previous version back if anything fails, and re-running the PowerShell installer works the same way. If an earlier upgrade already broke your install, re-run `irm https://raw.githubusercontent.com/colbymchenry/codegraph/main/install.ps1 | iex` to repair it. Thanks @tippmar-nr for the report. (#2185) +- On Windows, `codegraph init`, `codegraph index` and the background server's syncs no longer sometimes crash right after parsing on a busy machine (exit code 3221225477, an access violation). Shutting down the parsing threads could stop one while it was still starting up, which took the whole process down; they now finish starting first. +- On Windows, the CodeGraph background server no longer sometimes crashes as it shuts down (exit code 3221225477, an access violation) when it stops shortly after starting. Shutting down could stop one of its background threads while that thread was still starting up; it now lets the thread finish starting first. ## [1.6.1] - 2026-09-29 diff --git a/README.md b/README.md index 0b925ecac6..d198d1f2b1 100644 --- a/README.md +++ b/README.md @@ -52,7 +52,7 @@ Follow [@getcodegraph](https://x.com/getcodegraph) on X for updates. ## About this fork -This is **bompus/codegraph**, a fork of [colbymchenry/codegraph](https://github.com/colbymchenry/codegraph). Its default branch, `fork/consolidated`, contains upstream `main` through [`9d4e805b`](https://github.com/colbymchenry/codegraph/commit/9d4e805bec36d62332d7fba0a735329ba7621777) (after v1.6.1, merged 2026-10-02) plus the fork's own work, and it takes upstream changes as they land. Changes that suit upstream are also offered there as pull requests. +This is **bompus/codegraph**, a fork of [colbymchenry/codegraph](https://github.com/colbymchenry/codegraph). Its default branch, `fork/consolidated`, contains upstream `main` through [`3d86bcc0`](https://github.com/colbymchenry/codegraph/commit/3d86bcc0371079070d25901830fb2102100434c3) (after v1.6.1, merged 2026-10-02) plus the fork's own work, and it takes upstream changes as they land. Changes that suit upstream are also offered there as pull requests. The fork publishes no releases. The install scripts, npm package, badges and `codegraph upgrade` further down this page install **upstream's** releases. To run the fork, build it from source (below). diff --git a/__tests__/daemon-registry.test.ts b/__tests__/daemon-registry.test.ts index ccbb86ead1..d87a7e5d11 100644 --- a/__tests__/daemon-registry.test.ts +++ b/__tests__/daemon-registry.test.ts @@ -147,6 +147,48 @@ describe('daemon-registry', () => { } }, 15000); + it('waits for a daemon that has closed its socket but is still shutting down', async () => { + // A daemon's shutdown closes its socket first, then waits up to 15 s for + // query workers still starting up before it exits. Past the first 3 s the + // identity probe fails; that is a daemon still stopping, not a stranger. + const root = fs.mkdtempSync(path.join(tmpHome, 'stop-slow-')); + fs.mkdirSync(path.join(root, '.codegraph')); + const socketPath = getDaemonSocketPath(root); + const pid = startDetachedProcess(); + const server = net.createServer((socket) => { + socket.end(`${JSON.stringify({ protocol: 1, codegraph: 'test', pid, socketPath })}\n`); + }); + await new Promise((resolve, reject) => { + server.once('error', reject); + server.listen(socketPath, resolve); + }); + fs.writeFileSync( + getDaemonPidPath(root), + encodeLockInfo({ pid, version: 'test', socketPath, startedAt: Date.now() }), + ); + const originalKill = process.kill.bind(process); + let exitTimer: NodeJS.Timeout | undefined; + const kill = vi.spyOn(process, 'kill').mockImplementation((target, signal) => { + if (target === pid && signal === 'SIGTERM') { + server.close(); + exitTimer = setTimeout(() => originalKill(pid, 'SIGKILL'), 4000); + return true; + } + return originalKill(target, signal); + }); + try { + const result = await stopDaemonAt(root); + expect(result.outcome).toBe('term'); + expect(isProcessAlive(pid)).toBe(false); + expect(kill.mock.calls.some(([target, signal]) => target === pid && signal === 'SIGKILL')).toBe(false); + expect(fs.existsSync(getDaemonPidPath(root))).toBe(false); + } finally { + clearTimeout(exitTimer); + kill.mockRestore(); + fs.rmSync(root, { recursive: true, force: true }); + } + }, 15000); + it('preserves a newer lock installed during a successful stop identity probe', async () => { const root = fs.mkdtempSync(path.join(tmpHome, 'stop-race-')); fs.mkdirSync(path.join(root, '.codegraph')); diff --git a/__tests__/proxy-liveness-watchdog.test.ts b/__tests__/proxy-liveness-watchdog.test.ts index 8e9094ba45..b09bf91c55 100644 --- a/__tests__/proxy-liveness-watchdog.test.ts +++ b/__tests__/proxy-liveness-watchdog.test.ts @@ -45,7 +45,11 @@ async function exercise(mode: 'connecting-wedge' | 'fallback-wedge' | 'slow-fall await delay(2500); return { content: [{ type: 'text', text: 'finished' }] }; } }), - stop: () => {}, + // Slow, like a stop waiting on a query worker still starting up: the + // proxy must not exit before it settles. + stop: () => mode === 'slow-fallback' + ? delay(300).then(() => { require('fs').writeSync(1, 'STOPPED\\n'); }) + : undefined, }), }); `], { @@ -76,6 +80,7 @@ async function exercise(mode: 'connecting-wedge' | 'fallback-wedge' | 'slow-fall await until(exited); expect(child.exitCode, stderr).toBe(0); expect(child.signalCode).toBeNull(); + expect(stdout, stderr).toContain('STOPPED'); } else { await until(() => stdout.includes('WEDGED') || exited()); expect(stdout, stderr).toContain('WEDGED'); diff --git a/__tests__/query-pool.test.ts b/__tests__/query-pool.test.ts index 2087e6af47..7c28f48f04 100644 --- a/__tests__/query-pool.test.ts +++ b/__tests__/query-pool.test.ts @@ -182,6 +182,63 @@ describe('QueryPool', () => { expect(pool.healthy).toBe(false); }); + describe('destroy never terminates a worker still starting up', () => { + // Terminating a worker while it is still loading its modules can crash the + // whole process on Windows (0xC0000005), and the daemon destroys its pool + // whenever it stops. FakeWorker(…, null) never posts 'ready' by itself. + it('answers callers at once, then waits for the worker to start', async () => { + let worker!: FakeWorker; + const pool = new QueryPool({ + root: '/x', size: 1, softTimeoutMs: 10_000, + createWorker: () => (worker = new FakeWorker(() => ({ hang: true }), null)), + }); + const pending = pool.run('codegraph_explore', { query: 'q' }); + let destroyed = false; + const down = pool.destroy().then(() => { destroyed = true; }); + expect((await pending).isError).toBe(true); // not held behind the start + await sleep(50); + expect(destroyed).toBe(false); + expect(worker.alive).toBe(true); + worker.emitMessage({ type: 'ready', ok: true }); + await down; + expect(worker.alive).toBe(false); + }); + + it('terminates a started worker at once', async () => { + let worker!: FakeWorker; + const pool = new QueryPool({ root: '/x', size: 1, startSettleMs: 10_000, createWorker: () => (worker = new FakeWorker(() => ({ result: ok('r') }))) }); + await sleep(5); + const started = Date.now(); + await pool.destroy(); + expect(worker.alive).toBe(false); + expect(Date.now() - started).toBeLessThan(1_000); + }); + + it('gives up waiting on a start that never finishes', async () => { + let worker!: FakeWorker; + const pool = new QueryPool({ root: '/x', size: 1, startSettleMs: 60, createWorker: () => (worker = new FakeWorker(() => ({ hang: true }), null)) }); + const started = Date.now(); + await pool.destroy(); + expect(worker.alive).toBe(false); + expect(Date.now() - started).toBeGreaterThanOrEqual(55); + }); + + it('does not wait on a worker that died while starting', async () => { + const workers: FakeWorker[] = []; + const pool = new QueryPool({ + root: '/x', size: 1, startSettleMs: 10_000, + // The first never starts; its replacement does. + createWorker: () => { const w = new FakeWorker(() => ({ result: ok('r') }), workers.length ? true : null); workers.push(w); return w; }, + }); + workers[0].emitExit(); + await sleep(5); + const started = Date.now(); + await pool.destroy(); + expect(Date.now() - started).toBeLessThan(1_000); + expect(workers.every((w) => !w.alive)).toBe(true); + }); + }); + it('is not `ready` until a worker completes its cold start (#662 first-call stall)', async () => { // A worker cold start is seconds (tens under load); a call queued behind it // waits for the 45s busy backstop with nothing served. The ToolHandler must @@ -408,6 +465,50 @@ describe('MCP query pool with real projects (#1465)', () => { } }, 30000); + it('never terminates a real worker before it has started', async () => { + const alpha = await indexProject('alpha', 'alphaSymbol'); + const startedWhenEnded: boolean[] = []; + pool = new QueryPool({ + root: alpha, size: 1, + createWorker: () => { + const worker = new Worker(path.resolve(__dirname, '../dist/mcp/query-worker.js'), { workerData: { root: alpha } }); + let started = false; + worker.on('message', (m: { type?: string }) => { if (m?.type === 'ready') started = true; }); + const terminate = worker.terminate.bind(worker); + worker.terminate = () => { + startedWhenEnded.push(started); + return terminate(); + }; + return worker; + }, + }); + await pool.destroy(); // its eager worker is still loading + expect(startedWhenEnded).toEqual([true]); + }, 30000); + + it('engine stop waits for the query pool to shut down', async () => { + const alpha = await indexProject('alpha', 'alphaSymbol'); + const activeEngine = await start(alpha); + expect(pool).not.toBeNull(); + const realPool = pool!; + let release!: () => void; + const destroy = vi.spyOn(realPool, 'destroy').mockReturnValue(new Promise((r) => { release = r; })); + try { + let stopped = false; + const stopping = activeEngine.stop().then(() => { stopped = true; }); + // Everything else stop() closes is done well inside this; only the pool is held. + await Promise.race([stopping, sleep(1_500)]); + expect(stopped).toBe(false); + release(); + await stopping; + expect(stopped).toBe(true); + } finally { + release(); + destroy.mockRestore(); + await realPool.destroy(); + } + }, 30000); + it('honors size=0 for projectPath-only sessions', async () => { const alpha = await indexProject('alpha', 'alphaSymbol'); const workspace = path.join(tempDir, 'workspace'); diff --git a/src/extraction/parse-pool.ts b/src/extraction/parse-pool.ts index 44640d0ca8..383666ae58 100644 --- a/src/extraction/parse-pool.ts +++ b/src/extraction/parse-pool.ts @@ -70,11 +70,13 @@ const MAX_SCALED_PARSE_TIMEOUT_MS = 20_000; const HARD_KILL_MULTIPLIER = 3; /** * How long `destroy()` waits for a worker that has not yet reported - * 'grammars-loaded' before terminating it anyway. Upstream added this because - * terminating a worker mid WebAssembly grammar compile crashed the process on - * Windows (0xC0000005). This worker compiles no WebAssembly and answers as soon - * as its modules have loaded (the kernel loads on its first parse), so here the - * wait only spans that boot; the cap bounds a boot that is wedged. + * 'grammars-loaded' before terminating it anyway. The worker sends that once its + * modules have loaded (the kernel itself loads on the first parse). Terminating + * a worker while it is still loading its modules can take the whole process + * down with an access violation (0xC0000005, seen on Windows): a pool torn down + * right after a short index often has a late-spawned worker in exactly that + * state. A start finishes in well under a second normally and a few seconds + * under heavy load; the cap only bounds a start that is wedged. */ const GRAMMAR_LOAD_SETTLE_MS = 15_000; /** diff --git a/src/mcp/daemon-registry.ts b/src/mcp/daemon-registry.ts index c6c4043b60..e8bd3e969f 100644 --- a/src/mcp/daemon-registry.ts +++ b/src/mcp/daemon-registry.ts @@ -213,6 +213,14 @@ export async function clearStaleDaemonArtifacts(root: string): Promise const sleep = (ms: number): Promise => new Promise((r) => setTimeout(r, ms)); +/** + * How much longer `stopDaemonAt` waits for a daemon that has closed its socket + * but not yet exited. Its shutdown waits up to 15 s for each query worker still + * starting up (WORKER_START_SETTLE_MS in query-pool.ts), so this covers that + * plus the rest of the shutdown. + */ +const DAEMON_SHUTDOWN_GRACE_MS = 17_000; + async function waitForDeath(pid: number, timeoutMs: number): Promise { const deadline = Date.now() + timeoutMs; while (Date.now() < deadline) { @@ -287,9 +295,18 @@ export async function stopDaemonAt(root: string, options: { preserveUnverified?: let outcome: StopResult['outcome'] = 'term'; if (!(await waitForDeath(pid, 3000))) { // 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()) return { root, pid, outcome: 'still-running' }; + if (!await probeDaemonIdentity(identity)) { + // Socket closed, lock still held: the daemon is partway through its + // shutdown, which waits on query workers still starting up. Wait for + // it rather than report it still running; nothing is signalled here. + if (!sameLock() || !(await waitForDeath(pid, DAEMON_SHUTDOWN_GRACE_MS))) { + return { root, pid, outcome: 'still-running' }; + } + cleanupDaemonArtifacts(root, lockContents); + return { root, pid, outcome: 'term' }; } + if (!sameLock()) 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' }; diff --git a/src/mcp/engine.ts b/src/mcp/engine.ts index 4a4818b2a1..d666bad07f 100644 --- a/src/mcp/engine.ts +++ b/src/mcp/engine.ts @@ -271,13 +271,13 @@ export class MCPEngine { // Detach + terminate the worker pool first so no tool call routes to a // worker mid-teardown; outstanding pool calls resolve with graceful guidance. + // Stopping waits for the workers to end: the daemon exits right after, and + // exiting while a worker is still starting up can crash the process. this.toolHandler.setQueryPool(null); - if (this.queryPool) { - void this.queryPool.destroy(); - this.queryPool = null; - } + const poolDown = this.queryPool ? this.queryPool.destroy() : Promise.resolve(); + this.queryPool = null; const drained = this.toolHandler.closeAll(); - this.stopPromise = drained.then(async () => { + this.stopPromise = Promise.all([drained, poolDown]).then(async () => { if (this.initPromise) await this.initPromise; if (this.defaultLease) { await this.defaultLease.release(); diff --git a/src/mcp/proxy.ts b/src/mcp/proxy.ts index 56f0d69f15..5ba688f70b 100644 --- a/src/mcp/proxy.ts +++ b/src/mcp/proxy.ts @@ -250,6 +250,8 @@ export async function runLocalHandshakeProxy(deps: LocalHandshakeDeps): Promise< let engineReady: Promise | null = null; // Calls being served in-process; the engine is only ever stopped after they finish. const localCalls = new Set>(); + // Engines retired but not yet stopped; shutdown() waits for them too. + const retiring = new Set(); 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 +284,18 @@ 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). The backstop bounds a stop that never settles. + setTimeout(() => process.exit(0), 20_000); + 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 => { + // 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 +310,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 => { @@ -352,6 +365,7 @@ export async function runLocalHandshakeProxy(deps: LocalHandshakeDeps): Promise< let stdinBuf = ''; process.stdin.setEncoding('utf8'); process.stdin.on('data', (chunk: string) => { + if (shuttingDown) return; stdinBuf += chunk; let idx: number; while ((idx = stdinBuf.indexOf('\n')) !== -1) { diff --git a/src/mcp/query-pool.ts b/src/mcp/query-pool.ts index 4e3463d76f..630191ab42 100644 --- a/src/mcp/query-pool.ts +++ b/src/mcp/query-pool.ts @@ -84,6 +84,17 @@ function resolveIdleRetireMs(): number { return Number.isFinite(n) && n >= 0 ? Math.floor(n) : DEFAULT_IDLE_RETIRE_MS; } +/** + * How long `destroy()` waits for a worker that is still starting up before + * terminating it anyway. Terminating a worker while it is still loading its + * modules can take the whole process down with an access violation + * (0xC0000005, seen on Windows), and the daemon shuts its pool down whenever + * it stops — a session that ends within a few seconds of starting has a + * worker in exactly that state. A start takes about a second normally and a + * few under heavy load; the cap only bounds a start that is wedged. + */ +const WORKER_START_SETTLE_MS = 15_000; + /** Shape of a message a worker posts back (ready handshake or a tool result). */ interface WorkerMessage { type?: string; @@ -116,6 +127,8 @@ export interface QueryPoolOptions { createWorker?: () => PoolWorker; /** Idle time before a worker beyond the first is retired; 0 never retires. */ idleRetireMs?: number; + /** How long destroy() waits on a worker still starting up (tests shorten it). Default 15s. */ + startSettleMs?: number; } /** @@ -167,6 +180,10 @@ export class QueryPool { // a large DB open) saturate the box and starve the main loop. Grow only when // the queue outstrips idle + pending. private pendingWorkers = new Set(); + // Each worker's start, settled when it posts 'ready' (either way) or goes + // away — what destroy() waits on before terminating it (see + // WORKER_START_SETTLE_MS). + private starts = new Map; settle: () => void }>(); private nextId = 1; private totalCrashes = 0; private destroyed = false; @@ -179,6 +196,7 @@ export class QueryPool { private idleSince = new Map(); private readonly idleRetireMs: number; private retireTimer?: ReturnType; + private readonly startSettleMs: number; constructor(opts: QueryPoolOptions) { this.root = opts.root; @@ -191,6 +209,7 @@ export class QueryPool { this.retireTimer = setInterval(() => this.retireIdle(), Math.max(1_000, this.idleRetireMs / 4)); this.retireTimer.unref?.(); } + this.startSettleMs = opts.startSettleMs ?? WORKER_START_SETTLE_MS; this.spawnOne(); // one eager warm worker, ready for the first call } @@ -262,12 +281,18 @@ export class QueryPool { } this.workers.add(w); this.pendingWorkers.add(w); + let settle!: () => void; + const settled = new Promise((resolve) => { settle = resolve; }); + this.starts.set(w, { settled, settle }); w.on('message', (m) => this.onMessage(w, (m ?? {}) as WorkerMessage)); - w.on('error', () => this.onWorkerGone(w)); - w.on('exit', (code) => { if (code !== 0) this.onWorkerGone(w); }); + w.on('error', () => { this.startSettled(w); this.onWorkerGone(w); }); + w.on('exit', (code) => { this.startSettled(w); if (code !== 0) this.onWorkerGone(w); }); } private onMessage(w: PoolWorker, m: WorkerMessage): void { + // Before the retired-worker check: destroy() waits on this for a worker + // it has already let go of. + if (m?.type === 'ready') this.startSettled(w); if (!m || !this.workers.has(w)) return; // ignore late messages from retired workers if (m.type === 'ready') { if (!this.pendingWorkers.delete(w)) return; // already handled this handshake @@ -363,7 +388,36 @@ export class QueryPool { }); } - /** Terminate all workers and answer any outstanding calls gracefully. */ + private startSettled(w: PoolWorker): void { + const start = this.starts.get(w); + if (!start) return; + this.starts.delete(w); + start.settle(); + } + + /** + * Terminate a worker, but not while it is still starting up: wait for it to + * post 'ready' (or go away), up to `startSettleMs` (WORKER_START_SETTLE_MS — + * see that constant for the crash this avoids). + */ + private async terminateStarted(w: PoolWorker): Promise { + const start = this.starts.get(w); + if (start) { + let cap: NodeJS.Timeout | undefined; + await Promise.race([ + start.settled, + new Promise((resolve) => { cap = setTimeout(resolve, this.startSettleMs); cap.unref?.(); }), + ]); + clearTimeout(cap); + } + try { await w.terminate(); } catch { /* already gone */ } + } + + /** + * Terminate all workers and answer any outstanding calls gracefully. A worker + * still starting up is terminated once it has started (see + * WORKER_START_SETTLE_MS); outstanding calls are answered first. + */ async destroy(): Promise { if (this.destroyed) return; this.destroyed = true; @@ -377,6 +431,6 @@ export class QueryPool { } this.inflight.clear(); this.queue = []; - await Promise.all(ws.map((w) => Promise.resolve(w.terminate()).catch(() => { /* already gone */ }))); + await Promise.all(ws.map((w) => this.terminateStarted(w))); } }