From 304664d4dc2087a75da97e74585fd3e1c2ba560a Mon Sep 17 00:00:00 2001 From: Aaron Queen Date: Sun, 6 Sep 2026 01:15:05 -0600 Subject: [PATCH] fix(sessions): retry a WAL conversion that collides on a fresh database MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit `busy_timeout` covers an ordinary lock wait on `journal_mode = WAL` — a connection merely holding the database is waited out and the conversion then succeeds. What it does not cover is several processes converting the same brand-new file at the same moment: they collide inside the conversion rather than queueing on a lock, and one loses. Only the first run can hit this, since WAL is persistent in the file, but the first run is exactly when parallel tool calls all open the index at once. Both errors the collision raises are transient. Retrying either inside the existing busy budget takes six concurrent openers of one fresh database from 22 failures in 300 runs to 0 in 480. Tolerating a failed conversion instead does not work: the connection does not survive one, and the next statement on it fails too. --- __tests__/sessions-index.test.ts | 54 ++++++++++++++++++++++++++++++++ src/sessions/index.ts | 36 ++++++++++++++++++++- 2 files changed, 89 insertions(+), 1 deletion(-) diff --git a/__tests__/sessions-index.test.ts b/__tests__/sessions-index.test.ts index e4b3fe2fd..6c02c3490 100644 --- a/__tests__/sessions-index.test.ts +++ b/__tests__/sessions-index.test.ts @@ -21,6 +21,7 @@ import { } from '../src/sessions/claude-code'; import { SessionsIndex, + enterWalMode, ftsQuery, querySessions, sessionsSourceDir, @@ -204,6 +205,59 @@ describe('SessionsIndex', () => { expect(index.search('pool transcript threads')).toHaveLength(1); index.close(); }); + + it('opens a fresh database while another connection holds it, converting to WAL once free', async () => { + // An ordinary lock wait on the conversion is covered by busy_timeout: the + // holder commits and this open then converts, rather than throwing. + const dbPath = path.join(fixtureDir(), 'sessions.db'); + const other = new Worker( + `const { workerData, parentPort } = require('worker_threads'); + const { DatabaseSync } = require('node:sqlite'); + const db = new DatabaseSync(workerData.dbPath); + db.exec('BEGIN EXCLUSIVE'); + parentPort.postMessage('locked'); + Atomics.wait(new Int32Array(new SharedArrayBuffer(4)), 0, 0, 300); + db.exec('COMMIT'); + db.close();`, + { eval: true, workerData: { dbPath } }, + ); + await new Promise((resolve) => other.once('message', resolve)); + const index = SessionsIndex.open(dbPath); + await new Promise((resolve) => other.once('exit', resolve)); + const db = createDatabase(dbPath).db; + expect(String(db.pragma('journal_mode', { simple: true })).toLowerCase()).toBe('wal'); + db.close(); + index.close(); + }); + + it('retries a WAL conversion that collides with another process converting the same fresh file', () => { + // What busy_timeout does NOT cover: several processes converting one + // brand-new database at the same moment collide inside the conversion + // rather than queueing on a lock, and it surfaces as either of these two + // transient errors. That collision only reproduces probabilistically, so + // the retry is driven directly here. + for (const message of ['database is locked', 'disk I/O error']) { + let calls = 0; + const db = { + pragma(sql: string) { + if (!/journal_mode\s*=/i.test(sql)) return 'delete'; + if (++calls < 3) throw new Error(message); + return 'wal'; + }, + }; + expect(() => enterWalMode(db as never)).not.toThrow(); + expect(calls).toBe(3); + } + }); + + it('gives up on a WAL conversion error that is not the transient collision', () => { + const db = { + pragma() { + throw new Error('unable to open database file'); + }, + }; + expect(() => enterWalMode(db as never)).toThrow(/unable to open database file/); + }); }); describe('querySessions (project entry point)', () => { diff --git a/src/sessions/index.ts b/src/sessions/index.ts index 57228dc88..ecb91d4f2 100644 --- a/src/sessions/index.ts +++ b/src/sessions/index.ts @@ -36,6 +36,40 @@ export const BUSY_TIMEOUT_MS = 5000; /** Bump when the readers' notion of prose changes, so existing indexes rebuild. */ const INDEX_VERSION = 2; +/** + * `busy_timeout` does cover an ordinary lock wait on this pragma: a connection + * that merely holds the database is waited out and the conversion then + * succeeds. What it does not cover is several processes converting the SAME + * brand-new database at the same moment — they collide inside the conversion + * itself rather than queueing on a lock. That is only ever the first run: WAL + * is persistent, so once the file is in WAL nobody converts it again. + * + * Both errors that collision raises are transient and clear on their own. + * `database is locked` is the conversion losing the race; `disk I/O error` is + * the shared-memory `-shm` file being created underneath a concurrent opener, + * seen on Windows. Retrying either inside the existing budget is enough. + * Tolerating a failed conversion is not an option: the connection does not + * survive one, and the next statement on it fails too. + * + * Exported for the test that drives the retry directly — the collision itself + * only reproduces probabilistically, so the retry is asserted here instead. + */ +export function enterWalMode(db: SqliteDatabase): void { + const deadline = Date.now() + BUSY_TIMEOUT_MS; + for (;;) { + try { + db.pragma('journal_mode = WAL'); + return; + } catch (err) { + const message = err instanceof Error ? err.message : String(err); + const transient = /database is locked|database is busy|disk i\/o error/i.test(message); + if (!transient || Date.now() >= deadline) throw err; + // Jittered, so the losers of one collision do not retry in lockstep. + Atomics.wait(new Int32Array(new SharedArrayBuffer(4)), 0, 0, 5 + Math.random() * 20); + } + } +} + export interface SessionsIndexStats { /** Transcript files seen. */ files: number; @@ -122,7 +156,7 @@ export class SessionsIndex { // of waiting the few hundred milliseconds the winner's write takes. Set // before the constructor's schema and version writes, which race the same way. db.pragma(`busy_timeout = ${BUSY_TIMEOUT_MS}`); - if (dbPath !== ':memory:') db.pragma('journal_mode = WAL'); + if (dbPath !== ':memory:') enterWalMode(db); return new SessionsIndex(db); }