fix(sqlite): close databases after failed initialization

This commit is contained in:
Vincent Koc
2026-06-16 02:11:11 +02:00
parent d5c9e7ea99
commit ea346f4361
9 changed files with 176 additions and 77 deletions

View File

@@ -19,6 +19,17 @@ async function expectPathMissing(targetPath: string): Promise<void> {
await expect(fs.access(targetPath)).rejects.toThrow("ENOENT");
}
function listOpenFileDescriptorsForPath(targetPath: string): string[] {
return fsSync.readdirSync("/proc/self/fd").flatMap((fd) => {
try {
const descriptorPath = fsSync.readlinkSync(`/proc/self/fd/${fd}`);
return descriptorPath.startsWith(targetPath) ? [descriptorPath] : [];
} catch {
return [];
}
});
}
describe("openMemoryDatabaseAtPath readOnly probe", () => {
let fixtureRoot = "";
let caseId = 0;
@@ -59,6 +70,19 @@ describe("openMemoryDatabaseAtPath readOnly probe", () => {
expect(stat.size).toBeGreaterThan(0);
});
it.skipIf(process.platform !== "linux")(
"closes the database when SQLite maintenance configuration fails",
async () => {
const dbPath = path.join(fixtureRoot, `case-${caseId++}`, "malformed-index.sqlite");
await fs.mkdir(path.dirname(dbPath), { recursive: true });
await fs.writeFile(dbPath, "not a sqlite database");
expect(() => openMemoryDatabaseAtPath(dbPath, false, false)).toThrow(/not a database/);
expect(listOpenFileDescriptorsForPath(dbPath)).toEqual([]);
},
);
it("refuses to create a missing live database while a safe reindex holds the lock", async () => {
const dbPath = path.join(fixtureRoot, `case-${caseId++}`, "index.sqlite");
await fs.mkdir(path.dirname(dbPath), { recursive: true });

View File

@@ -127,11 +127,18 @@ export function cleanupAgedMemoryReindexTempFiles(dbPath: string, nowMs = Date.n
function openConfiguredMemoryDatabaseAtPath(dbPath: string, allowExtension: boolean): DatabaseSync {
const { DatabaseSync } = requireNodeSqlite();
const db = new DatabaseSync(dbPath, { allowExtension });
configureMemorySqliteWalMaintenance(db, {
busyTimeoutMs: 5000,
databasePath: dbPath,
});
return db;
try {
configureMemorySqliteWalMaintenance(db, {
busyTimeoutMs: 5000,
databasePath: dbPath,
});
return db;
} catch (err) {
try {
db.close();
} catch {}
throw err;
}
}
type ExistingMemoryDatabaseOpenResult =

View File

@@ -0,0 +1,13 @@
// Linux-only test support for proving failed opens release their file handles.
import fs from "node:fs";
export function listOpenFileDescriptorsForPath(targetPath: string): string[] {
return fs.readdirSync("/proc/self/fd").flatMap((fd) => {
try {
const descriptorPath = fs.readlinkSync(`/proc/self/fd/${fd}`);
return descriptorPath.startsWith(targetPath) ? [descriptorPath] : [];
} catch {
return [];
}
});
}

View File

@@ -3,6 +3,7 @@ import fs from "node:fs";
import os from "node:os";
import path from "node:path";
import { afterEach, describe, expect, it, vi } from "vitest";
import { listOpenFileDescriptorsForPath } from "../infra/open-file-descriptors.test-support.js";
import {
acquireDebugProxyCaptureStore,
closeDebugProxyCaptureStore,
@@ -31,6 +32,18 @@ function makeStore() {
}
describe("DebugProxyCaptureStore", () => {
it.runIf(process.platform === "linux")("closes the database when initialization fails", () => {
const root = fs.mkdtempSync(path.join(os.tmpdir(), "openclaw-proxy-capture-failed-open-"));
cleanupDirs.push(root);
const dbPath = path.join(root, "capture.sqlite");
fs.writeFileSync(dbPath, "not a sqlite database");
expect(() => new DebugProxyCaptureStore(dbPath, path.join(root, "blobs"))).toThrow(
"file is not a database",
);
expect(listOpenFileDescriptorsForPath(dbPath)).toEqual([]);
});
it("keeps the cached store open until the last lease releases", () => {
const root = fs.mkdtempSync(path.join(os.tmpdir(), "openclaw-proxy-capture-lease-"));
cleanupDirs.push(root);

View File

@@ -36,50 +36,57 @@ function openDatabase(dbPath: string): OpenedDatabase {
ensureParentDir(dbPath);
const { DatabaseSync } = requireNodeSqlite();
const db = new DatabaseSync(dbPath);
const walMaintenance = configureSqliteConnectionPragmas(db, {
busyTimeoutMs: 5000,
databaseLabel: "debug-proxy-capture",
databasePath: dbPath,
});
db.exec(`
CREATE TABLE IF NOT EXISTS capture_sessions (
id TEXT PRIMARY KEY,
started_at INTEGER NOT NULL,
ended_at INTEGER,
mode TEXT NOT NULL,
source_scope TEXT NOT NULL,
source_process TEXT NOT NULL,
proxy_url TEXT,
db_path TEXT NOT NULL,
blob_dir TEXT NOT NULL
);
CREATE TABLE IF NOT EXISTS capture_events (
id INTEGER PRIMARY KEY,
session_id TEXT NOT NULL,
ts INTEGER NOT NULL,
source_scope TEXT NOT NULL,
source_process TEXT NOT NULL,
protocol TEXT NOT NULL,
direction TEXT NOT NULL,
kind TEXT NOT NULL,
flow_id TEXT NOT NULL,
method TEXT,
host TEXT,
path TEXT,
status INTEGER,
close_code INTEGER,
content_type TEXT,
headers_json TEXT,
data_text TEXT,
data_blob_id TEXT,
data_sha256 TEXT,
error_text TEXT,
meta_json TEXT
);
CREATE INDEX IF NOT EXISTS capture_events_session_ts_idx ON capture_events(session_id, ts);
CREATE INDEX IF NOT EXISTS capture_events_flow_idx ON capture_events(flow_id, ts);
`);
return { db, walMaintenance };
let walMaintenance: SqliteWalMaintenance | undefined;
try {
walMaintenance = configureSqliteConnectionPragmas(db, {
busyTimeoutMs: 5000,
databaseLabel: "debug-proxy-capture",
databasePath: dbPath,
});
db.exec(`
CREATE TABLE IF NOT EXISTS capture_sessions (
id TEXT PRIMARY KEY,
started_at INTEGER NOT NULL,
ended_at INTEGER,
mode TEXT NOT NULL,
source_scope TEXT NOT NULL,
source_process TEXT NOT NULL,
proxy_url TEXT,
db_path TEXT NOT NULL,
blob_dir TEXT NOT NULL
);
CREATE TABLE IF NOT EXISTS capture_events (
id INTEGER PRIMARY KEY,
session_id TEXT NOT NULL,
ts INTEGER NOT NULL,
source_scope TEXT NOT NULL,
source_process TEXT NOT NULL,
protocol TEXT NOT NULL,
direction TEXT NOT NULL,
kind TEXT NOT NULL,
flow_id TEXT NOT NULL,
method TEXT,
host TEXT,
path TEXT,
status INTEGER,
close_code INTEGER,
content_type TEXT,
headers_json TEXT,
data_text TEXT,
data_blob_id TEXT,
data_sha256 TEXT,
error_text TEXT,
meta_json TEXT
);
CREATE INDEX IF NOT EXISTS capture_events_session_ts_idx ON capture_events(session_id, ts);
CREATE INDEX IF NOT EXISTS capture_events_flow_idx ON capture_events(flow_id, ts);
`);
return { db, walMaintenance };
} catch (err) {
walMaintenance?.close();
db.close();
throw err;
}
}
function serializeJson(value: unknown): string | null {

View File

@@ -6,6 +6,7 @@ import path from "node:path";
import { afterEach, describe, expect, it } from "vitest";
import { executeSqliteQueryTakeFirstSync, getNodeSqliteKysely } from "../infra/kysely-sync.js";
import { requireNodeSqlite } from "../infra/node-sqlite.js";
import { listOpenFileDescriptorsForPath } from "../infra/open-file-descriptors.test-support.js";
import { readSqliteNumberPragma } from "../infra/sqlite-pragma.test-support.js";
import type { DB as OpenClawAgentKyselyDatabase } from "./openclaw-agent-db.generated.js";
import {
@@ -95,6 +96,21 @@ describe("openclaw agent database", () => {
expect(registered?.sizeBytes).toBeGreaterThan(0);
});
it.runIf(process.platform === "linux")("closes the database when initialization fails", () => {
const stateDir = createTempStateDir();
const databasePath = path.join(stateDir, "agent.sqlite");
fs.writeFileSync(databasePath, "not a sqlite database");
expect(() =>
openOpenClawAgentDatabase({
agentId: "worker-1",
env: { OPENCLAW_STATE_DIR: stateDir },
path: databasePath,
}),
).toThrow("file is not a database");
expect(listOpenFileDescriptorsForPath(databasePath)).toEqual([]);
});
it("keeps multiple registered paths for the same agent", () => {
const stateDir = createTempStateDir();
const env = { OPENCLAW_STATE_DIR: stateDir };

View File

@@ -268,20 +268,24 @@ export function openOpenClawAgentDatabase(
ensureOpenClawAgentDatabasePermissions(pathname, databaseOptions);
const sqlite = requireNodeSqlite();
const db = new sqlite.DatabaseSync(pathname);
const walMaintenance = configureSqliteConnectionPragmas(db, {
busyTimeoutMs: OPENCLAW_SQLITE_BUSY_TIMEOUT_MS,
databaseLabel: `openclaw-agent:${agentId}`,
databasePath: pathname,
foreignKeys: true,
synchronous: "NORMAL",
});
try {
ensureAgentSchema(db, agentId, pathname);
} catch (err) {
walMaintenance.close();
db.close();
throw err;
}
const walMaintenance = (() => {
let maintenance: SqliteWalMaintenance | undefined;
try {
maintenance = configureSqliteConnectionPragmas(db, {
busyTimeoutMs: OPENCLAW_SQLITE_BUSY_TIMEOUT_MS,
databaseLabel: `openclaw-agent:${agentId}`,
databasePath: pathname,
foreignKeys: true,
synchronous: "NORMAL",
});
ensureAgentSchema(db, agentId, pathname);
return maintenance;
} catch (err) {
maintenance?.close();
db.close();
throw err;
}
})();
ensureOpenClawAgentDatabasePermissions(pathname, databaseOptions);
const database = { agentId, db, path: pathname, walMaintenance };
cachedDatabases.set(pathname, database);

View File

@@ -11,6 +11,7 @@ import {
getNodeSqliteKysely,
} from "../infra/kysely-sync.js";
import { requireNodeSqlite } from "../infra/node-sqlite.js";
import { listOpenFileDescriptorsForPath } from "../infra/open-file-descriptors.test-support.js";
import { readSqliteNumberPragma } from "../infra/sqlite-pragma.test-support.js";
import type { DB as OpenClawStateKyselyDatabase } from "./openclaw-state-db.generated.js";
import {
@@ -79,6 +80,16 @@ describe("openclaw state database", () => {
expect(database.path).toBe(path.join(stateDir, "state", "openclaw.sqlite"));
});
it.runIf(process.platform === "linux")("closes the database when initialization fails", () => {
const databasePath = path.join(createTempStateDir(), "openclaw.sqlite");
fs.writeFileSync(databasePath, "not a sqlite database");
expect(() => openOpenClawStateDatabase({ path: databasePath })).toThrow(
"file is not a database",
);
expect(listOpenFileDescriptorsForPath(databasePath)).toEqual([]);
});
it("migrates requester and executor attribution for existing cross-agent tasks", () => {
const stateDir = createTempStateDir();
const database = openOpenClawStateDatabase({

View File

@@ -977,20 +977,24 @@ export function openOpenClawStateDatabase(
ensureOpenClawStatePermissions(pathname, env);
const sqlite = requireNodeSqlite();
const db = new sqlite.DatabaseSync(pathname);
const walMaintenance = configureSqliteConnectionPragmas(db, {
busyTimeoutMs: OPENCLAW_SQLITE_BUSY_TIMEOUT_MS,
databaseLabel: "openclaw-state",
databasePath: pathname,
foreignKeys: true,
synchronous: "NORMAL",
});
try {
ensureSchema(db, pathname);
} catch (err) {
walMaintenance.close();
db.close();
throw err;
}
const walMaintenance = (() => {
let maintenance: SqliteWalMaintenance | undefined;
try {
maintenance = configureSqliteConnectionPragmas(db, {
busyTimeoutMs: OPENCLAW_SQLITE_BUSY_TIMEOUT_MS,
databaseLabel: "openclaw-state",
databasePath: pathname,
foreignKeys: true,
synchronous: "NORMAL",
});
ensureSchema(db, pathname);
return maintenance;
} catch (err) {
maintenance?.close();
db.close();
throw err;
}
})();
ensureOpenClawStatePermissions(pathname, env);
const database = { db, path: pathname, walMaintenance };
cachedDatabases.set(pathname, database);