Skip to content

Commit f31065c

Browse files
authored
fix(agentdb): cache prepared statements to plug sql.js leak (ruvnet#144)
Both sql.js wrappers in db-fallback.ts (SqlJsDatabase and wrapExistingSqlJsDatabase) tracked every prepare() call in activeStatements and only removed entries on finalize() or error. The idiomatic better-sqlite3 pattern db.prepare(sql).run(...) — which relies on V8 to GC the handle — leaks unbounded under sql.js because WASM-backed Statement objects need explicit .free(). Production observation (Cloud Run mcp-bridge): ~3 statements/s growth, hit 621 within minutes: [ruflo] ⚠️ Detected 562 active SQL statements - possible memory leak [ruflo] ⚠️ Detected 612 active SQL statements - possible memory leak [ruflo] ⚠️ Detected 621 active SQL statements - possible memory leak Fix: cache wrappers by SQL text. Repeated prepare(sameSql) returns the same wrapper; activeStatements.size is now bounded by the count of distinct SQL strings. Safe under single-threaded JS — bind/step/reset are synchronous so no two run/get/all calls can interleave on a shared stmt. finalize() is now a no-op on cached wrappers (close() owns teardown); auto-free-on-error also evicts from cache so a broken stmt isn't reused. Smoke test (5000 prepare(INSERT).run + 1 prepare(SELECT).get): activeStatements_size: 2 (was 5001 pre-patch) statementCache_size: 2 post_close_active: 0 post_close_cache: 0 Typecheck: clean. Test failures observed in repo are pre-existing (jest stubs in vitest project, hnsw zero-vector edge case) — unrelated to this change.
1 parent 8db2712 commit f31065c

1 file changed

Lines changed: 59 additions & 29 deletions

File tree

packages/agentdb/src/db-fallback.ts

Lines changed: 59 additions & 29 deletions
Original file line numberDiff line numberDiff line change
@@ -59,6 +59,13 @@ function createSqlJsWrapper(SQL: any) {
5959
private filename: string;
6060
// eslint-disable-next-line @typescript-eslint/no-explicit-any -- sql.js Statement instances
6161
private activeStatements: Map<number, any> = new Map();
62+
// Cache wrappers by SQL text so repeated db.prepare(sameSql) returns the
63+
// same underlying sql.js Statement and doesn't add a new entry to
64+
// activeStatements. Without this, callers using the
65+
// db.prepare(sql).run(...)-and-discard pattern (idiomatic in
66+
// better-sqlite3, where V8 GCs the handle) leak forever under sql.js.
67+
// eslint-disable-next-line @typescript-eslint/no-explicit-any -- prepared-statement wrapper objects
68+
private statementCache: Map<string, any> = new Map();
6269
private statementCounter: number = 0;
6370
private intervalId: NodeJS.Timeout | null = null;
6471

@@ -92,14 +99,31 @@ function createSqlJsWrapper(SQL: any) {
9299
}
93100

94101
prepare(sql: string) {
102+
// Reuse the cached wrapper for an identical SQL string. The cache owns
103+
// the underlying stmt's lifetime; finalize() is a no-op on a cached
104+
// wrapper so concurrent callers that all called prepare(sql) keep
105+
// working. close() drops everything.
106+
const cached = this.statementCache.get(sql);
107+
if (cached) return cached;
108+
95109
const stmt = this.db.prepare(sql);
96110
let isFinalized = false;
97111
const stmtId = ++this.statementCounter;
112+
const self = this;
98113

99114
// Track active statement
100115
this.activeStatements.set(stmtId, stmt);
101116

102-
return {
117+
const evictOnError = () => {
118+
if (!isFinalized) {
119+
stmt.free();
120+
isFinalized = true;
121+
self.activeStatements.delete(stmtId);
122+
self.statementCache.delete(sql);
123+
}
124+
};
125+
126+
const wrapper = {
103127
// eslint-disable-next-line @typescript-eslint/no-explicit-any -- sql.js bind accepts heterogeneous params
104128
run: (...params: any[]) => {
105129
if (isFinalized) throw new Error('Statement already finalized');
@@ -113,12 +137,7 @@ function createSqlJsWrapper(SQL: any) {
113137
lastInsertRowid: this.db.exec('SELECT last_insert_rowid()')[0]?.values[0]?.[0] || 0
114138
};
115139
} catch (error) {
116-
// Auto-free on error to prevent memory leak
117-
if (!isFinalized) {
118-
stmt.free();
119-
isFinalized = true;
120-
this.activeStatements.delete(stmtId);
121-
}
140+
evictOnError();
122141
throw error;
123142
}
124143
},
@@ -146,12 +165,7 @@ function createSqlJsWrapper(SQL: any) {
146165

147166
return result;
148167
} catch (error) {
149-
// Auto-free on error to prevent memory leak
150-
if (!isFinalized) {
151-
stmt.free();
152-
isFinalized = true;
153-
this.activeStatements.delete(stmtId);
154-
}
168+
evictOnError();
155169
throw error;
156170
}
157171
},
@@ -178,24 +192,20 @@ function createSqlJsWrapper(SQL: any) {
178192
stmt.reset();
179193
return results;
180194
} catch (error) {
181-
// Auto-free on error to prevent memory leak
182-
if (!isFinalized) {
183-
stmt.free();
184-
isFinalized = true;
185-
this.activeStatements.delete(stmtId);
186-
}
195+
evictOnError();
187196
throw error;
188197
}
189198
},
190199

191200
finalize: () => {
192-
if (!isFinalized) {
193-
stmt.free();
194-
isFinalized = true;
195-
this.activeStatements.delete(stmtId);
196-
}
201+
// No-op while cached: the wrapper is shared and finalize() must not
202+
// tear out a stmt another caller is about to use. close() handles
203+
// the real teardown.
197204
}
198205
};
206+
207+
this.statementCache.set(sql, wrapper);
208+
return wrapper;
199209
}
200210

201211
exec(sql: string) {
@@ -237,6 +247,7 @@ function createSqlJsWrapper(SQL: any) {
237247
}
238248
}
239249
this.activeStatements.clear();
250+
this.statementCache.clear();
240251

241252
// Save to file before closing
242253
this.save();
@@ -299,16 +310,31 @@ export async function createDatabase(filename: string, options?: unknown): Promi
299310
export function wrapExistingSqlJsDatabase(rawDb: any, filename: string = ':memory:'): any {
300311
// eslint-disable-next-line @typescript-eslint/no-explicit-any -- sql.js Statement instances
301312
const activeStatements = new Map<number, any>();
313+
// SQL-text cache; see SqlJsDatabase.statementCache for rationale.
314+
// eslint-disable-next-line @typescript-eslint/no-explicit-any -- prepared-statement wrapper objects
315+
const statementCache = new Map<string, any>();
302316
let statementCounter = 0;
303317

304318
return {
305319
prepare(sql: string) {
320+
const cached = statementCache.get(sql);
321+
if (cached) return cached;
322+
306323
const stmt = rawDb.prepare(sql);
307324
let isFinalized = false;
308325
const stmtId = ++statementCounter;
309326
activeStatements.set(stmtId, stmt);
310327

311-
return {
328+
const evictOnError = () => {
329+
if (!isFinalized) {
330+
stmt.free();
331+
isFinalized = true;
332+
activeStatements.delete(stmtId);
333+
statementCache.delete(sql);
334+
}
335+
};
336+
337+
const wrapper = {
312338
// eslint-disable-next-line @typescript-eslint/no-explicit-any -- sql.js bind accepts heterogeneous params
313339
run(...params: any[]) {
314340
if (isFinalized) throw new Error('Statement already finalized');
@@ -321,7 +347,7 @@ export function wrapExistingSqlJsDatabase(rawDb: any, filename: string = ':memor
321347
lastInsertRowid: rawDb.exec('SELECT last_insert_rowid()')[0]?.values[0]?.[0] || 0,
322348
};
323349
} catch (error) {
324-
if (!isFinalized) { stmt.free(); isFinalized = true; activeStatements.delete(stmtId); }
350+
evictOnError();
325351
throw error;
326352
}
327353
},
@@ -339,7 +365,7 @@ export function wrapExistingSqlJsDatabase(rawDb: any, filename: string = ':memor
339365
columns.forEach((col: string, idx: number) => { result[col] = values[idx]; });
340366
return result;
341367
} catch (error) {
342-
if (!isFinalized) { stmt.free(); isFinalized = true; activeStatements.delete(stmtId); }
368+
evictOnError();
343369
throw error;
344370
}
345371
},
@@ -359,14 +385,17 @@ export function wrapExistingSqlJsDatabase(rawDb: any, filename: string = ':memor
359385
stmt.reset();
360386
return results;
361387
} catch (error) {
362-
if (!isFinalized) { stmt.free(); isFinalized = true; activeStatements.delete(stmtId); }
388+
evictOnError();
363389
throw error;
364390
}
365391
},
366392
finalize() {
367-
if (!isFinalized) { stmt.free(); isFinalized = true; activeStatements.delete(stmtId); }
393+
// No-op while cached; close() handles teardown.
368394
},
369395
};
396+
397+
statementCache.set(sql, wrapper);
398+
return wrapper;
370399
},
371400

372401
exec(sql: string) {
@@ -387,6 +416,7 @@ export function wrapExistingSqlJsDatabase(rawDb: any, filename: string = ':memor
387416
try { stmt.free(); } catch { /* already freed */ }
388417
}
389418
activeStatements.clear();
419+
statementCache.clear();
390420
this.save();
391421
rawDb.close();
392422
},

0 commit comments

Comments
 (0)