Skip to content

Commit 17e265b

Browse files
committed
fix(native): thread config.db.busyTimeoutMs into NativeDatabase open factories
NativeDatabase::open_readonly/open_read_write hardcoded PRAGMA busy_timeout = 5000 in the Rust native DB layer, ignoring config.db.busyTimeoutMs entirely (#1763 only wired the TS-side better-sqlite3 pragma). Add an optional busy_timeout_ms param to both factories, defaulting to 5000 when omitted, and thread the already-resolved value through all six JS call sites: openRepo (via openRepoNative/tryOpenRepoNative) and openReadonlyWithNative in db/connection.ts, reopenNativeDb in native-db-lifecycle.ts, the two NativeDatabase.openReadWrite call sites in native-orchestrator.ts, and branch-compare.ts's fan-metrics helper (which previously did no config resolution at all). Impact: 9 functions changed, 47 affected
1 parent ab6a685 commit 17e265b

8 files changed

Lines changed: 234 additions & 19 deletions

File tree

crates/codegraph-core/src/db/connection.rs

Lines changed: 23 additions & 8 deletions
Original file line numberDiff line numberDiff line change
@@ -16,6 +16,11 @@ use crate::db::repository::edges::{self, EdgeRow};
1616
use crate::domain::graph::builder::stages::insert_nodes::{self, FileHashEntry, InsertNodesBatch};
1717
use crate::graph::classifiers::roles::{self, RoleSummary};
1818

19+
/// Fallback `PRAGMA busy_timeout` (ms) used when the caller doesn't pass a
20+
/// resolved value. Mirrors `DEFAULTS.db.busyTimeoutMs` in
21+
/// `src/infrastructure/config.ts` — keep both in sync.
22+
const DEFAULT_BUSY_TIMEOUT_MS: u32 = 5000;
23+
1924
// ── Migration DDL (mirrored from src/db/migrations.ts) ──────────────────
2025

2126
struct Migration {
@@ -483,8 +488,12 @@ pub struct NativeDatabase {
483488
impl NativeDatabase {
484489
/// Open a read-write connection to the database at `db_path`.
485490
/// Creates the file and parent directories if they don't exist.
491+
///
492+
/// `busy_timeout_ms` mirrors `config.db.busyTimeoutMs` on the TS side
493+
/// (`DEFAULTS.db.busyTimeoutMs` in `src/infrastructure/config.ts`);
494+
/// defaults to `DEFAULT_BUSY_TIMEOUT_MS` when omitted.
486495
#[napi(factory)]
487-
pub fn open_read_write(db_path: String) -> napi::Result<Self> {
496+
pub fn open_read_write(db_path: String, busy_timeout_ms: Option<u32>) -> napi::Result<Self> {
488497
let flags = OpenFlags::SQLITE_OPEN_READ_WRITE
489498
| OpenFlags::SQLITE_OPEN_CREATE
490499
| OpenFlags::SQLITE_OPEN_NO_MUTEX;
@@ -498,12 +507,13 @@ impl NativeDatabase {
498507
// are not cache-coherent on Windows, leading to SQLITE_CORRUPT (#715).
499508
// Read-only connections keep mmap since they don't share a WAL file
500509
// with a concurrent writer from a different library.
501-
conn.execute_batch(
510+
let busy_timeout_ms = busy_timeout_ms.unwrap_or(DEFAULT_BUSY_TIMEOUT_MS);
511+
conn.execute_batch(&format!(
502512
"PRAGMA journal_mode = WAL; \
503513
PRAGMA synchronous = NORMAL; \
504-
PRAGMA busy_timeout = 5000; \
514+
PRAGMA busy_timeout = {busy_timeout_ms}; \
505515
PRAGMA temp_store = MEMORY;",
506-
)
516+
))
507517
.map_err(|e| napi::Error::from_reason(format!("Failed to set pragmas: {e}")))?;
508518
Ok(Self {
509519
conn: SendWrapper::new(Some(conn)),
@@ -512,17 +522,22 @@ impl NativeDatabase {
512522
}
513523

514524
/// Open a read-only connection to the database at `db_path`.
525+
///
526+
/// `busy_timeout_ms` mirrors `config.db.busyTimeoutMs` on the TS side
527+
/// (`DEFAULTS.db.busyTimeoutMs` in `src/infrastructure/config.ts`);
528+
/// defaults to `DEFAULT_BUSY_TIMEOUT_MS` when omitted.
515529
#[napi(factory)]
516-
pub fn open_readonly(db_path: String) -> napi::Result<Self> {
530+
pub fn open_readonly(db_path: String, busy_timeout_ms: Option<u32>) -> napi::Result<Self> {
517531
let flags = OpenFlags::SQLITE_OPEN_READ_ONLY | OpenFlags::SQLITE_OPEN_NO_MUTEX;
518532
let conn = Connection::open_with_flags(&db_path, flags)
519533
.map_err(|e| napi::Error::from_reason(format!("Failed to open DB readonly: {e}")))?;
520534
conn.set_prepared_statement_cache_capacity(64);
521-
conn.execute_batch(
522-
"PRAGMA busy_timeout = 5000; \
535+
let busy_timeout_ms = busy_timeout_ms.unwrap_or(DEFAULT_BUSY_TIMEOUT_MS);
536+
conn.execute_batch(&format!(
537+
"PRAGMA busy_timeout = {busy_timeout_ms}; \
523538
PRAGMA mmap_size = 268435456; \
524539
PRAGMA temp_store = MEMORY;",
525-
)
540+
))
526541
.map_err(|e| napi::Error::from_reason(format!("Failed to set pragmas: {e}")))?;
527542
Ok(Self {
528543
conn: SendWrapper::new(Some(conn)),

src/db/connection.ts

Lines changed: 9 additions & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -474,7 +474,10 @@ export function resolveBusyTimeoutMs(customDbPath?: string): number {
474474
}
475475

476476
/** Open a NativeRepository via rusqlite, throwing DbError if the DB file is missing. */
477-
function openRepoNative(customDbPath?: string): { repo: Repository; close(): void } {
477+
function openRepoNative(
478+
customDbPath: string | undefined,
479+
busyTimeoutMs: number,
480+
): { repo: Repository; close(): void } {
478481
const dbPath = findDbPath(customDbPath);
479482
if (!fs.existsSync(dbPath)) {
480483
throw new DbError(
@@ -483,7 +486,7 @@ function openRepoNative(customDbPath?: string): { repo: Repository; close(): voi
483486
);
484487
}
485488
const native = getNative();
486-
const ndb = native.NativeDatabase.openReadonly(dbPath);
489+
const ndb = native.NativeDatabase.openReadonly(dbPath, busyTimeoutMs);
487490
try {
488491
warnOnVersionMismatch(() => ndb.getBuildMeta('codegraph_version'));
489492
const repo = new NativeRepository(ndb, dbPath);
@@ -529,10 +532,11 @@ function wrapInjectedRepo(repo: Repository): { repo: Repository; close(): void }
529532
function tryOpenRepoNative(
530533
customDbPath: string | undefined,
531534
engine: 'native' | 'wasm' | 'auto',
535+
busyTimeoutMs: number,
532536
): { repo: Repository; close(): void } | undefined {
533537
if (engine === 'wasm' || !isNativeAvailable()) return undefined;
534538
try {
535-
return openRepoNative(customDbPath);
539+
return openRepoNative(customDbPath, busyTimeoutMs);
536540
} catch (e) {
537541
// Re-throw user-visible errors (e.g. DB not found) — only silently
538542
// fall back for native-engine failures (e.g. incompatible native binary).
@@ -582,7 +586,7 @@ export function openRepo(
582586
// This ensures --engine wasm and benchmark workers bypass the native path.
583587
const { engine, busyTimeoutMs } = resolveDbSettings(customDbPath, opts.engine);
584588

585-
const native = tryOpenRepoNative(customDbPath, engine);
589+
const native = tryOpenRepoNative(customDbPath, engine, busyTimeoutMs);
586590
if (native) return native;
587591

588592
return openRepoSqliteFallback(customDbPath, busyTimeoutMs);
@@ -622,7 +626,7 @@ export function openReadonlyWithNative(
622626
try {
623627
const dbPath = findDbPath(customPath);
624628
const native = getNative();
625-
nativeDb = native.NativeDatabase.openReadonly(dbPath);
629+
nativeDb = native.NativeDatabase.openReadonly(dbPath, busyTimeoutMs);
626630
} catch (e) {
627631
const msg = toErrorMessage(e);
628632
if (isBusyOrLockedError(msg)) {

src/domain/graph/builder/stages/native-db-lifecycle.ts

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -44,7 +44,7 @@ export function reopenNativeDb(ctx: PipelineContext, label: string): void {
4444
const native = loadNative();
4545
if (!native?.NativeDatabase) return;
4646
try {
47-
ctx.nativeDb = native.NativeDatabase.openReadWrite(ctx.dbPath);
47+
ctx.nativeDb = native.NativeDatabase.openReadWrite(ctx.dbPath, ctx.config.db.busyTimeoutMs);
4848
} catch (e) {
4949
debug(`reopen nativeDb for ${label} failed: ${toErrorMessage(e)}`);
5050
ctx.nativeDb = undefined;

src/domain/graph/builder/stages/native-orchestrator.ts

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -579,7 +579,7 @@ async function runPostNativeAnalysis(
579579
const native = loadNative();
580580
if (native?.NativeDatabase) {
581581
try {
582-
ctx.nativeDb = native.NativeDatabase.openReadWrite(ctx.dbPath);
582+
ctx.nativeDb = native.NativeDatabase.openReadWrite(ctx.dbPath, ctx.config.db.busyTimeoutMs);
583583
if (ctx.engineOpts) ctx.engineOpts.nativeDb = ctx.nativeDb;
584584
} catch {
585585
ctx.nativeDb = undefined;
@@ -1955,7 +1955,7 @@ function openNativeDatabase(ctx: PipelineContext): void {
19551955
// is kept and transferred to the NativeDbProxy below, not released here.
19561956
ctx.db.close();
19571957
acquireAdvisoryLock(ctx.dbPath);
1958-
ctx.nativeDb = native.NativeDatabase.openReadWrite(ctx.dbPath);
1958+
ctx.nativeDb = native.NativeDatabase.openReadWrite(ctx.dbPath, ctx.config.db.busyTimeoutMs);
19591959
ctx.nativeDb.initSchema();
19601960
// Replace ctx.db with a NativeDbProxy so post-native JS fallback
19611961
// (structure, analysis) can use it without reopening better-sqlite3.

src/features/branch-compare.ts

Lines changed: 2 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -3,6 +3,7 @@ import fs from 'node:fs';
33
import os from 'node:os';
44
import path from 'node:path';
55
import { getDatabase } from '../db/better-sqlite3.js';
6+
import { resolveBusyTimeoutMs } from '../db/index.js';
67
import { buildGraph } from '../domain/graph/builder.js';
78
import { kindIcon } from '../domain/queries.js';
89
import { debug } from '../infrastructure/logger.js';
@@ -120,7 +121,7 @@ function openNativeDbForFanMetrics(dbPath: string): NativeDatabase | undefined {
120121
if (!isNativeAvailable()) return undefined;
121122
try {
122123
const native = getNative();
123-
return native.NativeDatabase.openReadonly(dbPath);
124+
return native.NativeDatabase.openReadonly(dbPath, resolveBusyTimeoutMs(dbPath));
124125
} catch (e) {
125126
debug(`loadSymbolsFromDb: native path failed: ${toErrorMessage(e)}`);
126127
return undefined;

src/types.ts

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -2390,8 +2390,8 @@ export interface NativeAddon {
23902390
extractDataflowAnalysisBatch?(filePaths: string[]): (DataflowResult | null)[];
23912391
ParseTreeCache: new () => NativeParseTreeCache;
23922392
NativeDatabase: {
2393-
openReadWrite(dbPath: string): NativeDatabase;
2394-
openReadonly(dbPath: string): NativeDatabase;
2393+
openReadWrite(dbPath: string, busyTimeoutMs?: number): NativeDatabase;
2394+
openReadonly(dbPath: string, busyTimeoutMs?: number): NativeDatabase;
23952395
};
23962396
}
23972397

Lines changed: 121 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,121 @@
1+
/**
2+
* Regression tests for issue #1882: the JS call sites that open a
3+
* `NativeDatabase` must pass the resolved `config.db.busyTimeoutMs` through
4+
* as the factory's `busyTimeoutMs` argument, instead of silently relying on
5+
* the Rust-side hardcoded default.
6+
*
7+
* Mocks `infrastructure/native.js` (the same pattern as
8+
* `tests/unit/openRepo-busy.test.ts`) so the assertions are about *what
9+
* argument each call site passes*, independent of whether a native addon is
10+
* actually built for the current platform.
11+
*/
12+
import fs from 'node:fs';
13+
import os from 'node:os';
14+
import path from 'node:path';
15+
import { afterAll, beforeAll, describe, expect, it, vi } from 'vitest';
16+
17+
const CUSTOM_BUSY_TIMEOUT_MS = 424242;
18+
19+
const openReadonlyCalls: Array<[string, number | undefined]> = [];
20+
const openReadWriteCalls: Array<[string, number | undefined]> = [];
21+
22+
function makeFakeNativeDb() {
23+
return {
24+
getBuildMeta: () => null,
25+
close: () => {},
26+
initSchema: () => {},
27+
exec: () => {},
28+
};
29+
}
30+
31+
vi.mock('../../src/infrastructure/native.js', () => ({
32+
isNativeAvailable: () => true,
33+
getNative: () => ({
34+
NativeDatabase: {
35+
openReadonly: (dbPath: string, busyTimeoutMs?: number) => {
36+
openReadonlyCalls.push([dbPath, busyTimeoutMs]);
37+
return makeFakeNativeDb();
38+
},
39+
openReadWrite: (dbPath: string, busyTimeoutMs?: number) => {
40+
openReadWriteCalls.push([dbPath, busyTimeoutMs]);
41+
return makeFakeNativeDb();
42+
},
43+
},
44+
}),
45+
loadNative: () => ({
46+
NativeDatabase: {
47+
openReadonly: (dbPath: string, busyTimeoutMs?: number) => {
48+
openReadonlyCalls.push([dbPath, busyTimeoutMs]);
49+
return makeFakeNativeDb();
50+
},
51+
openReadWrite: (dbPath: string, busyTimeoutMs?: number) => {
52+
openReadWriteCalls.push([dbPath, busyTimeoutMs]);
53+
return makeFakeNativeDb();
54+
},
55+
},
56+
}),
57+
}));
58+
59+
import {
60+
closeDb,
61+
initSchema,
62+
openDb,
63+
openReadonlyWithNative,
64+
openRepo,
65+
} from '../../src/db/index.js';
66+
import { PipelineContext } from '../../src/domain/graph/builder/context.js';
67+
import { reopenNativeDb } from '../../src/domain/graph/builder/stages/native-db-lifecycle.js';
68+
69+
let tmpDir: string;
70+
let dbPath: string;
71+
72+
beforeAll(() => {
73+
tmpDir = fs.mkdtempSync(path.join(os.tmpdir(), 'codegraph-native-busy-threading-'));
74+
dbPath = path.join(tmpDir, '.codegraph', 'graph.db');
75+
const db = openDb(dbPath);
76+
initSchema(db);
77+
closeDb(db);
78+
fs.writeFileSync(
79+
path.join(tmpDir, '.codegraphrc.json'),
80+
JSON.stringify({ db: { busyTimeoutMs: CUSTOM_BUSY_TIMEOUT_MS } }),
81+
);
82+
});
83+
84+
afterAll(() => {
85+
if (tmpDir) fs.rmSync(tmpDir, { recursive: true, force: true });
86+
});
87+
88+
describe('openRepo threads busyTimeoutMs into NativeDatabase.openReadonly', () => {
89+
it('passes the configured busyTimeoutMs to the native factory', () => {
90+
openReadonlyCalls.length = 0;
91+
const { close } = openRepo(dbPath);
92+
close();
93+
expect(openReadonlyCalls).toHaveLength(1);
94+
expect(openReadonlyCalls[0]?.[1]).toBe(CUSTOM_BUSY_TIMEOUT_MS);
95+
});
96+
});
97+
98+
describe('openReadonlyWithNative threads busyTimeoutMs into NativeDatabase.openReadonly', () => {
99+
it('passes the configured busyTimeoutMs to the native factory', () => {
100+
openReadonlyCalls.length = 0;
101+
const { close } = openReadonlyWithNative(dbPath);
102+
close();
103+
expect(openReadonlyCalls).toHaveLength(1);
104+
expect(openReadonlyCalls[0]?.[1]).toBe(CUSTOM_BUSY_TIMEOUT_MS);
105+
});
106+
});
107+
108+
describe('reopenNativeDb (build pipeline) threads ctx.config.db.busyTimeoutMs into NativeDatabase.openReadWrite', () => {
109+
it('passes ctx.config.db.busyTimeoutMs to the native factory', () => {
110+
openReadWriteCalls.length = 0;
111+
const ctx = new PipelineContext();
112+
ctx.dbPath = dbPath;
113+
ctx.opts = { engine: 'native' };
114+
ctx.config = { db: { busyTimeoutMs: CUSTOM_BUSY_TIMEOUT_MS } } as PipelineContext['config'];
115+
116+
reopenNativeDb(ctx, 'test');
117+
118+
expect(openReadWriteCalls).toHaveLength(1);
119+
expect(openReadWriteCalls[0]?.[1]).toBe(CUSTOM_BUSY_TIMEOUT_MS);
120+
});
121+
});
Lines changed: 74 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,74 @@
1+
/**
2+
* Regression tests for issue #1882: `config.db.busyTimeoutMs` must reach the
3+
* Rust native DB layer (`NativeDatabase::open_readonly` / `open_read_write`),
4+
* not just the TS-side better-sqlite3 pragma threaded by #1763.
5+
*
6+
* Verifies the applied `busy_timeout` via `queryGet('PRAGMA busy_timeout', [])`
7+
* rather than the `pragma()` helper — `pragma()` only supports TEXT-affinity
8+
* results (see #2019) and throws for INTEGER-returning pragmas like
9+
* `busy_timeout`.
10+
*/
11+
import fs from 'node:fs';
12+
import os from 'node:os';
13+
import path from 'node:path';
14+
import { afterEach, beforeEach, describe, expect, it } from 'vitest';
15+
import { DEFAULTS } from '../../src/infrastructure/config.js';
16+
import { getNative, isNativeAvailable } from '../../src/infrastructure/native.js';
17+
import type { NativeDatabase } from '../../src/types.js';
18+
19+
const hasNativeDb =
20+
isNativeAvailable() && typeof getNative().NativeDatabase?.openReadWrite === 'function';
21+
22+
/** Read the effective `busy_timeout` pragma value via queryGet (avoids the pragma() TEXT-only bug, #2019). */
23+
function readBusyTimeout(ndb: NativeDatabase): number {
24+
const row = ndb.queryGet('PRAGMA busy_timeout', []) as { timeout: number } | null;
25+
return row?.timeout as number;
26+
}
27+
28+
describe.skipIf(!hasNativeDb)('NativeDatabase busy_timeout_ms threading (Rust layer)', () => {
29+
let tmpDir: string;
30+
let dbPath: string;
31+
let nativeDb: NativeDatabase | undefined;
32+
33+
beforeEach(() => {
34+
tmpDir = fs.mkdtempSync(path.join(os.tmpdir(), 'cg-native-busy-'));
35+
dbPath = path.join(tmpDir, 'test.db');
36+
});
37+
38+
afterEach(() => {
39+
try {
40+
nativeDb?.close();
41+
} catch {
42+
/* already closed */
43+
}
44+
nativeDb = undefined;
45+
fs.rmSync(tmpDir, { recursive: true, force: true });
46+
});
47+
48+
it('openReadWrite applies a configured busyTimeoutMs', () => {
49+
const NativeDB = getNative().NativeDatabase;
50+
nativeDb = NativeDB.openReadWrite(dbPath, 424242);
51+
expect(readBusyTimeout(nativeDb)).toBe(424242);
52+
});
53+
54+
it('openReadWrite defaults to DEFAULTS.db.busyTimeoutMs when omitted', () => {
55+
const NativeDB = getNative().NativeDatabase;
56+
nativeDb = NativeDB.openReadWrite(dbPath);
57+
expect(readBusyTimeout(nativeDb)).toBe(DEFAULTS.db.busyTimeoutMs);
58+
});
59+
60+
it('openReadonly applies a configured busyTimeoutMs', () => {
61+
const NativeDB = getNative().NativeDatabase;
62+
// openReadonly requires the file to already exist.
63+
NativeDB.openReadWrite(dbPath).close();
64+
nativeDb = NativeDB.openReadonly(dbPath, 99999);
65+
expect(readBusyTimeout(nativeDb)).toBe(99999);
66+
});
67+
68+
it('openReadonly defaults to DEFAULTS.db.busyTimeoutMs when omitted', () => {
69+
const NativeDB = getNative().NativeDatabase;
70+
NativeDB.openReadWrite(dbPath).close();
71+
nativeDb = NativeDB.openReadonly(dbPath);
72+
expect(readBusyTimeout(nativeDb)).toBe(DEFAULTS.db.busyTimeoutMs);
73+
});
74+
});

0 commit comments

Comments
 (0)