From 544bda8ed8447bb2552e2ea2eab8c42bbcff700c Mon Sep 17 00:00:00 2001 From: ruv Date: Sat, 2 May 2026 00:23:59 -0400 Subject: [PATCH] fix(agentdb): cache prepared statements to plug sql.js leak MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit 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. Co-Authored-By: claude-flow --- packages/agentdb/src/db-fallback.ts | 88 +++++++++++++++++++---------- 1 file changed, 59 insertions(+), 29 deletions(-) diff --git a/packages/agentdb/src/db-fallback.ts b/packages/agentdb/src/db-fallback.ts index bd94611a6..b38129b54 100644 --- a/packages/agentdb/src/db-fallback.ts +++ b/packages/agentdb/src/db-fallback.ts @@ -59,6 +59,13 @@ function createSqlJsWrapper(SQL: any) { private filename: string; // eslint-disable-next-line @typescript-eslint/no-explicit-any -- sql.js Statement instances private activeStatements: Map = new Map(); + // Cache wrappers by SQL text so repeated db.prepare(sameSql) returns the + // same underlying sql.js Statement and doesn't add a new entry to + // activeStatements. Without this, callers using the + // db.prepare(sql).run(...)-and-discard pattern (idiomatic in + // better-sqlite3, where V8 GCs the handle) leak forever under sql.js. + // eslint-disable-next-line @typescript-eslint/no-explicit-any -- prepared-statement wrapper objects + private statementCache: Map = new Map(); private statementCounter: number = 0; private intervalId: NodeJS.Timeout | null = null; @@ -92,14 +99,31 @@ function createSqlJsWrapper(SQL: any) { } prepare(sql: string) { + // Reuse the cached wrapper for an identical SQL string. The cache owns + // the underlying stmt's lifetime; finalize() is a no-op on a cached + // wrapper so concurrent callers that all called prepare(sql) keep + // working. close() drops everything. + const cached = this.statementCache.get(sql); + if (cached) return cached; + const stmt = this.db.prepare(sql); let isFinalized = false; const stmtId = ++this.statementCounter; + const self = this; // Track active statement this.activeStatements.set(stmtId, stmt); - return { + const evictOnError = () => { + if (!isFinalized) { + stmt.free(); + isFinalized = true; + self.activeStatements.delete(stmtId); + self.statementCache.delete(sql); + } + }; + + const wrapper = { // eslint-disable-next-line @typescript-eslint/no-explicit-any -- sql.js bind accepts heterogeneous params run: (...params: any[]) => { if (isFinalized) throw new Error('Statement already finalized'); @@ -113,12 +137,7 @@ function createSqlJsWrapper(SQL: any) { lastInsertRowid: this.db.exec('SELECT last_insert_rowid()')[0]?.values[0]?.[0] || 0 }; } catch (error) { - // Auto-free on error to prevent memory leak - if (!isFinalized) { - stmt.free(); - isFinalized = true; - this.activeStatements.delete(stmtId); - } + evictOnError(); throw error; } }, @@ -146,12 +165,7 @@ function createSqlJsWrapper(SQL: any) { return result; } catch (error) { - // Auto-free on error to prevent memory leak - if (!isFinalized) { - stmt.free(); - isFinalized = true; - this.activeStatements.delete(stmtId); - } + evictOnError(); throw error; } }, @@ -178,24 +192,20 @@ function createSqlJsWrapper(SQL: any) { stmt.reset(); return results; } catch (error) { - // Auto-free on error to prevent memory leak - if (!isFinalized) { - stmt.free(); - isFinalized = true; - this.activeStatements.delete(stmtId); - } + evictOnError(); throw error; } }, finalize: () => { - if (!isFinalized) { - stmt.free(); - isFinalized = true; - this.activeStatements.delete(stmtId); - } + // No-op while cached: the wrapper is shared and finalize() must not + // tear out a stmt another caller is about to use. close() handles + // the real teardown. } }; + + this.statementCache.set(sql, wrapper); + return wrapper; } exec(sql: string) { @@ -237,6 +247,7 @@ function createSqlJsWrapper(SQL: any) { } } this.activeStatements.clear(); + this.statementCache.clear(); // Save to file before closing this.save(); @@ -299,16 +310,31 @@ export async function createDatabase(filename: string, options?: unknown): Promi export function wrapExistingSqlJsDatabase(rawDb: any, filename: string = ':memory:'): any { // eslint-disable-next-line @typescript-eslint/no-explicit-any -- sql.js Statement instances const activeStatements = new Map(); + // SQL-text cache; see SqlJsDatabase.statementCache for rationale. + // eslint-disable-next-line @typescript-eslint/no-explicit-any -- prepared-statement wrapper objects + const statementCache = new Map(); let statementCounter = 0; return { prepare(sql: string) { + const cached = statementCache.get(sql); + if (cached) return cached; + const stmt = rawDb.prepare(sql); let isFinalized = false; const stmtId = ++statementCounter; activeStatements.set(stmtId, stmt); - return { + const evictOnError = () => { + if (!isFinalized) { + stmt.free(); + isFinalized = true; + activeStatements.delete(stmtId); + statementCache.delete(sql); + } + }; + + const wrapper = { // eslint-disable-next-line @typescript-eslint/no-explicit-any -- sql.js bind accepts heterogeneous params run(...params: any[]) { if (isFinalized) throw new Error('Statement already finalized'); @@ -321,7 +347,7 @@ export function wrapExistingSqlJsDatabase(rawDb: any, filename: string = ':memor lastInsertRowid: rawDb.exec('SELECT last_insert_rowid()')[0]?.values[0]?.[0] || 0, }; } catch (error) { - if (!isFinalized) { stmt.free(); isFinalized = true; activeStatements.delete(stmtId); } + evictOnError(); throw error; } }, @@ -339,7 +365,7 @@ export function wrapExistingSqlJsDatabase(rawDb: any, filename: string = ':memor columns.forEach((col: string, idx: number) => { result[col] = values[idx]; }); return result; } catch (error) { - if (!isFinalized) { stmt.free(); isFinalized = true; activeStatements.delete(stmtId); } + evictOnError(); throw error; } }, @@ -359,14 +385,17 @@ export function wrapExistingSqlJsDatabase(rawDb: any, filename: string = ':memor stmt.reset(); return results; } catch (error) { - if (!isFinalized) { stmt.free(); isFinalized = true; activeStatements.delete(stmtId); } + evictOnError(); throw error; } }, finalize() { - if (!isFinalized) { stmt.free(); isFinalized = true; activeStatements.delete(stmtId); } + // No-op while cached; close() handles teardown. }, }; + + statementCache.set(sql, wrapper); + return wrapper; }, exec(sql: string) { @@ -387,6 +416,7 @@ export function wrapExistingSqlJsDatabase(rawDb: any, filename: string = ':memor try { stmt.free(); } catch { /* already freed */ } } activeStatements.clear(); + statementCache.clear(); this.save(); rawDb.close(); },