Skip to content

F2-L2: Fence zombie writers — sandboxes.version epoch stamped into the script-tx #131

Description

@Hazzng

Verdict: Real — High (confidence: high) — the complete F2 fix (fencing)

The original review's mechanism is wrong, but the lost-update conclusion is real via a different path. This issue adds the fencing token that closes it for good — and the same primitive closes #136 (F7) and #139 (F9b)'s collision class.

Corrected mechanism (important)

  • The review claimed appendFile reads in tx1 / writes in tx2 and that the advisory lock doesn't serialize the composite. False in the exec path: #withTx and #withBareTx both route to the same #scriptTx, so getBlob (sql-fs.ts:806) and writeFileComposite (:819) run in one advisory-locked transaction. The two-tx interleave cannot occur, and the review's fix-(3) ("make appendFile single-transaction") is a no-op against a real bug.
  • The real lost update: appendFile captures its base from the in-memory pathCache (existing.contentSha256, sql-fs.ts:805) before any lock, and getBlob is content-addressed (reads by hash, postgres.ts:616). A zombie writer A (lease lapsed) reads its stale base even after a live writer B (other replica) committed a newer dirent, then replaces the dirent with new-inode(stale-base + append), silently erasing B's commit. The advisory lock can't prevent it because the base was captured in memory and the read is by content hash.
  • Reachability (narrow): A's Redis writer flag must expire (PX leaseMs, default 60 s) before A opens its script-tx (advisory lock acquired lazily on first write via #openScriptTxsetSandboxContextWithLock, postgres.ts:265). Window = A holds the distributed lock → runs an event-loop-blocking CPU/WASM prefix (heartbeat setTimeout can't fire, lease silently expires) → has not yet issued its first write.

Fix to ship (Layer 2 — the only complete fix)

  • Add version BIGINT NOT NULL DEFAULT 0 to the sandboxes table (new migration, per dialect).
  • Pin the expected epoch at script-scope open (derive from the version read under the lease).
  • Add UPDATE sandboxes SET version = version + 1 WHERE id = $s AND version = $expected inside the advisory-locked ctx CTE of the composite writes. Zero rows updated ⇒ abort the tx ⇒ the zombie's COMMIT fails in Postgres, which doesn't share the zombie's delusions.
  • Must respect set_config('app.sandbox_id')/RLS and be wrapped in runTrustedDbAsync (defense-in-depth).

Acceptance criteria

  • Integration test: two writers, force lease expiry on A before its first write, B commits, A's COMMIT must fail (epoch mismatch); B's append survives.
  • RLS / SET LOCAL / transaction-pooling unaffected; defense-in-depth wrapping preserved.
  • Migration applies cleanly on boot (src/api/migrations.ts) for all configured dialects.

Parallelization

Start now — no hard blocker (pairs with #130 L1). Design the sandboxes.version epoch so it is reused by #136 (tombstone/sandbox-gone) and #139 (collision-class closure) — see "Cluster C" in the verification doc.


References: verified analysis thoughts/shared/research/2026-06-13_distributed-flaws-verification.md · original review thoughts/shared/research/2026-06-10_distributed-design-review.md

Metadata

Metadata

Assignees

No one assigned

    Labels

    area:distributedDistributed locking / caching / coherencebugSomething isn't workingseverity:highHigh severity

    Projects

    No projects

    Milestone

    No milestone

    Relationships

    None yet

    Development

    No branches or pull requests

    Issue actions