From 39900569bf807dea05260abde90fb07619922aa9 Mon Sep 17 00:00:00 2001 From: Claude Date: Mon, 19 Oct 2026 10:46:18 +0000 Subject: [PATCH] Release the embedded-database lock when the database closes The lock was held until the process exited, so a process that opened and closed the database and then started a server on the same folder (as the HTTP verification scripts do) made the server refuse to start. Several openers in one process now share a claim that is released on close. Co-Authored-By: Claude Sonnet 5.5 Claude-Session: https://claude.ai/code/session_01Lu73uozkZGX8gP8LWNbBqt --- mikro-orm.config.ts | 10 ++++-- src/server/adapters/db/config.ts | 3 -- src/server/adapters/db/orm.ts | 18 ++++++++-- .../adapters/db/pglite-guard.db.test.ts | 36 ++++++++++++++++++- src/server/adapters/db/pglite-guard.ts | 33 +++++++++++++---- 5 files changed, 86 insertions(+), 14 deletions(-) diff --git a/mikro-orm.config.ts b/mikro-orm.config.ts index 49588e8..f8d1909 100644 --- a/mikro-orm.config.ts +++ b/mikro-orm.config.ts @@ -1,3 +1,9 @@ -import { createMikroOrmOptions } from "./src/server/adapters/db/config"; +import { createMikroOrmOptions, loadDatabaseConfig } from "./src/server/adapters/db/config"; +import { claimPgliteDirectory } from "./src/server/adapters/db/pglite-guard"; -export default createMikroOrmOptions(); +// The MikroORM CLI (`npm run db:migrate`) opens the database from this file. PGlite has no lock of its own, so claim the +// folder for the life of the command; it is released when the command exits. +const config = loadDatabaseConfig(); +if (config.profile === "pglite") claimPgliteDirectory(config.dataDir); + +export default createMikroOrmOptions(config); diff --git a/src/server/adapters/db/config.ts b/src/server/adapters/db/config.ts index 99fa09c..5d86cd8 100644 --- a/src/server/adapters/db/config.ts +++ b/src/server/adapters/db/config.ts @@ -8,7 +8,6 @@ import { PostgreSqlDriver } from "@mikro-orm/postgresql"; import { z } from "zod"; import { persistenceEntities } from "./entities"; -import { claimPgliteDirectory } from "./pglite-guard"; import { nextMigrationName } from "./schema-version"; const databaseEnvironmentSchema = z @@ -123,8 +122,6 @@ export function createMikroOrmOptions( // PGlite creates its directory without `recursive`, so a clean checkout (no `.data`) would fail with ENOENT. mkdirSync(path.dirname(config.dataDir), { recursive: true }); - // PGlite has no lock of its own; this also covers the MikroORM CLI (`npm run db:migrate`), which builds its options here. - claimPgliteDirectory(config.dataDir); return { ...commonOptions, diff --git a/src/server/adapters/db/orm.ts b/src/server/adapters/db/orm.ts index a6c660c..56f533a 100644 --- a/src/server/adapters/db/orm.ts +++ b/src/server/adapters/db/orm.ts @@ -11,7 +11,7 @@ import { type DatabaseConfig, } from "./config"; import { migrateOnStartup } from "./migration-startup"; -import { openGuarded } from "./pglite-guard"; +import { claimPgliteDirectory, openGuarded, releasePgliteDirectory } from "./pglite-guard"; import { createPersistenceRepositories } from "./repositories"; type DatabaseGlobal = typeof globalThis & { @@ -33,7 +33,21 @@ export async function createDatabaseOrm( throw error; } }; - return config.profile === "pglite" ? openGuarded(config.dataDir, open) : open(); + if (config.profile !== "pglite") return open(); + // The claim lasts as long as the database is open, so a process that closes it can hand the folder to another one. + claimPgliteDirectory(config.dataDir); + try { + const orm = await openGuarded(config.dataDir, open); + const close = orm.close.bind(orm); + let released = false; + orm.close = async (...args: Parameters) => { + try { return await close(...args); } finally { if (!released) { released = true; releasePgliteDirectory(config.dataDir); } } + }; + return orm; + } catch (error) { + releasePgliteDirectory(config.dataDir); + throw error; + } } export async function bootstrapRuntimeDatabase(orm: MikroORM) { diff --git a/src/server/adapters/db/pglite-guard.db.test.ts b/src/server/adapters/db/pglite-guard.db.test.ts index 6b62f7b..9c206c2 100644 --- a/src/server/adapters/db/pglite-guard.db.test.ts +++ b/src/server/adapters/db/pglite-guard.db.test.ts @@ -3,7 +3,8 @@ import { mkdirSync, mkdtempSync, rmSync, writeFileSync } from "node:fs"; import { tmpdir } from "node:os"; import { join } from "node:path"; import { describe, expect, it } from "vitest"; -import { claimPgliteDirectory, DatabaseOpenError, lockPathFor } from "./pglite-guard"; +import { existsSync } from "node:fs"; +import { claimPgliteDirectory, DatabaseOpenError, lockPathFor, releasePgliteDirectory } from "./pglite-guard"; function scratch() { const dir = mkdtempSync(join(tmpdir(), "a2a-guard-")); @@ -36,6 +37,39 @@ describe("PGlite directory lock", () => { }); }); +describe("PGlite directory lock lifetime", () => { + it("shares one claim between openers in a process and removes the lock file with the last release", () => { + const { dataDir, done } = scratch(); + try { + claimPgliteDirectory(dataDir); claimPgliteDirectory(dataDir); + expect(existsSync(lockPathFor(dataDir))).toBe(true); + releasePgliteDirectory(dataDir); + expect(existsSync(lockPathFor(dataDir))).toBe(true); + releasePgliteDirectory(dataDir); + expect(existsSync(lockPathFor(dataDir))).toBe(false); + releasePgliteDirectory(dataDir); // an extra release is harmless + } finally { done(); } + }); + + it("a process that has closed its database no longer blocks another, even while it keeps running", () => { + const { dataDir, done } = scratch(); + try { + // The HTTP verification scripts do this: open, close, then start the server as a child while they keep running. + const result = run(` + import { createDatabaseOrm } from "./src/server/adapters/db/orm"; + const a = await createDatabaseOrm(); await a.migrator.up(); await a.close(true); + const { spawnSync } = await import("node:child_process"); + const child = spawnSync(process.execPath, ["--import", "tsx", "--input-type=module", "-e", + 'import { createDatabaseOrm } from "./src/server/adapters/db/orm"; try { const o = await createDatabaseOrm(); console.log("OPENED"); await o.close(true); } catch (e) { console.log(e.name + ": " + e.message); } process.exit(0);'], + { encoding: "utf8", env: process.env }); + console.log(child.stdout); + process.exit(0);`, dataDir); + expect(result.stdout).toContain("OPENED"); + expect(result.stdout).not.toContain("already open"); + } finally { done(); } + }, 120_000); +}); + describe("PGlite open failures", () => { it("a second process is refused with a clear message instead of sharing the folder", () => { const { dataDir, done } = scratch(); diff --git a/src/server/adapters/db/pglite-guard.ts b/src/server/adapters/db/pglite-guard.ts index 1f9b588..157c823 100644 --- a/src/server/adapters/db/pglite-guard.ts +++ b/src/server/adapters/db/pglite-guard.ts @@ -11,7 +11,9 @@ export class DatabaseOpenError extends Error { } } -const held = new Set(); +/** Folders this process has claimed, with how many openers inside it hold each. */ +const held = new Map(); +let exitHookInstalled = false; function alive(pid: number): boolean { try { process.kill(pid, 0); return true; } catch (error) { return (error as NodeJS.ErrnoException).code === "EPERM"; } @@ -22,22 +24,25 @@ export const lockPathFor = (dataDir: string) => `${dataDir}.lock`; /** * PGlite is a single-owner database with no lock of its own: a second process that opens the same folder succeeds, and * both then write to it, which damages it. This takes a lock file beside the folder (never inside it) and refuses while - * another live process holds it. A lock left by a process that died is taken over. Held until this process exits. + * another live process holds it. A lock left by a process that died is taken over. Several openers in one process share + * one claim; call `releasePgliteDirectory` once per claim when each closes, so a process that has closed its database + * (and then starts a server on the same folder, for instance) no longer blocks anyone. */ export function claimPgliteDirectory(dataDir: string): void { - if (held.has(dataDir)) return; + const count = held.get(dataDir); + if (count !== undefined) { held.set(dataDir, count + 1); return; } const file = lockPathFor(dataDir); for (let attempt = 0; attempt < 2; attempt++) { try { writeFileSync(file, `${JSON.stringify({ pid: process.pid, startedAt: new Date().toISOString() })}\n`, { flag: "wx" }); - held.add(dataDir); - process.once("exit", () => { try { rmSync(file); } catch { /* already gone */ } }); + held.set(dataDir, 1); + installExitHook(); return; } catch (error) { if ((error as NodeJS.ErrnoException).code !== "EEXIST") return; // an unwritable location is reported by the database itself let owner: number | undefined; try { owner = (JSON.parse(readFileSync(file, "utf8")) as { pid?: number }).pid; } catch { /* unreadable: treat as stale */ } - if (owner === process.pid) { held.add(dataDir); return; } + if (owner === process.pid) { held.set(dataDir, 1); installExitHook(); return; } if (owner !== undefined && alive(owner)) { throw new DatabaseOpenError( `The embedded database in ${dataDir} is already open in another process (pid ${owner}). ` + @@ -50,6 +55,22 @@ export function claimPgliteDirectory(dataDir: string): void { } } +/** Gives up one claim. The lock file goes when the last opener in this process lets go. */ +export function releasePgliteDirectory(dataDir: string): void { + const count = held.get(dataDir); + if (count === undefined) return; + if (count > 1) { held.set(dataDir, count - 1); return; } + held.delete(dataDir); + try { rmSync(lockPathFor(dataDir)); } catch { /* already gone */ } +} + +/** A process that ends while still holding a claim takes its lock files with it. */ +function installExitHook() { + if (exitHookInstalled) return; + exitHookInstalled = true; + process.once("exit", () => { for (const dataDir of held.keys()) try { rmSync(lockPathFor(dataDir)); } catch { /* already gone */ } }); +} + /** * PGlite reports an unreadable folder by throwing `RuntimeError: Aborted()` from inside its WebAssembly, which escapes as an * uncaught exception and ends the whole process instead of rejecting the open. Catch that for the duration of the open and