An abandoned archive extraction can replace a repo directory under a later writer holding the advisory lock

[beardthelion]

decompress_repo unpacks into a sibling temp dir and swaps it in with remove_dir_all(local_path) + rename(tmp_dir, local_path) (crates/gitlawb-node/src/git/tigris.rs, decompress_repo). That swap is serialized against other swaps by publish_lock, and it is genuinely atomic with respect to a corrupt archive. It is not serialized against a git process working in local_path.

The extraction runs inside tokio::task::spawn_blocking. Dropping a JoinHandle does not cancel a blocking task, so once the download is under way the swap will happen whether or not the request that started it is still alive.

acquire_write bounds the under-lock refresh with a timeout. On expiry it refuses the acquire and returns, which drops the guard, and the guard's Drop releases the per-repo advisory lock. That is the right call for the request itself: it does not know whether it has the latest tree, so it must not write and re-upload. But the abandoned extraction is still running, and the lock it was protecting is now free.

So:

1. Writer A calls acquire_write, takes the advisory lock, starts the refresh.
2. The refresh exceeds its bound. A is refused and releases the lock. A's extraction keeps running in a blocking thread.
3. Writer B takes the now-free lock, gets local_path, and starts running git.
4. A's extraction finishes and takes publish_lock, which B does not hold and has no reason to. It removes local_path and renames its own tree into place.
5. B's git work is operating in an unlinked directory. B reports success and uploads, or fails in a way that does not obviously point here.

The outcome is a 200 on a push whose objects are gone locally and reverted in object storage, or an unexplained git failure. The window is exactly as long as an extraction outlives its own request.

This is pre-existing in kind: before the timeout, an extraction could outlive its request only if the process died or the client vanished mid-transfer. What changed is that abandonment is now a designed outcome on a timer, reachable by any slow or stalled storage response rather than only by a crash. Worth stating plainly since the timeout is otherwise a strict improvement.

Two directions, both real work rather than a patch:

• Thread cancellation into the download so decompress_repo checks it after unpacking and before taking publish_lock, discarding the temp dir rather than touching local_path. Keeps the temp-dir cost, removes the swap.
• Make the pending publish observable: register the path before spawn_blocking and have acquire_write refuse (or wait) while a publish is pending for it. Bounds it without touching the transfer path, at the cost of a process-local registry that a multi-process deployment does not share.

The first is more correct, the second is cheaper. Either way acquire_write should not hand out a lock on a path with a publish in flight.

Related but not duplicates: #279 is the session-affinity defect whose fix introduced the timeout; #282 is write-lock hold time. Neither addresses the swap.

Two things I want to correct against how this might first read: publish_lock does exist and does its job, which is swap-versus-swap only, and the temp directories are not orphaned, because the blocking task runs to completion and either renames or removes its own dir. Leftover .{repo}.git.tmp-extract.{uuid} dirs only survive a process death.

Established by reading the code, not by driving it. Reproducing it needs a seam to stall an object-storage response, which the crate does not currently have.