diff --git a/packages/runtime-playground/src/playground-wordpress-archive-cache.ts b/packages/runtime-playground/src/playground-wordpress-archive-cache.ts index 7338b7b25..10c8c3d13 100644 --- a/packages/runtime-playground/src/playground-wordpress-archive-cache.ts +++ b/packages/runtime-playground/src/playground-wordpress-archive-cache.ts @@ -147,7 +147,16 @@ export async function withPlaygroundArchiveCacheLock(cacheDirectory: string, if (lease) { break } - if (!await cacheLockIsActive(lockPath, Date.now(), policy, true)) { + let active: boolean + try { + active = await cacheLockIsActive(lockPath, Date.now(), policy, true) + } catch (error) { + // A waiter can observe the completed owner removing its expired lease sidecar. + // Re-read the path rather than treating this expected handoff as a replacement attack. + if (isLeaseSidecarHandoff(error)) continue + throw error + } + if (!active) { continue } if (Date.now() - startedAt > 120_000) { @@ -370,9 +379,11 @@ async function tryAcquireCacheLock(lockPath: string, leaseMs: number): Promise undefined) - await rmdir(lockPath).catch(() => undefined) - if (isConcurrentDisappearance(error)) return undefined + if (directory) { + await removeLeaseDirectoryIfCurrent(directory).catch(() => undefined) + await directory.handle.close().catch(() => undefined) + } + if (isConcurrentDisappearance(error) || isLeaseSidecarHandoff(error)) return undefined throw error } } @@ -459,6 +470,7 @@ async function cacheLockIsActive(lockPath: string, now: number, policy: Pick { - if (!isConcurrentDisappearance(error) && !errorHasCode(error, "ENOTEMPTY")) throw error - }) - } } return active } @@ -709,12 +718,32 @@ function leaseDirectoryChildPath(directory: LeaseDirectory, name?: string): stri async function assertLeaseDirectoryGeneration(directory: LeaseDirectory): Promise { if (directory.access !== "generation-checked-path") return + await assertLeaseDirectoryPathGeneration(directory) +} + +async function assertLeaseDirectoryPathGeneration(directory: LeaseDirectory): Promise { const [pathStat, handleStat] = await Promise.all([lstat(directory.path), directory.handle.stat()]) if (!pathStat.isDirectory() || pathStat.isSymbolicLink() || pathStat.dev !== handleStat.dev || pathStat.ino !== handleStat.ino) { throw new LeaseSidecarUnsafeError(`Playground cache lease sidecar changed while accessing: ${directory.path}`) } } +async function removeLeaseDirectoryIfCurrent(directory: LeaseDirectory): Promise { + try { + await assertLeaseDirectoryPathGeneration(directory) + } catch (error) { + if (isConcurrentDisappearance(error) || isLeaseSidecarHandoff(error)) return false + throw error + } + try { + await rmdir(directory.path) + return true + } catch (error) { + if (isConcurrentDisappearance(error) || errorHasCode(error, "ENOTEMPTY")) return false + throw error + } +} + async function readLeaseDirectory(directory: LeaseDirectory): Promise { await assertLeaseDirectoryGeneration(directory) try { @@ -844,6 +873,10 @@ function isConcurrentDisappearance(error: unknown): boolean { return errorHasCode(error, "ENOENT") || errorHasCode(error, "ESTALE") } +function isLeaseSidecarHandoff(error: unknown): boolean { + return error instanceof LeaseSidecarUnsafeError && error.message.includes("changed while accessing") +} + async function unlinkIfPresent(path: string): Promise { try { await unlink(path) diff --git a/tests/fixtures/playground-cache-lock-child.ts b/tests/fixtures/playground-cache-lock-child.ts new file mode 100644 index 000000000..2d34e024d --- /dev/null +++ b/tests/fixtures/playground-cache-lock-child.ts @@ -0,0 +1,34 @@ +import { lstat, writeFile } from "node:fs/promises" + +import { withPlaygroundArchiveCacheLock } from "../../packages/runtime-playground/src/playground-wordpress-archive-cache.js" + +const [cacheDirectory, version, archivePath, iterations, readyPath, startPath] = process.argv.slice(2) +if (!cacheDirectory || !version || !archivePath || !iterations || !readyPath || !startPath) { + throw new Error("cacheDirectory, version, archivePath, iterations, readyPath, and startPath are required") +} + +await writeFile(readyPath, "ready") +while (!await exists(startPath)) { + await new Promise((resolve) => setTimeout(resolve, 5)) +} + +for (let index = 0; index < Number(iterations); index += 1) { + await withPlaygroundArchiveCacheLock(cacheDirectory, version, async () => { + try { + await writeFile(archivePath, `${process.pid}\n`, { flag: "wx" }) + } catch (error) { + if (!error || typeof error !== "object" || !("code" in error) || error.code !== "EEXIST") throw error + } + await new Promise((resolve) => setTimeout(resolve, 2)) + }) +} + +async function exists(path: string): Promise { + try { + await lstat(path) + return true + } catch (error) { + if (error && typeof error === "object" && "code" in error && error.code === "ENOENT") return false + throw error + } +} diff --git a/tests/playground-custom-archive-cache-process.test.ts b/tests/playground-custom-archive-cache-process.test.ts index dd274b79a..d6fac1e06 100644 --- a/tests/playground-custom-archive-cache-process.test.ts +++ b/tests/playground-custom-archive-cache-process.test.ts @@ -1,6 +1,6 @@ import assert from "node:assert/strict" import { spawn, type ChildProcess } from "node:child_process" -import { lstat, mkdtemp, readFile, readdir, rm, utimes, writeFile } from "node:fs/promises" +import { lstat, mkdir, mkdtemp, readFile, readdir, rm, utimes, writeFile } from "node:fs/promises" import { tmpdir } from "node:os" import { join } from "node:path" @@ -8,6 +8,7 @@ import { maintainPlaygroundCustomArchiveCache } from "../packages/runtime-playgr const root = await mkdtemp(join(tmpdir(), "wp-codebox-playground-cache-process-")) const fixture = join(process.cwd(), "tests/fixtures/playground-cache-lease-child.ts") +const lockFixture = join(process.cwd(), "tests/fixtures/playground-cache-lock-child.ts") const tsx = join(process.cwd(), "node_modules/.bin/tsx") const children: ChildProcess[] = [] @@ -37,6 +38,33 @@ try { assert.ok(afterExpiry.removedCount >= 1, JSON.stringify(afterExpiry)) assert.ok(!await exists(archivePath), "expired crashed-process lease must become reclaimable") + const lockArchivePath = join(root, "7.0.3.zip") + const firstLockChild = await startLockChild("first-lock-child", "7.0.3", lockArchivePath, 300) + const secondLockChild = await startLockChild("second-lock-child", "7.0.3", lockArchivePath, 300) + await Promise.all([writeFile(firstLockChild.startPath, "start"), writeFile(secondLockChild.startPath, "start")]) + const lockResults = await Promise.all([firstLockChild.result, secondLockChild.result]) + assert.deepEqual(lockResults.map((result) => result.code), [0, 0], lockResults.map((result) => result.output).join("\n")) + assert.match(await readFile(lockArchivePath, "utf8"), /^\d+\n$/, "exactly one lock owner must materialize the archive") + + const staleVersion = "7.0.4" + const staleLockPath = join(root, `${staleVersion}.zip.lock`) + await mkdir(staleLockPath) + await writeFile(join(staleLockPath, "owner.json"), JSON.stringify({ + schema: "wp-codebox/playground-cache-lease/v1", + token: "crashed-owner", + hostname: "test-host", + bootId: "test-boot", + pid: 1, + processStart: "0", + createdAt: new Date(0).toISOString(), + heartbeatAt: new Date(0).toISOString(), + expiresAt: new Date(0).toISOString(), + })) + const staleChild = await startLockChild("stale-lock-child", staleVersion, join(root, `${staleVersion}.zip`), 1) + await writeFile(staleChild.startPath, "start") + const staleResult = await staleChild.result + assert.equal(staleResult.code, 0, staleResult.output) + console.log("playground custom archive separate-process leases passed") } finally { for (const child of children) { @@ -66,6 +94,25 @@ async function stopChild(entry: { child: ChildProcess; stopPath: string }): Prom await childExit(entry.child) } +async function startLockChild(name: string, version: string, archivePath: string, iterations: number): Promise<{ startPath: string; result: Promise<{ code: number | null; output: string }> }> { + const readyPath = join(root, `${name}.ready`) + const startPath = join(root, `${name}.start`) + let child!: ChildProcess + const result = new Promise<{ code: number | null; output: string }>((resolve, reject) => { + child = spawn(tsx, [lockFixture, root, version, archivePath, String(iterations), readyPath, startPath], { + env: { ...process.env, WP_CODEBOX_PLAYGROUND_CUSTOM_ARCHIVE_LEASE_MS: "300" }, + stdio: ["ignore", "pipe", "pipe"], + }) + let output = "" + child.stdout?.on("data", (chunk) => { output += String(chunk) }) + child.stderr?.on("data", (chunk) => { output += String(chunk) }) + child.once("error", reject) + child.once("close", (code) => resolve({ code, output })) + }) + await waitForPath(readyPath, child) + return { startPath, result } +} + async function waitForPath(path: string, child: ChildProcess): Promise { for (let attempt = 0; attempt < 100; attempt += 1) { if (await exists(path)) return