diff --git a/src/storage/README.md b/src/storage/README.md index 9ac05af5..68a25e58 100644 --- a/src/storage/README.md +++ b/src/storage/README.md @@ -39,7 +39,7 @@ - `parseEventLogContent` and `validateEventRecords` require each record to match `EventRecordSchema`, the first `seq` to be 0, and each next `seq` to be the previous one plus 1. Blank lines are skipped. [eventLogCodec.ts:parseEventLogContent] [eventLogCodec.ts:validateEventRecords] - Read and write paths in `manifests.ts` must be non-empty and absolute. Invalid JSON and schema failures throw `MANIFEST_VALIDATION_ERROR`. Other read errors throw `STORAGE_READ_ERROR`. Write failures throw `STORAGE_WRITE_ERROR`, except that `writeManifest` and `writeDestroyMarker` throw `SESSION_NOT_FOUND` when the session directory is gone. `writeDestroyMarker` writes an empty file through a temp file and a rename, like `writeManifest`, so it replaces a symlink at that path instead of writing through it, and it creates no folder. [manifests.ts:readValidatedJsonFile] [manifests.ts:writeTextFileAtomic] [manifests.ts:writeManifest] [manifests.ts:writeDestroyMarker] - `ArtifactEntrySchema` and `ArtifactManifestSchema` are `.strict()`. `kind` is `screenshot`, `snapshot`, `recording` or `video`. `capturedAtSeq` is an integer of at least -1 (-1 for a capture before the first event). The manifest `version` is the literal `1`. The manifest and entry `sessionId` must equal the session directory name. [artifactManifest.ts:ArtifactEntrySchema] [artifactManifest.ts:ArtifactManifestSchema] -- `appendArtifactWithRollback` serializes appends per session directory with an in-process `KeyedSerializer` (from `src/util/keyedSerializer.ts`). `publishArtifactWithRollback` runs its publish step in the same queue, so a failed append cannot delete a file that another capture reused in between. Inside that queue, `withArtifactManifestLock` holds `/artifacts/manifest.json.lock` around the publish step and the manifest read and write, so the host and CLI processes do not drop each other's entries. The lock file is created from a temp file with `createWithoutReplacing` (a `link()`, or an exclusive copy on a filesystem without hard links), with the temp file's timestamps set just before each attempt, and holds `{ version: 1, token, pid, process }`, where `process` is the `captureHost` record or `null`. A writer waits up to 10 s and then throws a retryable `STORAGE_WRITE_ERROR` with `lockPath` and `holderPid`. Other file errors while taking the lock throw `STORAGE_WRITE_ERROR` too (`SESSION_NOT_FOUND` when the folder is gone), and `appendArtifactWithRollback` then removes `rollbackArtifactPath` unless the manifest already lists a file of that name. That check runs without the lock, so a concurrent writer that lists the same file name between the check and the removal is left with a missing file. A lock path that is not a regular file (a FIFO or a symlink) reads as an `unknown` owner, so a writer never blocks on it. It breaks the lock when `judgeHost` says the owner is dead, or when the owner reads `unknown` (a live pid on macOS, another pid namespace, an owner without a process record on Linux, or lock contents that do not parse) and the lock's modification time is more than 60 s away from now, in either direction. To break, a writer first takes `manifest.json.lock.break` the same exclusive way, re-reads the lock, removes it only when its bytes still equal the ones it judged, and then removes its break file. A break file older than 60 s is removed by age alone. Release removes the lock only when it holds this writer's token. It tries 3 times. When all fail, the process remembers the lock, and its next acquisition on that session breaks it at once. Until then, other processes wait for it as for a live owner. A writer that takes the lock also removes `manifest.json.lock.tmp-*` and break files that are older than 60 s. Limits: a writer with an `unknown` owner that stops for over 60 s inside the section loses the lock while it still runs, so two writers can overlap. A breaker stopped for over 60 s while it holds the break file loses it the same way. [artifactManifest.ts:withArtifactManifestLock] [artifactManifest.ts:appendArtifactWithRollback] [artifactManifest.ts:publishArtifactWithRollback] +- `appendArtifactWithRollback` serializes appends per session directory with an in-process `KeyedSerializer` (from `src/util/keyedSerializer.ts`). `publishArtifactWithRollback` runs its publish step in the same queue, so a failed append cannot delete a file that another capture reused in between. Inside that queue, `withArtifactManifestLock` holds `/artifacts/manifest.json.lock` around the publish step and the manifest read and write, so the host and CLI processes do not drop each other's entries. The lock file is created from a temp file with `createWithoutReplacing` (a `link()`, or an exclusive copy on a filesystem without hard links), with the temp file's timestamps set just before each attempt, and holds `{ version: 1, token, pid, process }`, where `process` is the `captureHost` record or `null`. A writer waits up to 10 s and then throws a retryable `STORAGE_WRITE_ERROR` with `lockPath` and `holderPid`. Other file errors while taking the lock throw `STORAGE_WRITE_ERROR` too (`SESSION_NOT_FOUND` when the folder is gone), and `appendArtifactWithRollback` then removes `rollbackArtifactPath` unless the manifest already lists a file of that name. That check runs without the lock, so a concurrent writer that lists the same file name between the check and the removal is left with a missing file. A lock path that is not a regular file (a FIFO or a symlink) reads as an `unknown` owner, so a writer never blocks on it. It breaks the lock when `judgeHost` says the owner is dead, or when the owner reads `unknown` (a live pid on macOS, another pid namespace, an owner without a process record on Linux, or lock contents that do not parse) and the lock's modification time is more than 60 s away from now, in either direction. To break, a writer first announces itself with `manifest.json.lock.breakers/`, a link of its temp file in a folder that holds only announcements, and lists that folder. So the listing costs the same however many artifacts the session holds. It creates the folder without `recursive`, and the last breaker removes it with `rmdir`, which removes only an empty folder. When another breaker's announcement is there and not stale (by the break-file rule below), it removes its own and waits. It removes stale announcements, whose names no other writer uses. So breakers act one at a time: of two that announce, the one that reads later sees the other. The acting breaker takes `manifest.json.lock.break` the same exclusive way as the lock, which also keeps out older agent-tty versions, re-reads the lock, removes it only when its bytes still equal the ones it judged, and then removes its break file and its announcement. A breaker that saw another one waits a random 0.5 to 1.5 times a backoff of 5 ms that doubles with each such meeting, up to 200 ms. Breakers that keep meeting so spread their attempts until one of them is alone. This makes a meeting less likely each time. It does not rule one out. The break file and the announcement hold the breaker's record. Another writer removes them when `judgeHost` says the breaker is dead, or when the file is more than 60 s old, also while the breaker lives. Release removes the lock only when it holds this writer's token. It tries 3 times. When all fail, the process remembers the lock, and its next acquisition on that session breaks it at once. Until then, other processes wait for it as for a live owner. A writer that found the lock taken, once it holds the lock, removes each break file and announcement that it could remove as above, and each `manifest.json.lock.tmp-*` file whose owner is dead, or reads `unknown` and the file is more than 60 s old. A writer that finds the lock free does not read the artifacts folder, so the temp file of a waiter that died stays until a later writer finds the lock taken. A waiting writer whose temp file another writer removed writes it again before its next attempt. Limits: a writer with an `unknown` owner that stops for over 60 s inside the section loses the lock while it still runs, so two writers can overlap. A breaker stopped for over 60 s while it holds its announcement or the break file loses them the same way. Writers of agent-tty versions without announcements take only the break file, and removing a stale break file reads it and removes it by path in two steps. So an older writer and a newer one that both remove a dead breaker's break file at the same time can still overlap, as two older writers can. A writer with an `unknown` owner that stops for over 60 s between setting its temp file's timestamps and the `link` fails with `STORAGE_WRITE_ERROR` when another writer removed the temp file in that time. [artifactManifest.ts:withArtifactManifestLock] [artifactManifest.ts:appendArtifactWithRollback] [artifactManifest.ts:publishArtifactWithRollback] - `computeArtifactHealth` maps only `MANIFEST_VALIDATION_ERROR` to health `manifest-invalid` and rethrows other errors. It validates its result with `ArtifactHealthSummarySchema`. [artifactHealth.ts:computeArtifactHealth] - The registry read returns `[]` on a read error, invalid JSON or a schema failure. The read checks each `homes` entry on its own and drops an invalid entry, so the valid entries survive. `resolveHomeRegistryPath` ignores an empty or relative `XDG_STATE_HOME`. [homeRegistry.ts:createHomeRegistry] [homeRegistry.ts:resolveHomeRegistryPath] - `upsert` reads and then writes the registry with no lock. A code comment states that concurrent creates can race and drop an entry, which registers again on the next create. [homeRegistry.ts:createHomeRegistry] diff --git a/src/storage/artifactManifest.ts b/src/storage/artifactManifest.ts index a5137ebf..dc689b7e 100644 --- a/src/storage/artifactManifest.ts +++ b/src/storage/artifactManifest.ts @@ -1,5 +1,14 @@ import { randomUUID } from 'node:crypto'; -import { lstat, readdir, rm, stat, utimes, writeFile } from 'node:fs/promises'; +import { + lstat, + mkdir, + readdir, + rm, + rmdir, + stat, + utimes, + writeFile, +} from 'node:fs/promises'; import { basename, dirname, isAbsolute, join, resolve } from 'node:path'; import process from 'node:process'; import { setTimeout as sleep } from 'node:timers/promises'; @@ -33,6 +42,10 @@ import { KeyedSerializer } from '../util/keyedSerializer.js'; const ARTIFACT_MANIFEST_FILENAME = 'manifest.json'; const ARTIFACT_MANIFEST_LOCK_FILENAME = 'manifest.json.lock'; const MAX_LOCK_POLL_MS = 50; +// A breaker that met another waits this long times 2^(meetings - 1), up +// to MAX_MEETING_BACKOFF_MS, with jitter. +const MEETING_BACKOFF_MS = 5; +const MAX_MEETING_BACKOFF_MS = 200; const MAX_LOCK_RELEASE_ATTEMPTS = 3; const NonEmptyStringSchema = z.string().min(1); const IsoDatetimeSchema = z.iso.datetime(); @@ -268,14 +281,13 @@ function isOlderThan(state: LockFileState, limitMs: number): boolean { return Math.abs(Date.now() - state.modifiedAtMs) > limitMs; } -function isStaleLock( - lockPath: string, +// The lock, break and temp files hold their writer's record. Their owner is +// gone when judgeHost says it is dead, or when it reads unknown and the file +// is older than unknownOwnerStaleMs. +function isOwnerGone( state: LockFileState, timing: ManifestLockTiming, ): boolean { - if (unreleasedLocks.get(lockPath) === state.contents) { - return true; - } const owner = parseLockOwner(state.contents); // Without a process record the pid may belong to another pid namespace. const verdict = @@ -287,6 +299,28 @@ function isStaleLock( : verdict === 'dead'; } +// A break file is a lease: it also expires after unknownOwnerStaleMs while +// its breaker lives, so a stopped breaker cannot block every writer. +function isStaleBreakFile( + state: LockFileState, + timing: ManifestLockTiming, +): boolean { + return ( + isOwnerGone(state, timing) || isOlderThan(state, timing.unknownOwnerStaleMs) + ); +} + +function isStaleLock( + lockPath: string, + state: LockFileState, + timing: ManifestLockTiming, +): boolean { + return ( + unreleasedLocks.get(lockPath) === state.contents || + isOwnerGone(state, timing) + ); +} + async function removeIfUnchanged( path: string, contents: string | null, @@ -297,16 +331,54 @@ async function removeIfUnchanged( } } +// Breakers announce themselves in their own folder, so listing them costs +// nothing however many artifacts the session holds. +function announcementFolder(lockPath: string): string { + return `${lockPath}.breakers`; +} + +// Returns true when another breaker that is not stale announced itself. +// It removes the announcements of stale breakers on the way. Each has a +// name that no other writer uses, so removing it by path cannot remove a +// live breaker's announcement. +async function otherBreakerAnnounced( + lockPath: string, + announcementPath: string, + timing: ManifestLockTiming, +): Promise { + const folder = announcementFolder(lockPath); + for (const name of await readdir(folder)) { + const path = join(folder, name); + if (path === announcementPath) { + continue; + } + const state = await readLockFileState(path); + if (state === null) { + continue; + } + if (!isStaleBreakFile(state, timing)) { + return true; + } + await removeIfUnchanged(path, state.contents); + } + return false; +} + /** - * Breakers take `.break` first, so only one of them can remove the - * stale lock, and only while it still holds the bytes that were judged. + * A breaker first announces itself with `.breakers/` and acts only + * when no other live breaker announced, so breakers act one at a time: of + * two that announce, the one that scans later sees the other. The acting + * breaker takes `.break` the same exclusive way, which also keeps out + * older agent-tty versions, and removes the stale lock only while it still + * holds the bytes that were judged. */ async function breakStaleManifestLock( lockPath: string, temporaryPath: string, + announcementPath: string, contents: string, timing: ManifestLockTiming, -): Promise<'retry' | 'held'> { +): Promise<'retry' | 'held' | 'met'> { const judged = await readLockFileState(lockPath); if (judged === null) { return 'retry'; @@ -315,23 +387,51 @@ async function breakStaleManifestLock( return 'held'; } - const breakPath = `${lockPath}.break`; - if (!(await createWithoutReplacing(temporaryPath, breakPath))) { - const breaker = await readLockFileState(breakPath); - if (breaker !== null && isOlderThan(breaker, timing.unknownOwnerStaleMs)) { - await removeIfUnchanged(breakPath, breaker.contents); + // mkdir without recursive, so a removed session folder stays removed. + const folder = announcementFolder(lockPath); + await mkdir(folder).catch((error: unknown) => { + if (!hasErrorCode(error, 'EEXIST')) { + throw error; } - return 'held'; + }); + try { + // The name is this writer's own, so it can already exist only from this + // writer's earlier attempt. + await createWithoutReplacing(temporaryPath, announcementPath); + } catch (error) { + // Another breaker removed the empty folder after the mkdir. The next + // attempt makes it again. + if (hasErrorCode(error, 'ENOENT')) { + return 'held'; + } + throw error; } try { - await removeIfUnchanged(lockPath, judged.contents); - if (unreleasedLocks.get(lockPath) === judged.contents) { - unreleasedLocks.delete(lockPath); + if (await otherBreakerAnnounced(lockPath, announcementPath, timing)) { + return 'met'; } + const breakPath = `${lockPath}.break`; + if (!(await createWithoutReplacing(temporaryPath, breakPath))) { + const breaker = await readLockFileState(breakPath); + if (breaker !== null && isStaleBreakFile(breaker, timing)) { + await removeIfUnchanged(breakPath, breaker.contents); + } + return 'held'; + } + try { + await removeIfUnchanged(lockPath, judged.contents); + if (unreleasedLocks.get(lockPath) === judged.contents) { + unreleasedLocks.delete(lockPath); + } + } finally { + await removeIfUnchanged(breakPath, contents); + } + return 'retry'; } finally { - await removeIfUnchanged(breakPath, contents); + await rm(announcementPath, { force: true }); + // rmdir removes only an empty folder, so no announcement goes with it. + await rmdir(folder).catch(() => undefined); } - return 'retry'; } async function removeOldLockFiles( @@ -341,20 +441,52 @@ async function removeOldLockFiles( ): Promise { const directory = dirname(lockPath); const lockName = basename(lockPath); - const names = await readdir(directory).catch((): string[] => []); - for (const name of names) { - const path = join(directory, name); - if ( - path === temporaryPath || - (name !== `${lockName}.break` && !name.startsWith(`${lockName}.tmp-`)) - ) { + const folder = announcementFolder(lockPath); + const paths = [ + ...(await readdir(directory).catch((): string[] => [])) + .filter( + (name) => + name === `${lockName}.break` || name.startsWith(`${lockName}.tmp-`), + ) + .map((name) => join(directory, name)), + ...(await readdir(folder).catch((): string[] => [])).map((name) => + join(folder, name), + ), + ]; + for (const path of paths) { + if (path === temporaryPath) { continue; } + const breakFile = !basename(path).startsWith(`${lockName}.tmp-`); const state = await readLockFileState(path).catch(() => null); - if (state !== null && isOlderThan(state, timing.unknownOwnerStaleMs)) { + if (state === null) { + continue; + } + const stale = breakFile + ? isStaleBreakFile(state, timing) + : isOwnerGone(state, timing); + if (stale) { await removeIfUnchanged(path, state.contents).catch(() => undefined); } } + await rmdir(folder).catch(() => undefined); +} + +// A writer stopped for over unknownOwnerStaleMs while it waits can find that +// another writer removed its temp file, so it writes the file again. +async function refreshTemporaryFile( + path: string, + contents: string, +): Promise { + const now = new Date(); + try { + await utimes(path, now, now); + } catch (error) { + if (!hasErrorCode(error, 'ENOENT')) { + throw error; + } + await writeFile(path, contents, { flag: 'wx', mode: 0o600 }); + } } async function acquireManifestLock( @@ -362,24 +494,29 @@ async function acquireManifestLock( contents: string, timing: ManifestLockTiming, ): Promise { - const temporaryPath = `${lockPath}.tmp-${randomUUID()}`; + const id = randomUUID(); + const temporaryPath = `${lockPath}.tmp-${id}`; + const announcementPath = join(announcementFolder(lockPath), id); try { await writeFile(temporaryPath, contents, { flag: 'wx', mode: 0o600 }); const deadline = Date.now() + timing.waitMs; let pollMs = 2; + let meetings = 0; + let foundTaken = false; for (;;) { - const now = new Date(); - await utimes(temporaryPath, now, now); + await refreshTemporaryFile(temporaryPath, contents); if (await createWithoutReplacing(temporaryPath, lockPath)) { - await removeOldLockFiles(lockPath, temporaryPath, timing); + // Writers that die while they wait for or hold the lock leave lock + // files behind. Only a writer that found the lock taken scans the + // artifacts folder for them, so a free lock costs no scan. + if (foundTaken) { + await removeOldLockFiles(lockPath, temporaryPath, timing); + } return; } - const outcome = await breakStaleManifestLock( - lockPath, - temporaryPath, - contents, - timing, - ); + foundTaken = true; + // The check comes right after a failed link: a writer stopped past + // its deadline inside the break attempt still tries the link once. if (Date.now() >= deadline) { const holder = await readLockFile(lockPath).catch(() => null); throw makeCliError(ERROR_CODES.STORAGE_WRITE_ERROR, { @@ -388,8 +525,24 @@ async function acquireManifestLock( details: { lockPath, holderPid: parseLockOwner(holder)?.pid ?? null }, }); } - if (outcome === 'held') { - await sleep(pollMs); + const outcome = await breakStaleManifestLock( + lockPath, + temporaryPath, + announcementPath, + contents, + timing, + ); + if (outcome === 'met') { + // Breakers that saw each other all wait. Each meeting doubles the + // wait, and the jitter spreads their next attempts apart. + meetings += 1; + const backoffMs = Math.min( + MEETING_BACKOFF_MS * 2 ** (meetings - 1), + MAX_MEETING_BACKOFF_MS, + ); + await sleep(backoffMs * (0.5 + Math.random())); + } else if (outcome === 'held') { + await sleep(pollMs * (0.5 + Math.random())); pollMs = Math.min(pollMs * 2, MAX_LOCK_POLL_MS); } } diff --git a/test/unit/storage/artifactManifestLock.test.ts b/test/unit/storage/artifactManifestLock.test.ts index e0b3a8c6..dbd7daf0 100644 --- a/test/unit/storage/artifactManifestLock.test.ts +++ b/test/unit/storage/artifactManifestLock.test.ts @@ -1,5 +1,7 @@ +import { AsyncLocalStorage } from 'node:async_hooks'; import { spawn, spawnSync } from 'node:child_process'; import { once } from 'node:events'; +import { readdirSync } from 'node:fs'; import { mkdir, mkdtemp, @@ -7,6 +9,7 @@ import { readFile, realpath, rm, + rmdir, stat, utimes, writeFile, @@ -27,6 +30,9 @@ const fsHooks = vi.hoisted(() => ({ beforeLink: undefined as ((to: string) => Promise) | undefined, beforeRm: undefined as ((path: string) => Promise) | undefined, beforeWriteFile: undefined as ((path: string) => Promise) | undefined, + beforeUtimes: undefined as ((path: string) => Promise) | undefined, + beforeReaddir: undefined as ((path: string) => Promise) | undefined, + beforeLstat: undefined as ((path: string) => Promise) | undefined, })); vi.mock('node:fs/promises', async () => { @@ -51,6 +57,18 @@ vi.mock('node:fs/promises', async () => { ); return actual.writeFile(...args); }, + lstat: async (...args: Parameters) => { + await fsHooks.beforeLstat?.(String(args[0])); + return actual.lstat(...args); + }, + readdir: async (...args: Parameters) => { + await fsHooks.beforeReaddir?.(String(args[0])); + return actual.readdir(...args); + }, + utimes: async (...args: Parameters) => { + await fsHooks.beforeUtimes?.(String(args[0])); + return actual.utimes(...args); + }, }; }); @@ -119,6 +137,9 @@ describe('artifact manifest lock', () => { fsHooks.beforeLink = undefined; fsHooks.beforeRm = undefined; fsHooks.beforeWriteFile = undefined; + fsHooks.beforeUtimes = undefined; + fsHooks.beforeReaddir = undefined; + fsHooks.beforeLstat = undefined; vi.useRealTimers(); await rm(home, { recursive: true, force: true }); }); @@ -224,7 +245,7 @@ describe('artifact manifest lock', () => { }, ); - it('removes old lock temp and break files when it takes the lock', async () => { + it('removes lock temp and break files whose owner is gone once it had to wait for the lock', async () => { const artifacts = join(sessionDirectory, 'artifacts'); const twoMinutesAgo = new Date(Date.now() - 120_000); for (const name of [ @@ -234,13 +255,338 @@ describe('artifact manifest lock', () => { await writeFile(join(artifacts, name), 'left over'); await utimes(join(artifacts, name), twoMinutesAgo, twoMinutesAgo); } + await writeFile( + join(artifacts, 'manifest.json.lock.tmp-dead'), + await deadOwnerRecord(), + ); await writeFile(join(artifacts, 'manifest.json.lock.tmp-new'), 'in use'); + await writeFile(lockPath, await deadOwnerRecord()); + freezeLockClock(); await withArtifactManifestLock(sessionDirectory, () => Promise.resolve()); expect(await readdir(artifacts)).toEqual(['manifest.json.lock.tmp-new']); }); + it('does not read the artifacts folder for leftovers when it finds the lock free', async () => { + const artifacts = join(sessionDirectory, 'artifacts'); + const twoMinutesAgo = new Date(Date.now() - 120_000); + await writeFile(join(artifacts, 'manifest.json.lock.tmp-old'), 'left over'); + await utimes( + join(artifacts, 'manifest.json.lock.tmp-old'), + twoMinutesAgo, + twoMinutesAgo, + ); + + await withArtifactManifestLock(sessionDirectory, () => Promise.resolve()); + + expect(await readdir(artifacts)).toEqual(['manifest.json.lock.tmp-old']); + }); + + it('removes the break file of a breaker that died and breaks the lock at once', async () => { + await writeFile(lockPath, await deadOwnerRecord()); + await writeFile(`${lockPath}.break`, await deadOwnerRecord()); + freezeLockClock(); + + await expect( + withArtifactManifestLock( + sessionDirectory, + () => Promise.resolve('ran'), + FAST, + ), + ).resolves.toBe('ran'); + expect(await readdir(join(sessionDirectory, 'artifacts'))).toEqual([]); + }); + + it('keeps the break file of a live breaker until it is older than unknownOwnerStaleMs', async () => { + const breakPath = `${lockPath}.break`; + await writeFile(lockPath, await deadOwnerRecord()); + await writeFile(breakPath, ownerRecord(process.pid, true)); + + await expect( + withArtifactManifestLock(sessionDirectory, () => Promise.resolve(), FAST), + ).rejects.toMatchObject({ code: ERROR_CODES.STORAGE_WRITE_ERROR }); + expect(await readFile(breakPath, 'utf8')).toBe( + ownerRecord(process.pid, true), + ); + + const twoMinutesAgo = new Date(Date.now() - 120_000); + await utimes(breakPath, twoMinutesAgo, twoMinutesAgo); + freezeLockClock(); + await expect( + withArtifactManifestLock( + sessionDirectory, + () => Promise.resolve('ran'), + FAST, + ), + ).resolves.toBe('ran'); + expect(await readdir(join(sessionDirectory, 'artifacts'))).toEqual([]); + }); + + it("lets one breaker act at a time when one stops while it removes a dead breaker's break file", async () => { + // The trace: W2 judges a dead breaker's break file and stops before its + // rm. W1 then takes a new break file, reads the dead owner's lock and + // stops before its rm. W2 resumes, removes W1's break file by path, + // breaks the lock and enters. W1 resumes and removes W2's fresh lock. + await writeFile(lockPath, await deadOwnerRecord()); + await writeFile(`${lockPath}.break`, await deadOwnerRecord()); + const writer = new AsyncLocalStorage(); + const deferred = (): { promise: Promise; resolve: () => void } => { + let resolve = (): void => undefined; + const promise = new Promise((done) => { + resolve = done; + }); + return { promise, resolve }; + }; + const w2AtBreakRm = deferred(); + const resumeW2 = deferred(); + const w1AtLockRm = deferred(); + const resumeW1 = deferred(); + const firstInside = deferred(); + const secondInside = deferred(); + const leave = deferred(); + let w2Paused = false; + let w2Resumed = false; + let w1Paused = false; + fsHooks.beforeRm = async (path) => { + const who = writer.getStore(); + if (who === 'W2' && path === `${lockPath}.break` && !w2Paused) { + w2Paused = true; + w2AtBreakRm.resolve(); + await resumeW2.promise; + } + if (who === 'W1' && path === lockPath && !w2Resumed && !w1Paused) { + w1Paused = true; + w1AtLockRm.resolve(); + await resumeW1.promise; + } + }; + let inside = 0; + let maxInside = 0; + const update = async (): Promise => { + inside += 1; + maxInside = Math.max(maxInside, inside); + (inside === 1 ? firstInside : secondInside).resolve(); + await leave.promise; + inside -= 1; + }; + const contend = (who: string): Promise => + writer.run(who, () => + withArtifactManifestLock(sessionDirectory, update, { + waitMs: 10_000, + unknownOwnerStaleMs: 60_000, + }), + ); + + const w2 = contend('W2'); + await w2AtBreakRm.promise; + const w1 = contend('W1'); + await Promise.race([w1AtLockRm.promise, sleep(300)]); + w2Resumed = true; + resumeW2.resolve(); + await firstInside.promise; + resumeW1.resolve(); + await Promise.race([secondInside.promise, sleep(300)]); + leave.resolve(); + await Promise.all([w1, w2]); + + expect(maxInside).toBe(1); + expect(await readdir(join(sessionDirectory, 'artifacts'))).toEqual([]); + }); + + it('lists only the announcements while it announces itself, not the artifacts folder', async () => { + // A listing of a large artifacts folder while the announcement exists + // made breakers that started together keep seeing each other. + const artifacts = join(sessionDirectory, 'artifacts'); + await writeFile(lockPath, await deadOwnerRecord()); + const announced = (): boolean => { + const inFolder = (path: string): string[] => { + try { + return readdirSync(path); + } catch { + return []; + } + }; + return ( + inFolder(`${lockPath}.breakers`).length > 0 || + inFolder(artifacts).some((name) => + name.startsWith('manifest.json.lock.breaker'), + ) + ); + }; + const listedWhileAnnounced: string[] = []; + fsHooks.beforeReaddir = (path) => { + if (announced()) { + listedWhileAnnounced.push(path); + } + return Promise.resolve(); + }; + + await withArtifactManifestLock(sessionDirectory, () => Promise.resolve()); + fsHooks.beforeReaddir = undefined; + + expect(listedWhileAnnounced).not.toHaveLength(0); + expect(listedWhileAnnounced).not.toContain(artifacts); + expect(await readdir(artifacts)).toEqual([]); + }); + + it("removes a dead breaker's fresh break file once it had to wait for a live lock", async () => { + await writeFile(lockPath, ownerRecord(process.pid, true)); + await writeFile(`${lockPath}.break`, await deadOwnerRecord()); + freezeLockClock(); + let refreshes = 0; + fsHooks.beforeUtimes = async (path) => { + if (path.startsWith(`${lockPath}.tmp-`) && (refreshes += 1) === 2) { + await rm(lockPath); + } + }; + + await withArtifactManifestLock(sessionDirectory, () => Promise.resolve()); + + expect(refreshes).toBe(2); + expect(await readdir(join(sessionDirectory, 'artifacts'))).toEqual([]); + }); + + it('takes a lock that was let go while it was stopped past its deadline', async () => { + // A writer stopped (SIGSTOP) while it judges the lock resumes after its + // deadline. The owner let go in the meantime, so the lock is free. + await writeFile(lockPath, ownerRecord(process.pid, true)); + freezeLockClock(); + fsHooks.beforeLstat = async (path) => { + if (path === lockPath) { + fsHooks.beforeLstat = undefined; + vi.setSystemTime(Date.now() + 60_000); + await rm(lockPath); + } + }; + + await expect( + withArtifactManifestLock( + sessionDirectory, + () => Promise.resolve('ran'), + FAST, + ), + ).resolves.toBe('ran'); + }); + + it('announces itself again when another breaker removed the empty folder before its link', async () => { + const folder = `${lockPath}.breakers`; + await writeFile(lockPath, await deadOwnerRecord()); + freezeLockClock(); + let removed = false; + fsHooks.beforeLink = async (to) => { + if (to.startsWith(`${folder}/`) && !removed) { + removed = true; + await rmdir(folder); + } + }; + + await expect( + withArtifactManifestLock( + sessionDirectory, + () => Promise.resolve('ran'), + FAST, + ), + ).resolves.toBe('ran'); + expect(removed).toBe(true); + expect(await readdir(join(sessionDirectory, 'artifacts'))).toEqual([]); + }); + + it('waits longer each time it meets another breaker', async () => { + // A live breaker's announcement that stays: every attempt meets it. + const folder = `${lockPath}.breakers`; + await writeFile(lockPath, await deadOwnerRecord()); + await mkdir(folder); + await writeFile(join(folder, 'live'), ownerRecord(process.pid, true)); + let listings = 0; + fsHooks.beforeReaddir = (path) => { + if (path === folder) { + listings += 1; + } + return Promise.resolve(); + }; + + await expect( + withArtifactManifestLock(sessionDirectory, () => Promise.resolve(), { + waitMs: 1_500, + unknownOwnerStaleMs: 60_000, + }), + ).rejects.toMatchObject({ code: ERROR_CODES.STORAGE_WRITE_ERROR }); + // A backoff from 5 ms that doubles up to 200 ms allows about 12 + // attempts in 1.5 s. A backoff that stays at 5 ms allows over 100. + expect(listings).toBeGreaterThan(3); + expect(listings).toBeLessThan(40); + }); + + it("removes a dead breaker's announcement and breaks the lock at once", async () => { + const folder = `${lockPath}.breakers`; + await writeFile(lockPath, await deadOwnerRecord()); + await mkdir(folder); + await writeFile(join(folder, 'dead'), await deadOwnerRecord()); + + await expect( + withArtifactManifestLock(sessionDirectory, () => Promise.resolve('ran'), { + waitMs: 2_000, + unknownOwnerStaleMs: 60_000, + }), + ).resolves.toBe('ran'); + expect(await readdir(join(sessionDirectory, 'artifacts'))).toEqual([]); + }); + + it("sweeps a dead breaker's announcement once it had to wait for a live lock", async () => { + // The lock is live, so this writer never breaks and never lists the + // announcements. Only the sweep can remove the dead one. + const folder = `${lockPath}.breakers`; + await writeFile(lockPath, ownerRecord(process.pid, true)); + await mkdir(folder); + await writeFile(join(folder, 'dead'), await deadOwnerRecord()); + freezeLockClock(); + let refreshes = 0; + fsHooks.beforeUtimes = async (path) => { + if (path.startsWith(`${lockPath}.tmp-`) && (refreshes += 1) === 2) { + await rm(lockPath); + } + }; + + await withArtifactManifestLock(sessionDirectory, () => Promise.resolve()); + + expect(refreshes).toBe(2); + expect(await readdir(join(sessionDirectory, 'artifacts'))).toEqual([]); + }); + + it('writes its temp file again when another writer removed it during the wait', async () => { + await writeFile(lockPath, ownerRecord(process.pid, true)); + freezeLockClock(); + let refreshes = 0; + // On the second attempt, a writer that took the lock while this one was + // stopped has removed this one's temp file and let go of the lock. + fsHooks.beforeUtimes = async (path) => { + if (!path.startsWith(`${lockPath}.tmp-`)) { + return; + } + refreshes += 1; + if (refreshes === 2) { + await rm(path); + await rm(lockPath); + } + }; + + await expect( + withArtifactManifestLock( + sessionDirectory, + async () => { + const owner = JSON.parse(await readFile(lockPath, 'utf8')) as { + pid: number; + }; + return owner.pid; + }, + FAST, + ), + ).resolves.toBe(process.pid); + expect(refreshes).toBe(2); + expect(await readdir(join(sessionDirectory, 'artifacts'))).toEqual([]); + }); + it('breaks a lock whose owner is gone', async () => { await writeFile(lockPath, await deadOwnerRecord()); freezeLockClock();