From 88b38db008ffd67050360336ba1c3ad40c579b0e Mon Sep 17 00:00:00 2001 From: Alex Lavaee Date: Tue, 27 Oct 2026 17:06:35 -0700 Subject: [PATCH] Release the writer lease when Chopin is stopped while it starts A Chopin stopped soon after it started could exit without releasing the `chopin:writer` lease. The next process on the same database then refused to start with "another Chopin instance owns the database" until the lease expired, up to 30 seconds later. Two gaps caused this: - The SIGINT and SIGTERM handlers were registered only at the very end of startup: after the server was already listening, and after research recovery and the job runner started. A stop in that window took Node's default action, so the process exited without running the drain that releases the lease. Supervisors that start Chopin, wait for it to answer and then stop it hit this often, especially on slower machines. - Startup's session reset replaces the held lease with a re-fenced one. A drain that ran while the reset was in flight released the old fencing value, which no longer matched the row. The handlers are now registered as soon as the lease is held. The drain waits for the session reset before it releases, and skips stopping the HTTP server when it isn't listening yet. Reproduced by starting Chopin, sending SIGTERM as soon as it answered HTTP, and starting it again at once: the restart was refused 3 times in 5 before this change and 0 in 10 after it. Assistant-model: Claude Opus 5.5 (fast) Assistant-workflow: inline Assistant-verification: red-before-fix passed: the new startup.test.ts case, which stops the server as soon as it holds the lease, fails without the change (lease still held after exit) and passes 8 runs in a row with it Assistant-verification: manual scenario passed: SIGTERM as soon as Chopin answered, then an immediate restart; refused 3/5 before the fix, 0/10 after Assistant-verification: bun test passed: GIT_CONFIG_GLOBAL=/dev/null bun test (0 fail), and the PostgreSQL lifecycle suite (122 pass) Assistant-verification: typecheck and lint passed: bun run types and bun run ci Co-authored-by: Alex Lavaee --- apps/server/src/main.ts | 18 +++++++---- .../src/storage/postgres/startup.test.ts | 30 +++++++++++++++++++ 2 files changed, 43 insertions(+), 5 deletions(-) diff --git a/apps/server/src/main.ts b/apps/server/src/main.ts index af64aab90..af687e26f 100644 --- a/apps/server/src/main.ts +++ b/apps/server/src/main.ts @@ -1847,6 +1847,8 @@ function listen(): Server { /** Nothing in memory is worth losing to a Ctrl-C. */ let draining: Promise | undefined; +/** Startup's session reset, which replaces the held writer lease with a re-fenced one. */ +let resettingLease: Promise | undefined; function drain(): Promise { return draining ??= (async () => { @@ -1861,7 +1863,7 @@ function drain(): Promise { record(err); } }; - await attempt(() => server.stop(true)); + if (server) await attempt(() => server.stop(true)); if (researchRecoveryTimer) clearTimeout(researchRecoveryTimer); if (experimentTimer) clearInterval(experimentTimer); if (recoveringResearch) await attempt(() => recoveringResearch!); @@ -1897,6 +1899,7 @@ function drain(): Promise { } await attempt(() => shutdownHarnesses()); await attempt(() => writerLease.stop()); + if (resettingLease) await attempt(() => resettingLease!); let lease = writerLease.held; if (lease) await attempt(() => storage.leases.release(lease)); await attempt(() => storage.close()); @@ -2867,8 +2870,15 @@ try { ); if (!acquired) throw new Error("another Chopin instance owns the database"); writerLease.hold(acquired); - let reset = await storage.sessions.reset(new Date(), acquired, LEASE_TTL_MS); - writerLease.held = reset.lease; + // A stop that arrives while startup still runs must release the lease, or the next start finds it held. + process.on("SIGINT", signal); + process.on("SIGTERM", signal); + let resetting = storage.sessions.reset(new Date(), acquired, LEASE_TTL_MS).then(reset => { + writerLease.held = reset.lease; + return reset; + }); + resettingLease = resetting; + let reset = await resetting; // Before any room opens, so no child Conversation is ever loaded or served. let children = await storage.collaboration.dropChildConversations(reset.lease); if (children.cleared.length) { @@ -2926,7 +2936,5 @@ try { await storage.close().catch(() => {}); throw err; } -process.on("SIGINT", signal); -process.on("SIGTERM", signal); console.log(describe(config)); diff --git a/apps/server/src/storage/postgres/startup.test.ts b/apps/server/src/storage/postgres/startup.test.ts index 1decabd7f..0ed8efabc 100644 --- a/apps/server/src/storage/postgres/startup.test.ts +++ b/apps/server/src/storage/postgres/startup.test.ts @@ -161,6 +161,36 @@ if (database) { await storage.close(); }, 20_000); + it("releases the writer lease when stopped while it is still starting", async () => { + let storage = new PostgresStorage(database); + await storage.migrate(); + let sql = new SQL(database); + let held = async () => { + let [row] = await sql<{ owner: string }[]>` + SELECT owner FROM storage_leases + WHERE name = 'chopin:writer' AND expires_at > clock_timestamp() + `; + return row?.owner; + }; + let before = await held(); + let child = spawn(9077, "pipe"); + let owner: string | undefined; + for (let attempt = 0; attempt < 2_000 && (!owner || owner === before); attempt++) { + owner = await held(); + if (!owner || owner === before) await Bun.sleep(5); + } + expect(owner).toBeDefined(); + expect(owner).not.toBe(before); + child.kill("SIGTERM"); + await child.exited; + expect(await held()).toBeUndefined(); + let next = await storage.leases.acquire("chopin:writer", crypto.randomUUID(), 30_000); + expect(next).toBeDefined(); + await storage.leases.release(next!); + await sql.close(); + await storage.close(); + }, 20_000); + it("recovers an inline Research card without an open room, once across restarts", async () => { let storage = new PostgresStorage(database); await storage.migrate();