[fazxes]

Description

Every in-band write takes a proper-lockfile lock on the run's count file (withFenceLock, .locks/runs/<runId>.in-band, stale: 30_000). proper-lockfile's lock is a directory that records nothing about its holder, so when a process is killed while it holds the lock, the next in-band writer on the run, usually the orchestrator taking the run over after the crash, waits until the directory is 30 seconds old.

libfx's crash campaign, which SIGKILLs processes at random points of durable sessions on world-local, hits this often. In 30 trials passing world-local 5.1.1 to libfx's world(), 6 left the lock behind, and each took 31.7 to 37.1 s instead of 1 to 7 s.

This change gives the fence lock an fs for proper-lockfile, lockHolderFs() in the new src/lock-holder.ts:

• The lock names its holder. Taking the lock also makes a directory inside it, holder-<pid>-<host>-<token>, in the same synchronous step, so a process killed while taking the lock almost never leaves it unnamed. <host> is a hash of the hostname and, on Linux, the boot id and PID namespace, so two containers that share a hostname and a data directory are different hosts. The name is made before proper-lockfile reads the lock's mtime, which making it changes.
• A dead holder's lock is taken at once. A writer that finds the lock held reads the name. When it is a process on this host that no longer runs (process.kill(pid, 0) fails with ESRCH), the writer removes the name, then the empty lock directory, and takes the lock.
• Anything else waits, as before. A holder that still runs, a pid in use again by another process, another host, or a lock with no name (a holder killed before it named itself, or an older world-local) goes stale after 30 s, as today.
• Removing a lock removes its name. A holder releasing removes only its own name, so a lock that another writer has since taken and named stays. A writer removing a stale lock removes every name in it first, since proper-lockfile removes the lock with rmdir.

Why the fence stays sound:

• Removing the dead holder's name is what decides the takeover. It succeeds only while the lock directory is still the dead holder's, since a later lock carries a name with a new random token, and only for one writer. A taker that read a holder's name before that holder released, and before a new writer took the lock, fails to remove the old name and waits.
• Of several writers racing for a dead holder's lock, one removes the name. The others go back to proper-lockfile's usual retries, and mkdir decides who takes the lock next.
• The pending mark is unchanged: a writer killed between its check and its publish is still counted by the next writer through the lock.
• A lock older than stale is removed by proper-lockfile as before, and two writers removing one stale lock race as they do today.

Compatibility: a world-local without this change cannot remove a stale lock that names its holder, because its rmdir fails on the non-empty directory, and it fails with "Could not acquire the in-band fence lock" after its retries. That needs a process with this change to die holding the lock while an older process on another host, or one that cannot take it over, writes the same run next. Writers with this change take over a dead holder's lock on their host at once.

Scope: only the in-band fence lock. The hook token claim lock (withHookTokenClaimLock, stale: 30_000) and the stream index lock (addToRunStreams, stale: 10_000) have the same shape and could pass lockHolderFs() too; that is left for a follow-up.

How did you test your changes?

• New src/lock-holder.test.ts. A forked process (src/lock-holder.child.mjs, run with Node's type stripping like in-band-fence.child.mjs) holds a lock taken with lockHolderFs():

  • the lock names its holder, and the name goes with the lock;
  • a lock whose holder was SIGKILLed is taken within 1 s, with the fence lock's settings;
  • a lock whose holder still runs is not taken (ELOCKED), and is once the holder releases it;
  • a killed holder's lock that names a pid in use, another host, or no holder is not taken;
  • a stale lock that names its holder is still removed when it goes stale;
  • of 8 writers racing for a killed holder's lock, one at a time holds it.

• src/storage/in-band-fence.test.ts, across OS processes:

  • after a process is killed while it holds a run's fence lock, the next in-band write is accepted within 2 s;
  • after that, of in-band writers in four processes holding one count, exactly one is accepted.

  Without the change to events-storage.ts, both time out at 30 s.

• vitest run src in packages/world-local: 28 files, 681 tests pass. tsc --noEmit passes. biome check on the changed files reports only the six existing complexity and non-null warnings in events-storage.ts.

• End to end, libfx's crash campaign (30 trials, seeds 1000 to 1029, world-local passed to world()), with world-local 5.1.1 and then with this change built into it. Every trial passed its invariants both times.

  	trials whose kill left the fence lock behind	those trials took	whole campaign
  5.1.1	6	31.7 to 37.1 s	295 s
  this change	4	2.6 to 13.8 s	150 s

  The slowest of those, 13.8 s, waited for libfx's own 15 s session lease, which applies to a World without liveness, not for the lock.

PR Checklist - Required to merge

• 📦 pnpm changeset was run to create a changelog for this PR
  • Use the correct semver bump type: patch for bug fixes, minor for new features, major for breaking changes.
  • Use pnpm changeset --empty if you are changing documentation or workbench apps
• 🔒 DCO sign-off passes (run git commit --signoff on your commits)
• 📝 Ping @vercel/workflow in a comment once the PR is ready, and the above checklist is complete

[changeset-bot]

🦋 Changeset detected

Latest commit: 94ce42b

The changes in this PR will be included in the next version bump.

This PR includes changesets to release 19 packages

Name	Type
@workflow/world-local	Patch
@workflow/cli	Patch
@workflow/core	Patch
@workflow/vitest	Patch
@workflow/web	Patch
@workflow/world-postgres	Patch
workflow	Patch
@workflow/world-testing	Patch
@workflow/builders	Patch
@workflow/next	Patch
@workflow/nitro	Patch
@workflow/web-shared	Patch
@workflow/astro	Patch
@workflow/nest	Patch
@workflow/rollup	Patch
@workflow/sveltekit	Patch
@workflow/vite	Patch
@workflow/nuxt	Patch
@workflow/world-vercel	Patch

Not sure what this means? Click here to learn what changesets are.

Click here if you're a maintainer who wants to add another changeset to this PR

[vercel]

@fazxes is attempting to deploy a commit to the Vercel Labs Team on Vercel.

A member of the Team first needs to authorize it.

[VaguelySerious]

(AI) Preliminary review:

AI review: no blocking issues

[VaguelySerious]

AI Review: Note

Whether this is safe depends on HOST being unique per PID namespace. Before this change, a Linux process that couldn't read /proc/sys/kernel/random/boot_id or /proc/self/ns/pid hashed '' for that part. Two containers in that state, sharing a hostname (e.g. --network host) and a data directory, got the same HOST. A kill(pid, 0) that returns ESRCH for the other container's live pid would then let one take over a live holder's lock, and two writers would be in the fence at once.

I pushed 01e2fa0 for this. On Linux, a value that can't be read is now replaced with random bytes, so that process neither takes over a lock nor has its own taken over, and its locks go stale after 30 s as they do today. Non-Linux platforms are unchanged (hostname only). I checked that HOST differs across unshare -Urpf here.

[VaguelySerious]

(AI) Addressed in 01e2fa0 (random bytes on Linux when boot id or PID namespace can't be read). No further change.

[VaguelySerious]

AI Review: Note

The description covers the compatibility gap, but it is permanent, not just a failure after retries. I reproduced it: with the default fs, proper-lockfile 4.1.2 hits ENOTEMPTY on a stale lock that holds a holder-… entry, and it keeps hitting it on every acquisition. Until a writer with this change comes along, every in-band write to that run from an older world-local fails with "Could not acquire the in-band fence lock".

A reboot leads to the same state. The boot id changes, so a newer writer can't take over the lock (it waits for it to go stale and removes it, which is fine), but an older writer stays stuck.

world-local is a dev World and the trigger needs SIGKILL, OOM or power loss, so I don't think this blocks. It may be worth a sentence in the changeset, since downgrading world-local over an existing data directory is how it would show up.

[VaguelySerious]

(AI) Confirmed in proper-lockfile 4.1.2: removeLock passes the ENOTEMPTY from rmdir back to acquireLock, and nothing removes the holder-… entry, so it repeats on every acquisition. Added a sentence about the downgrade case to the changeset in 2257926.

[VaguelySerious]

AI Review: Nit

proper-lockfile caches the mtime precision on the fs object it is given (mtime-precision.js, via a private symbol). A fresh lockHolderFs() per lock() call misses that cache every time, so each acquisition does an extra utimes + stat probe on top of the two extra mkdir/rmdir calls for the name.

I measured 500 sequential in-band writes on tmpfs: about 1.20 ms per write with this change against 1.05 ms without (~0.15 ms, ~12%). That's negligible for a local World, so this is just for the record, not a request.

[VaguelySerious]

(AI) Leaving as is. lockHolderFs() carries per-acquisition state (own token, held), so sharing one instance to keep proper-lockfile's mtime-precision cache would need that state keyed by lock path. Not worth it for ~0.15 ms per write on a local World.

[VaguelySerious]

(AI) Closing as superseded by #5000, which landed on main on 10-28 and fixes the same 30 s stall after a fence-lock holder is SIGKILLed (#4967). #5000 moved the fence lock off proper-lockfile into storage/fence-lock.ts, which records its holder in owner.json inside the lock directory and takes over a dead holder's lock at once. The events-storage.ts change here no longer has a proper-lockfile call to attach to, and a second holder format in the same package would not recognize the first.

Two parts of this PR would still be useful as follow-ups:

• The two in-band-fence.test.ts cases that kill a holder and then write through storage.events.create. fix(world-local): reduce in-band fence lock recovery from 30s to <1s after SIGKILL #5000 tests withFenceFileLock directly, not at the storage level. Porting them would mean holding the lock through fence-lock.child.mjs instead of lock-holder.child.mjs.
• The hook token claim lock (withHookTokenClaimLock) and stream index lock (streamer.ts) still use proper-lockfile and still stall for their full staleness. Moving them onto the same mechanism as the fence lock would fix that.

Thanks for the detailed investigation and the crash-campaign numbers.