From c5c92e6be1027e10f278cecadd61f07e377eebfd Mon Sep 17 00:00:00 2001 From: jiarung Date: Sun, 15 Mar 2026 05:42:29 +0000 Subject: [PATCH] fix(file-lock): reclaim lock files with invalid or empty content MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit The lock file is created (empty) by open("wx") before pid/createdAt are written by the subsequent writeFile. A process that crashes in this narrow window leaves an empty .lock file whose content readLockPayload() cannot parse (returns null). Previously isStaleLock skipped both the pid-alive and the age checks when payload was null, falling through to the mtime stat. If the mtime was still within staleMs the function returned false, making the empty lock appear live indefinitely — every future writer would time out and silently drop its usage record until the file was manually deleted. Fix: treat null payload (empty, truncated, or non-JSON content) as stale immediately. Such a file could only have been left by a process that never completed the write, so it is safe to reclaim without waiting for the mtime timeout. The mtime stat fallback is also removed: its only useful case was exactly this null-payload scenario (it was redundant when payload is valid, since the pid-alive and createdAt-age checks already cover the live-lock and aged-out-lock cases). Tests added: - empty lock file → reclaimed, callback runs - truncated/invalid JSON lock file → reclaimed - pid field not a number → reclaimed --- src/plugin-sdk/file-lock.test.ts | 91 ++++++++++++++++++++++++++++++++ src/plugin-sdk/file-lock.ts | 23 ++++---- 2 files changed, 103 insertions(+), 11 deletions(-) create mode 100644 src/plugin-sdk/file-lock.test.ts diff --git a/src/plugin-sdk/file-lock.test.ts b/src/plugin-sdk/file-lock.test.ts new file mode 100644 index 00000000000..e5d015ad9cf --- /dev/null +++ b/src/plugin-sdk/file-lock.test.ts @@ -0,0 +1,91 @@ +import fs from "node:fs/promises"; +import os from "node:os"; +import path from "node:path"; +import { afterEach, beforeEach, describe, expect, it } from "vitest"; +import { withFileLock } from "./file-lock.js"; + +const LOCK_OPTIONS = { + retries: { + retries: 2, + factor: 1, + minTimeout: 20, + maxTimeout: 50, + }, + stale: 5_000, +}; + +describe("withFileLock", () => { + let tmpDir: string; + let targetFile: string; + + beforeEach(async () => { + tmpDir = await fs.mkdtemp(path.join(os.tmpdir(), "file-lock-test-")); + targetFile = path.join(tmpDir, "data.json"); + }); + + afterEach(async () => { + await fs.rm(tmpDir, { recursive: true, force: true }); + }); + + it("acquires and releases the lock, allowing a second caller to proceed", async () => { + const order: string[] = []; + await withFileLock(targetFile, LOCK_OPTIONS, async () => { + order.push("first-start"); + await new Promise((r) => setTimeout(r, 10)); + order.push("first-end"); + }); + await withFileLock(targetFile, LOCK_OPTIONS, async () => { + order.push("second"); + }); + expect(order).toEqual(["first-start", "first-end", "second"]); + }); + + it("reclaims an empty lock file left by a crash between open and writeFile", async () => { + // Simulate a crash in the open("wx")-to-writeFile window: the .lock file + // exists but has empty (unparseable) content. + const lockPath = `${targetFile}.lock`; + await fs.mkdir(path.dirname(targetFile), { recursive: true }); + await fs.writeFile(lockPath, ""); // empty — no pid/createdAt written + + // withFileLock must not time out; it should reclaim the empty lock and + // run the callback without error. + let ran = false; + await expect( + withFileLock(targetFile, LOCK_OPTIONS, async () => { + ran = true; + }), + ).resolves.toBeUndefined(); + expect(ran).toBe(true); + }); + + it("reclaims a lock file containing partial/invalid JSON", async () => { + const lockPath = `${targetFile}.lock`; + await fs.mkdir(path.dirname(targetFile), { recursive: true }); + await fs.writeFile(lockPath, '{"pid":'); // truncated JSON + + let ran = false; + await expect( + withFileLock(targetFile, LOCK_OPTIONS, async () => { + ran = true; + }), + ).resolves.toBeUndefined(); + expect(ran).toBe(true); + }); + + it("reclaims a lock file whose pid field is not a number", async () => { + const lockPath = `${targetFile}.lock`; + await fs.mkdir(path.dirname(targetFile), { recursive: true }); + await fs.writeFile( + lockPath, + JSON.stringify({ pid: "not-a-number", createdAt: new Date().toISOString() }), + ); + + let ran = false; + await expect( + withFileLock(targetFile, LOCK_OPTIONS, async () => { + ran = true; + }), + ).resolves.toBeUndefined(); + expect(ran).toBe(true); + }); +}); diff --git a/src/plugin-sdk/file-lock.ts b/src/plugin-sdk/file-lock.ts index 98277381868..ee80bcd1d81 100644 --- a/src/plugin-sdk/file-lock.ts +++ b/src/plugin-sdk/file-lock.ts @@ -64,21 +64,22 @@ async function resolveNormalizedFilePath(filePath: string): Promise { async function isStaleLock(lockPath: string, staleMs: number): Promise { const payload = await readLockPayload(lockPath); - if (payload?.pid && !isPidAlive(payload.pid)) { + // A lock file with missing or unparseable content was left by a process + // that crashed between open("wx") (which creates the file) and the + // subsequent writeFile (which fills in the pid/createdAt). Treat it as + // stale immediately so it can be reclaimed rather than blocking every + // future writer until the mtime-based timeout expires. + if (payload === null) { return true; } - if (payload?.createdAt) { - const createdAt = Date.parse(payload.createdAt); - if (!Number.isFinite(createdAt) || Date.now() - createdAt > staleMs) { - return true; - } - } - try { - const stat = await fs.stat(lockPath); - return Date.now() - stat.mtimeMs > staleMs; - } catch { + if (!isPidAlive(payload.pid)) { return true; } + const createdAt = Date.parse(payload.createdAt); + if (!Number.isFinite(createdAt) || Date.now() - createdAt > staleMs) { + return true; + } + return false; } export type FileLockHandle = {