From 04eef0e197775b3e9c2d5e10b1db44a808b33a38 Mon Sep 17 00:00:00 2001 From: fatihcvs <105765934+fatihcvs@users.noreply.github.com> Date: Mon, 21 Sep 2026 06:39:12 +0300 Subject: [PATCH] fix: retry lock acquisition only for existing locks --- src/core/store.ts | 5 +-- src/orchestrator/state.ts | 10 +++--- test/unit/lock-errors.test.ts | 62 +++++++++++++++++++++++++++++++++++ 3 files changed, 71 insertions(+), 6 deletions(-) create mode 100644 test/unit/lock-errors.test.ts diff --git a/src/core/store.ts b/src/core/store.ts index e972b91..c1da4fc 100644 --- a/src/core/store.ts +++ b/src/core/store.ts @@ -334,8 +334,9 @@ export class StateStore { try { fd = openSync(this.lockPath, "wx") break - } catch { - // Either EEXIST (someone else holds it) or a race lost. Inspect age. + } catch (error) { + if ((error as NodeJS.ErrnoException).code !== "EEXIST") throw error + // Someone else holds it, or another creator won the race. Inspect age. try { const st = statSync(this.lockPath) if (Date.now() - st.mtimeMs > LOCK_STALE_MS) { diff --git a/src/orchestrator/state.ts b/src/orchestrator/state.ts index aacd583..3a94a12 100644 --- a/src/orchestrator/state.ts +++ b/src/orchestrator/state.ts @@ -390,13 +390,13 @@ export class OrchestratorStore { private async ownLock(fn: () => T): Promise { mkdirSync(this.dir, { recursive: true }) const deadline = Date.now() + ORCH_LOCK_TIMEOUT_MS + let fd: number for (;;) { try { - const fd = openSync(this.lockPath, "wx") - writeFileSync(fd, `${process.pid}@${Date.now()}`, "utf8") - closeSync(fd) + fd = openSync(this.lockPath, "wx") break - } catch { + } catch (error) { + if ((error as NodeJS.ErrnoException).code !== "EEXIST") throw error try { if (Date.now() - statSync(this.lockPath).mtimeMs > ORCH_LOCK_STALE_MS) { try { @@ -415,8 +415,10 @@ export class OrchestratorStore { } } try { + writeFileSync(fd, `${process.pid}@${Date.now()}`, "utf8") return fn() } finally { + closeSync(fd) try { unlinkSync(this.lockPath) } catch { diff --git a/test/unit/lock-errors.test.ts b/test/unit/lock-errors.test.ts new file mode 100644 index 0000000..aa024aa --- /dev/null +++ b/test/unit/lock-errors.test.ts @@ -0,0 +1,62 @@ +import { test } from "node:test" +import assert from "node:assert/strict" +import fs from "node:fs" +import { syncBuiltinESMExports } from "node:module" +import { tmpdir } from "node:os" +import { join } from "node:path" +import { StateStore } from "../../src/core/store.js" +import { OrchestratorStore } from "../../src/orchestrator/state.js" + +for (const Store of [StateStore, OrchestratorStore]) { + for (const code of ["EACCES", "EROFS", "EMFILE"]) { + test(`${Store.name} immediately preserves ${code} from lock creation`, async (t) => { + const dir = fs.mkdtempSync(join(tmpdir(), "opencomms-lock-error-")) + const error = Object.assign(new Error(`${code}: cannot create lock`), { code }) + let attempts = 0 + let called = false + t.mock.method(fs, "openSync", () => { + attempts++ + throw error + }) + syncBuiltinESMExports() + try { + await assert.rejects( + new Store(dir).withLock(() => { + called = true + }), + (err) => err === error, + ) + assert.equal(attempts, 1) + assert.equal(called, false) + } finally { + t.mock.restoreAll() + syncBuiltinESMExports() + fs.rmSync(dir, { recursive: true, force: true }) + } + }) + } + + test(`${Store.name} retries EEXIST and releases the acquired lock`, async (t) => { + const dir = fs.mkdtempSync(join(tmpdir(), "opencomms-lock-retry-")) + const originalOpen = fs.openSync + let attempts = 0 + t.mock.method(fs, "openSync", (...args: Parameters) => { + if (++attempts === 1) throw Object.assign(new Error("already exists"), { code: "EEXIST" }) + return originalOpen(...args) + }) + syncBuiltinESMExports() + try { + const store = new Store(dir) + assert.equal(await store.withLock(() => "acquired"), "acquired") + assert.equal(attempts, 2) + assert.deepEqual( + fs.readdirSync(store.dir).filter((name) => name.endsWith(".lock")), + [], + ) + } finally { + t.mock.restoreAll() + syncBuiltinESMExports() + fs.rmSync(dir, { recursive: true, force: true }) + } + }) +}