mirror of
https://github.com/diegosouzapw/OmniRoute.git
synced 2026-09-19 21:32:20 +03:00
fix(db): close failed initialization connections (#13342)
* fix(db): close failed initialization connections * docs: add changelog fragment for #13303 db handle-leak fix --------- Co-authored-by: voidstackloop <voidstackloop@users.noreply.github.com> Co-authored-by: diegosouzapw <8016841+diegosouzapw@users.noreply.github.com>
This commit is contained in:
@@ -1088,8 +1088,9 @@ export function getDbInstance(): SqliteDatabase {
|
||||
// Detect and handle old schema format — preserve data when possible (#146)
|
||||
// Uses a single probe connection that becomes the real connection when possible.
|
||||
if (fs.existsSync(sqliteFile)) {
|
||||
let probe: SqliteDatabase | null = null;
|
||||
try {
|
||||
const probe = openSqliteDatabase(sqliteFile, { readonly: true });
|
||||
probe = openSqliteDatabase(sqliteFile, { readonly: true });
|
||||
// #9934: init asymmetry — bin/cli/sqlite.mjs::openOmniRouteDb (used by
|
||||
// `omniroute setup`) creates storage.sqlite with only the partial inline
|
||||
// schema (key_value + provider_connections) and never runs migrations.
|
||||
@@ -1156,6 +1157,12 @@ export function getDbInstance(): SqliteDatabase {
|
||||
closeProbeIfSafe(probe);
|
||||
}
|
||||
} catch (e: unknown) {
|
||||
try {
|
||||
closeProbeIfSafe(probe);
|
||||
probe = null;
|
||||
} catch {
|
||||
/* ignore */
|
||||
}
|
||||
const message = e instanceof Error ? e.message : String(e);
|
||||
console.warn("[DB] Could not probe existing DB:", message);
|
||||
|
||||
@@ -1214,6 +1221,12 @@ export function getDbInstance(): SqliteDatabase {
|
||||
/* ok */
|
||||
}
|
||||
}
|
||||
} finally {
|
||||
try {
|
||||
closeProbeIfSafe(probe);
|
||||
} catch {
|
||||
/* ignore */
|
||||
}
|
||||
}
|
||||
}
|
||||
|
||||
@@ -1234,40 +1247,41 @@ export function getDbInstance(): SqliteDatabase {
|
||||
}
|
||||
|
||||
const db = openSqliteDatabase(sqliteFile);
|
||||
// Emit the same "[DB] Driver: ..." line openDatabaseAsync() prints so the
|
||||
// packaged-app smoke guard (#7592) can assert the native driver was
|
||||
// selected on the server's primary DB path too, not only the backup-import
|
||||
// route.
|
||||
console.log(`[DB] Driver: ${db.driver} | file: ${sqliteFile}`);
|
||||
// better-sqlite3 is synchronous, so a contended write parks the Node event loop for up to
|
||||
// busy_timeout ms (a 0-CPU freeze that stacks under load → /health stops responding). The
|
||||
// hot-path writers here (usage_history, call_logs) are best-effort and the WinUI host opens
|
||||
// the same DB, so cap the block at 2s instead of 5s: normal writes complete in <1ms, and a
|
||||
// contended op can no longer freeze the loop past the host watchdog's 6s liveness probe.
|
||||
//
|
||||
// Install the busy handler before the connection's first statement. `journal_mode = WAL`
|
||||
// needs a SHARED lock, and another process closing its WAL connection briefly holds the
|
||||
// file EXCLUSIVE (checkpoint + WAL delete); node:sqlite opens with busy timeout 0, so with
|
||||
// the pragmas in the other order that window surfaced as `database is locked` at startup.
|
||||
db.pragma("busy_timeout = 2000");
|
||||
db.pragma("journal_mode = WAL");
|
||||
db.pragma("synchronous = NORMAL");
|
||||
db.pragma(`cache_size = -${DEFAULT_DATABASE_SETTINGS.optimization.cacheSize}`);
|
||||
db.pragma("temp_store = MEMORY");
|
||||
// Tables first, then the legacy-column healers, and only then the indexes: an upgraded
|
||||
// database can already own call_logs/usage_history/provider_connections with an older
|
||||
// column set, where the CREATE TABLE is a no-op but the indexes still reference columns
|
||||
// the healers below are the ones adding.
|
||||
db.exec(SCHEMA_TABLES_SQL);
|
||||
ensureProviderConnectionsColumns(db);
|
||||
ensureUsageHistoryColumns(db);
|
||||
ensureCallLogsColumns(db);
|
||||
db.exec(SCHEMA_INDEXES_SQL);
|
||||
try {
|
||||
// Emit the same "[DB] Driver: ..." line openDatabaseAsync() prints so the
|
||||
// packaged-app smoke guard (#7592) can assert the native driver was
|
||||
// selected on the server's primary DB path too, not only the backup-import
|
||||
// route.
|
||||
console.log(`[DB] Driver: ${db.driver} | file: ${sqliteFile}`);
|
||||
// better-sqlite3 is synchronous, so a contended write parks the Node event loop for up to
|
||||
// busy_timeout ms (a 0-CPU freeze that stacks under load → /health stops responding). The
|
||||
// hot-path writers here (usage_history, call_logs) are best-effort and the WinUI host opens
|
||||
// the same DB, so cap the block at 2s instead of 5s: normal writes complete in <1ms, and a
|
||||
// contended op can no longer freeze the loop past the host watchdog's 6s liveness probe.
|
||||
//
|
||||
// Install the busy handler before the connection's first statement. `journal_mode = WAL`
|
||||
// needs a SHARED lock, and another process closing its WAL connection briefly holds the
|
||||
// file EXCLUSIVE (checkpoint + WAL delete); node:sqlite opens with busy timeout 0, so with
|
||||
// the pragmas in the other order that window surfaced as `database is locked` at startup.
|
||||
db.pragma("busy_timeout = 2000");
|
||||
db.pragma("journal_mode = WAL");
|
||||
db.pragma("synchronous = NORMAL");
|
||||
db.pragma(`cache_size = -${DEFAULT_DATABASE_SETTINGS.optimization.cacheSize}`);
|
||||
db.pragma("temp_store = MEMORY");
|
||||
// Tables first, then the legacy-column healers, and only then the indexes: an upgraded
|
||||
// database can already own call_logs/usage_history/provider_connections with an older
|
||||
// column set, where the CREATE TABLE is a no-op but the indexes still reference columns
|
||||
// the healers below are the ones adding.
|
||||
db.exec(SCHEMA_TABLES_SQL);
|
||||
ensureProviderConnectionsColumns(db);
|
||||
ensureUsageHistoryColumns(db);
|
||||
ensureCallLogsColumns(db);
|
||||
db.exec(SCHEMA_INDEXES_SQL);
|
||||
|
||||
// ── Versioned Migrations ──
|
||||
// Auto-seed 001 as applied (the inline SCHEMA_SQL already created these tables)
|
||||
// then run any new migrations (002+)
|
||||
db.exec(`
|
||||
// ── Versioned Migrations ──
|
||||
// Auto-seed 001 as applied (the inline SCHEMA_SQL already created these tables)
|
||||
// then run any new migrations (002+)
|
||||
db.exec(`
|
||||
CREATE TABLE IF NOT EXISTS _omniroute_migrations (
|
||||
version TEXT PRIMARY KEY,
|
||||
name TEXT NOT NULL,
|
||||
@@ -1277,73 +1291,81 @@ export function getDbInstance(): SqliteDatabase {
|
||||
VALUES ('001', 'initial_schema');
|
||||
`);
|
||||
|
||||
runMigrations(db, { isNewDb, databaseExistedBeforeInitialization });
|
||||
// Fresh installs need the same post-migration index guarantee as upgraded
|
||||
// databases, including recovery from an interrupted migration 127 attempt.
|
||||
ensureUsageHistoryAccountIndex(db);
|
||||
runMigrations(db, { isNewDb, databaseExistedBeforeInitialization });
|
||||
// Fresh installs need the same post-migration index guarantee as upgraded
|
||||
// databases, including recovery from an interrupted migration 127 attempt.
|
||||
ensureUsageHistoryAccountIndex(db);
|
||||
|
||||
applyStoredDatabaseOptimizationSettings(db);
|
||||
applyStoredDatabaseOptimizationSettings(db);
|
||||
|
||||
// Apply mmap_size from stored settings (migration 046), fallback to 256MiB
|
||||
try {
|
||||
const mmapRow = db
|
||||
.prepare("SELECT value FROM key_value WHERE namespace = ? AND key = ?")
|
||||
.get("databaseSettings", "mmapSize") as { value: string } | undefined;
|
||||
const mmapSize = mmapRow ? Math.max(0, parseInt(mmapRow.value, 10) || 0) : 268435456;
|
||||
if (mmapSize > 0) {
|
||||
db.pragma(`mmap_size = ${mmapSize}`);
|
||||
}
|
||||
} catch {
|
||||
// mmap_size is best-effort; not available in all runtimes (e.g. web)
|
||||
}
|
||||
|
||||
offloadLegacyCallLogDetails(db);
|
||||
|
||||
// Auto-migrate from db.json if exists
|
||||
if (jsonDbFile && fs.existsSync(jsonDbFile)) {
|
||||
migrateFromJson(db, jsonDbFile);
|
||||
}
|
||||
|
||||
if (failedProbePath && preservedCriticalState.preservedTables.length > 0) {
|
||||
// Apply mmap_size from stored settings (migration 046), fallback to 256MiB
|
||||
try {
|
||||
const restoredTables = restoreCriticalDbState(db, preservedCriticalState);
|
||||
console.log(
|
||||
`[DB] Restored preserved critical DB state after probe failure: ${summarizePreservedTables(
|
||||
restoredTables
|
||||
)}`
|
||||
);
|
||||
} catch (error: unknown) {
|
||||
const message = error instanceof Error ? error.message : String(error);
|
||||
try {
|
||||
closeProbeIfSafe(db);
|
||||
} catch {
|
||||
/* ignore */
|
||||
const mmapRow = db
|
||||
.prepare("SELECT value FROM key_value WHERE namespace = ? AND key = ?")
|
||||
.get("databaseSettings", "mmapSize") as { value: string } | undefined;
|
||||
const mmapSize = mmapRow ? Math.max(0, parseInt(mmapRow.value, 10) || 0) : 268435456;
|
||||
if (mmapSize > 0) {
|
||||
db.pragma(`mmap_size = ${mmapSize}`);
|
||||
}
|
||||
cleanupRecreatedSqliteFiles(sqliteFile);
|
||||
throw new Error(
|
||||
`[DB] Automatic recovery aborted after probe failure. ` +
|
||||
`Preserved database: ${failedProbePath}. ` +
|
||||
`Restore failure: ${message}.`
|
||||
);
|
||||
} catch {
|
||||
// mmap_size is best-effort; not available in all runtimes (e.g. web)
|
||||
}
|
||||
|
||||
offloadLegacyCallLogDetails(db);
|
||||
|
||||
// Auto-migrate from db.json if exists
|
||||
if (jsonDbFile && fs.existsSync(jsonDbFile)) {
|
||||
migrateFromJson(db, jsonDbFile);
|
||||
}
|
||||
|
||||
if (failedProbePath && preservedCriticalState.preservedTables.length > 0) {
|
||||
try {
|
||||
const restoredTables = restoreCriticalDbState(db, preservedCriticalState);
|
||||
console.log(
|
||||
`[DB] Restored preserved critical DB state after probe failure: ${summarizePreservedTables(
|
||||
restoredTables
|
||||
)}`
|
||||
);
|
||||
} catch (error: unknown) {
|
||||
const message = error instanceof Error ? error.message : String(error);
|
||||
try {
|
||||
closeProbeIfSafe(db);
|
||||
} catch {
|
||||
/* ignore */
|
||||
}
|
||||
cleanupRecreatedSqliteFiles(sqliteFile);
|
||||
throw new Error(
|
||||
`[DB] Automatic recovery aborted after probe failure. ` +
|
||||
`Preserved database: ${failedProbePath}. ` +
|
||||
`Restore failure: ${message}.`
|
||||
);
|
||||
}
|
||||
}
|
||||
|
||||
// Store schema version
|
||||
const versionStmt = db.prepare(
|
||||
"INSERT OR REPLACE INTO db_meta (key, value) VALUES ('schema_version', '1')"
|
||||
);
|
||||
versionStmt.run();
|
||||
|
||||
// Register the singleton BEFORE the health-check gate below: the flag read
|
||||
// (isDbHealthcheckStartupDeferredEnabled, like any DB-backed feature-flag
|
||||
// override) itself calls getDbInstance(), and with the singleton still
|
||||
// unset at this point that call would see no existing instance and open a
|
||||
// second, independent connection — which hits the very same unset-singleton
|
||||
// gate on its own way through, recursing without end (each recursion opens
|
||||
// its own real DB connection and re-runs migrations-check, so this is a
|
||||
// real resource-exhaustion loop, not just a deep call stack). #13717 rework.
|
||||
setDb(db);
|
||||
} catch (error) {
|
||||
try {
|
||||
closeProbeIfSafe(db);
|
||||
} catch {
|
||||
/* ignore */
|
||||
}
|
||||
throw error;
|
||||
}
|
||||
|
||||
// Store schema version
|
||||
const versionStmt = db.prepare(
|
||||
"INSERT OR REPLACE INTO db_meta (key, value) VALUES ('schema_version', '1')"
|
||||
);
|
||||
versionStmt.run();
|
||||
|
||||
// Register the singleton BEFORE the health-check gate below: the flag read
|
||||
// (isDbHealthcheckStartupDeferredEnabled, like any DB-backed feature-flag
|
||||
// override) itself calls getDbInstance(), and with the singleton still
|
||||
// unset at this point that call would see no existing instance and open a
|
||||
// second, independent connection — which hits the very same unset-singleton
|
||||
// gate on its own way through, recursing without end (each recursion opens
|
||||
// its own real DB connection and re-runs migrations-check, so this is a
|
||||
// real resource-exhaustion loop, not just a deep call stack). #13717 rework.
|
||||
setDb(db);
|
||||
|
||||
if (shouldRunStartupDbHealthCheck()) {
|
||||
if (isDbHealthcheckStartupDeferredEnabled()) {
|
||||
// Opt-in (#13717): defer the check past startup via the bounded/paged,
|
||||
|
||||
Reference in New Issue
Block a user