Skip to content

Commit 545d0ff

Browse files
committed
fix: tolerate concurrent cache lock reclamation
1 parent 8cb0630 commit 545d0ff

2 files changed

Lines changed: 112 additions & 41 deletions

File tree

packages/runtime-playground/src/playground-wordpress-archive-cache.ts

Lines changed: 102 additions & 41 deletions
Original file line numberDiff line numberDiff line change
@@ -42,6 +42,8 @@ interface LeaseState {
4242
token?: string
4343
}
4444

45+
class LeaseSidecarUnsafeError extends Error {}
46+
4547
export interface PlaygroundArchiveCacheLock {
4648
path: string
4749
waitedMs: number
@@ -223,7 +225,7 @@ export async function maintainPlaygroundCustomArchiveCache(cacheDirectory = play
223225
filesystemProtected,
224226
})
225227
} catch (error) {
226-
if (!errorHasCode(error, "ENOENT")) {
228+
if (!isConcurrentDisappearance(error)) {
227229
throw error
228230
}
229231
}
@@ -265,7 +267,7 @@ export async function maintainPlaygroundCustomArchiveCache(cacheDirectory = play
265267
try {
266268
current = await lstat(candidate.path)
267269
} catch (error) {
268-
if (errorHasCode(error, "ENOENT")) {
270+
if (isConcurrentDisappearance(error)) {
269271
continue
270272
}
271273
throw error
@@ -364,6 +366,7 @@ async function tryAcquireCacheLock(lockPath: string, leaseMs: number): Promise<L
364366
} catch (error) {
365367
await directoryHandle?.close().catch(() => undefined)
366368
await rmdir(lockPath).catch(() => undefined)
369+
if (isConcurrentDisappearance(error)) return undefined
367370
throw error
368371
}
369372
}
@@ -434,41 +437,56 @@ async function cacheLockIsActive(lockPath: string, now: number, policy: Pick<Pla
434437
try {
435438
lockStat = await lstat(lockPath)
436439
} catch (error) {
437-
if (errorHasCode(error, "ENOENT")) return false
440+
if (isConcurrentDisappearance(error)) return false
438441
throw error
439442
}
440443
if (!lockStat.isDirectory() || lockStat.isSymbolicLink()) return true
441-
const directoryHandle = await openSafeLeaseDirectory(lockPath, false)
444+
let directoryHandle: FileHandle
445+
try {
446+
directoryHandle = await openSafeLeaseDirectory(lockPath, false)
447+
} catch (error) {
448+
if (isConcurrentDisappearance(error)) return false
449+
if (error instanceof LeaseSidecarUnsafeError || errorHasCode(error, "ELOOP") || errorHasCode(error, "ENOTDIR")) return true
450+
throw error
451+
}
442452
let active = false
443453
try {
444454
const anchoredDirectory = fileDescriptorPath(directoryHandle)
445-
const names = await readdir(anchoredDirectory)
455+
let names: string[]
456+
try {
457+
names = await readdir(anchoredDirectory)
458+
} catch (error) {
459+
if (isConcurrentDisappearance(error)) return false
460+
throw error
461+
}
446462
for (const name of names.sort()) {
447463
const path = join(anchoredDirectory, name)
448464
if (name !== "owner.json" && !name.endsWith(".lease.json")) {
449-
const unknownStat = await lstat(path)
465+
let unknownStat
466+
try {
467+
unknownStat = await lstat(path)
468+
} catch (error) {
469+
if (isConcurrentDisappearance(error)) continue
470+
throw error
471+
}
450472
if (unknownStat.isDirectory() || now - lockStat.mtimeMs <= policy.staleLockMs || !removeStale) {
451473
active = true
452474
} else {
453-
await unlink(path)
475+
await unlinkIfPresent(path)
454476
}
455477
continue
456478
}
457479
const state = await inspectLeaseFile(path, now, policy.staleLockMs, removeStale)
458480
if (state.active) {
459481
active = true
460-
} else if (removeStale) {
461-
await unlink(path).catch((error) => {
462-
if (!errorHasCode(error, "ENOENT")) throw error
463-
})
464482
}
465483
}
466484
if (names.length === 0 && now - lockStat.mtimeMs <= policy.staleLockMs) active = true
467485
} finally {
468486
await directoryHandle.close()
469487
if (removeStale && !active) {
470488
await rmdir(lockPath).catch((error) => {
471-
if (!errorHasCode(error, "ENOENT") && !errorHasCode(error, "ENOTEMPTY")) throw error
489+
if (!isConcurrentDisappearance(error) && !errorHasCode(error, "ENOTEMPTY")) throw error
472490
})
473491
}
474492
}
@@ -481,17 +499,34 @@ async function activeArchiveReferenceCount(archivePath: string, now: number, pol
481499
try {
482500
referencesStat = await lstat(referencesDirectory)
483501
} catch (error) {
484-
if (errorHasCode(error, "ENOENT")) return 0
502+
if (isConcurrentDisappearance(error)) return 0
485503
throw error
486504
}
487505
if (!referencesStat.isDirectory() || referencesStat.isSymbolicLink()) {
488506
diagnostics.push({ code: "custom-archive-refs-not-directory", message: "Archive reference sidecar is not a real directory and was not followed.", severity: "warning", path: referencesDirectory })
489-
if (removeStale) await unlink(referencesDirectory)
507+
if (removeStale) await unlinkIfPresent(referencesDirectory)
490508
return removeStale ? 0 : 1
491509
}
492-
const directoryHandle = await openSafeLeaseDirectory(referencesDirectory, false)
510+
let directoryHandle: FileHandle
511+
try {
512+
directoryHandle = await openSafeLeaseDirectory(referencesDirectory, false)
513+
} catch (error) {
514+
if (isConcurrentDisappearance(error)) return 0
515+
if (error instanceof LeaseSidecarUnsafeError || errorHasCode(error, "ELOOP") || errorHasCode(error, "ENOTDIR")) {
516+
diagnostics.push({ code: "custom-archive-refs-changed", message: "Archive reference sidecar changed while opening and was conservatively protected.", severity: "warning", path: referencesDirectory })
517+
return 1
518+
}
519+
throw error
520+
}
493521
const anchoredDirectory = fileDescriptorPath(directoryHandle)
494-
const names = await readdir(anchoredDirectory)
522+
let names: string[]
523+
try {
524+
names = await readdir(anchoredDirectory)
525+
} catch (error) {
526+
await directoryHandle.close()
527+
if (isConcurrentDisappearance(error)) return 0
528+
throw error
529+
}
495530
let active = 0
496531
try {
497532
for (const name of names.sort()) {
@@ -512,12 +547,12 @@ async function inspectLeaseFile(path: string, now: number, legacyStaleMs: number
512547
try {
513548
pathStat = await lstat(path)
514549
} catch (error) {
515-
if (errorHasCode(error, "ENOENT")) return { active: false }
550+
if (isConcurrentDisappearance(error)) return { active: false }
516551
throw error
517552
}
518553
if (!pathStat.isFile() || pathStat.isSymbolicLink() || pathStat.nlink !== 1) {
519554
diagnostics.push({ code: "custom-archive-ref-entry-unsafe", message: "Archive lease entry is not a singly-linked regular file and was not followed.", severity: "warning", path })
520-
if (removeUnsafe && !pathStat.isDirectory()) await unlink(path)
555+
if (removeUnsafe && !pathStat.isDirectory()) await unlinkIfPresent(path)
521556
return { active: !removeUnsafe || pathStat.isDirectory() }
522557
}
523558

@@ -533,14 +568,14 @@ async function inspectLeaseFile(path: string, now: number, legacyStaleMs: number
533568
if (owner) {
534569
const expiresAt = Date.parse(owner.expiresAt)
535570
if (Number.isFinite(expiresAt) && expiresAt > now) return { active: true, token: owner.token }
536-
if (removeUnsafe) await unlink(path)
571+
if (removeUnsafe) await unlinkIfPresent(path)
537572
return { active: false, token: owner.token }
538573
}
539574
const active = now - pathStat.mtimeMs <= legacyStaleMs
540-
if (!active && removeUnsafe) await unlink(path)
575+
if (!active && removeUnsafe) await unlinkIfPresent(path)
541576
return { active }
542577
} catch (error) {
543-
if (errorHasCode(error, "ENOENT")) return { active: false }
578+
if (isConcurrentDisappearance(error)) return { active: false }
544579
if (errorHasCode(error, "ELOOP")) {
545580
diagnostics.push({ code: "custom-archive-ref-entry-unsafe", message: "Archive lease entry became a symlink and was conservatively protected.", severity: "warning", path })
546581
return { active: true }
@@ -592,13 +627,12 @@ async function removeOrphanReferencesDirectory(path: string, now: number, policy
592627
try {
593628
sidecarStat = await lstat(path)
594629
} catch (error) {
595-
if (errorHasCode(error, "ENOENT")) return false
630+
if (isConcurrentDisappearance(error)) return false
596631
throw error
597632
}
598633
if (!sidecarStat.isDirectory() || sidecarStat.isSymbolicLink()) {
599634
diagnostics.push({ code: "custom-archive-refs-not-directory", message: "Archive reference sidecar is not a real directory and was unlinked without following it.", severity: "warning", path })
600-
await unlink(path)
601-
return true
635+
return unlinkIfPresent(path)
602636
}
603637
if (await activeReferenceCountInDirectory(path, now, policy, true, diagnostics) > 0) {
604638
return false
@@ -607,7 +641,7 @@ async function removeOrphanReferencesDirectory(path: string, now: number, policy
607641
await rmdir(path)
608642
return true
609643
} catch (error) {
610-
if (errorHasCode(error, "ENOENT") || errorHasCode(error, "ENOTEMPTY")) {
644+
if (isConcurrentDisappearance(error) || errorHasCode(error, "ENOTEMPTY")) {
611645
return false
612646
}
613647
diagnostics.push({ code: "orphan-reference-cleanup-failed", message: errorMessage(error), severity: "warning", path })
@@ -616,22 +650,35 @@ async function removeOrphanReferencesDirectory(path: string, now: number, policy
616650
}
617651

618652
async function openSafeLeaseDirectory(path: string, create: boolean): Promise<FileHandle> {
619-
if (create) {
620-
await mkdir(path).catch((error) => {
621-
if (!errorHasCode(error, "EEXIST")) throw error
622-
})
623-
}
624-
const pathStat = await lstat(path)
625-
if (!pathStat.isDirectory() || pathStat.isSymbolicLink()) {
626-
throw new Error(`Playground cache lease sidecar must be a real directory: ${path}`)
627-
}
628-
const handle = await open(path, constants.O_RDONLY | constants.O_DIRECTORY | constants.O_NOFOLLOW)
629-
const handleStat = await handle.stat()
630-
if (handleStat.dev !== pathStat.dev || handleStat.ino !== pathStat.ino) {
631-
await handle.close()
632-
throw new Error(`Playground cache lease sidecar changed while opening: ${path}`)
653+
for (let attempt = 0; attempt < (create ? 3 : 1); attempt += 1) {
654+
try {
655+
if (create) {
656+
await mkdir(path).catch((error) => {
657+
if (!errorHasCode(error, "EEXIST")) throw error
658+
})
659+
}
660+
const pathStat = await lstat(path)
661+
if (!pathStat.isDirectory() || pathStat.isSymbolicLink()) {
662+
throw new LeaseSidecarUnsafeError(`Playground cache lease sidecar must be a real directory: ${path}`)
663+
}
664+
const handle = await open(path, constants.O_RDONLY | constants.O_DIRECTORY | constants.O_NOFOLLOW)
665+
let handleStat
666+
try {
667+
handleStat = await handle.stat()
668+
} catch (error) {
669+
await handle.close()
670+
throw error
671+
}
672+
if (handleStat.dev !== pathStat.dev || handleStat.ino !== pathStat.ino) {
673+
await handle.close()
674+
throw new LeaseSidecarUnsafeError(`Playground cache lease sidecar changed while opening: ${path}`)
675+
}
676+
return handle
677+
} catch (error) {
678+
if (!create || !isConcurrentDisappearance(error) || attempt === 2) throw error
679+
}
633680
}
634-
return handle
681+
throw new Error(`Unable to create Playground cache lease sidecar: ${path}`)
635682
}
636683

637684
function fileDescriptorPath(handle: FileHandle): string {
@@ -725,6 +772,20 @@ function errorHasCode(error: unknown, code: string): boolean {
725772
return Boolean(error && typeof error === "object" && "code" in error && error.code === code)
726773
}
727774

775+
function isConcurrentDisappearance(error: unknown): boolean {
776+
return errorHasCode(error, "ENOENT") || errorHasCode(error, "ESTALE")
777+
}
778+
779+
async function unlinkIfPresent(path: string): Promise<boolean> {
780+
try {
781+
await unlink(path)
782+
return true
783+
} catch (error) {
784+
if (isConcurrentDisappearance(error)) return false
785+
throw error
786+
}
787+
}
788+
728789
function errorMessage(error: unknown): string {
729790
return error instanceof Error ? error.message.slice(0, 500) : String(error).slice(0, 500)
730791
}
@@ -734,7 +795,7 @@ async function pathExists(path: string): Promise<boolean> {
734795
await lstat(path)
735796
return true
736797
} catch (error) {
737-
if (errorHasCode(error, "ENOENT")) {
798+
if (isConcurrentDisappearance(error)) {
738799
return false
739800
}
740801
throw error

tests/playground-custom-archive-cache.test.ts

Lines changed: 10 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -257,6 +257,16 @@ try {
257257
})
258258
assert.equal(acquiredRecoveredTempLock, true, "stale heartbeat temp files from an older generation must not wedge archive acquisition")
259259

260+
const stressRoot = await mkdtemp(join(tmpdir(), "wp-codebox-playground-maintenance-stress-"))
261+
for (let iteration = 0; iteration < 25; iteration += 1) {
262+
const stressArchive = join(stressRoot, `custom-stress-${iteration}.zip`)
263+
await writeFile(stressArchive, Buffer.alloc(4_096, "s"))
264+
await utimes(stressArchive, 1, 1)
265+
await Promise.all(Array.from({ length: 4 }, () => maintainPlaygroundCustomArchiveCache(stressRoot, { mode: "apply", maxAgeMs: 1, maxCount: 0 })))
266+
}
267+
assert.deepEqual((await readdir(stressRoot)).filter((name) => name.startsWith("custom-")), [], "concurrent maintenance must converge without inspect-open disappearance failures")
268+
await rm(stressRoot, { recursive: true, force: true })
269+
260270
const samePathLockVersion = "custom-same-path-lock-release"
261271
const samePathLockDirectory = join(root, `${samePathLockVersion}.zip.lock`)
262272
let replacementLockEntry = ""

0 commit comments

Comments
 (0)