[Mikola Lysenko (mikolalysenko)]

Problem

update::swap::tests::update_lock_is_exclusive_and_released_on_drop flakes. On 2026-10-20 at 18:13 UTC it evicted #1591 from the merge queue: the coverage job panicked at crates/socket-patch-core/src/update/swap.rs:467, the unwrap() on the re-acquire after drop(first), which returned Err(InProgress). The other 6547 core tests passed. The same code passed coverage in #1617's queue run (39471359983) and on #1591's own CI, so the failure has nothing to do with #1591's diff. Each eviction like this costs a full queue rebuild for every entry behind it.

Root cause

A flock belongs to the open file description, not to the fd. While any other thread forks (core tests spawn git, package managers and so on in parallel), the child holds a copy of the lock fd until its exec closes it (O_CLOEXEC). UpdateLock released the lock only by closing its own fd, so if a child was inside that fork-to-exec window when the guard dropped, the lock stayed held and the next acquire saw InProgress. Coverage instrumentation makes that window longer, so the coverage job hits it most.

It's a real bug in the guard as well, not just the test. In production the window is microseconds and nothing re-acquires inside it, though.

Fix

impl Drop for UpdateLock calls FileExt::unlock (flock(LOCK_UN)) before the fd closes. LOCK_UN on any copy of the description releases the lock for all copies, so a child that is still forking can no longer keep the lock. On Windows it's the matching UnlockFile. It's best effort: the close that follows still releases the lock if the unlock fails.

New regression test update_lock_released_on_drop_while_other_threads_spawn (unix only, in the same update_state_dir_env serial group): two threads spawn true in a loop while the test acquires and drops the lock 1000 times.

Proof

• Without the fix, the new test failed 3 of 3 runs, with the lock still held after drop 277, 152 and 605 times out of 1000. A 5000-iteration scratch version failed 1225 to 2734 times out of 5000.
• With the fix: 0 out of 1000 on every run. The whole update_lock group passed 30 of 30 loops, and update:: (88 tests) passes.
• cargo clippy -p socket-patch-core --all-features -- -D warnings (the CI invocation) is clean. rustfmt ran on the touched file only.

Where tests run

No test was removed or moved. The new test runs wherever core lib tests already run (test and coverage jobs).

/code-review high

Fixed in 1028c9e:

• The stress test now stops its spawner threads and restores `SOCKET_UPDATE_STATE_DIR` from a Drop guard, so a panic mid-test can't leave fork traffic or a dangling state dir behind.
• The stress test asserts that a process was actually spawned before it measures, so it can't pass without exercising anything.
• `concurrent_update_lock_held` (CLI e2e) released its externally held `update.lock` by closing the file alone, the same race. It now calls `FileExt::unlock` before the drop.

Not fixed here:

• `apply_lock.rs` `LockGuard` also releases by closing only. That's production apply-lock behaviour with unlink and identity checks, outside a CI flake fix, so I'm leaving it for a separate PR.
• Suggestion to use std `File::unlock` instead of the fs2 `FileExt::unlock`: acquisition goes through fs2's `try_lock_exclusive`, so the release uses the same API, fully qualified on purpose to avoid the std inherent method. Moving both to std is a separate cleanup.

---

Generated by Claude Code

---

Note

Medium Risk
Changes self-update concurrency locking behavior on Unix; the fix is narrowly scoped to release semantics and is covered by new stress and e2e tests.

Overview
Fixes a flock release bug where dropping UpdateLock only closed the fd, so a concurrently forked child could keep the lock until exec and the next acquire would see update_in_progress.

UpdateLock now calls FileExt::unlock in Drop before the fd closes, so LOCK_UN releases the advisory lock for all descriptor copies. The e2e concurrent_update_lock_held test unlocks its externally held update.lock the same way before retrying an update.

Adds a Unix regression test that spawns processes from background threads while acquiring and dropping the lock 1000 times, with cleanup on panic so parallel tests are not polluted.

^{Reviewed by Cursor Bugbot for commit 1028c9e. Configure here.}

---

Generated by Claude Code

[Mikola Lysenko (mikolalysenko)]

bugbot run

---

Generated by Claude Code

[cursor]

Stale Bugbot comment from a previous run.

[Mikola Lysenko (mikolalysenko)]

Labeled Ready for review (burn-down agent).

• Head: a06ee08. Mergeable, no conflicts; CHANGELOG.md not touched.
• CI: 54/54 checks green (35 success, 19 skipped). No main-wide failures.
• Bugbot: reviewed a06ee08, no findings; no open review threads.
• Reviewer note: the PR's own status said it still wants a /code-review pass before it leaves for human review; worth a careful look at the explicit unlock-on-drop change.
• Slack: not announced this run (no Slack send tool available); the next run will retry.

---

Generated by Claude Code

[Mikola Lysenko (mikolalysenko)]

bugbot run

---

Generated by Claude Code

[cursor]

✅ Bugbot reviewed your changes and found no new issues!

Comment @cursor review or bugbot run to trigger another review on this PR

^{Reviewed by Cursor Bugbot for commit 1028c9e. Configure here.}

[Mikola Lysenko (mikolalysenko)]

Ready for review re-confirmed at the new head (burn-down agent).

• Head: 1028c9e. It is mergeable with no conflicts, and CHANGELOG.md is not touched.
• CI: 54/54 green (35 success, 19 skipped). No main-wide failures.
• Bugbot reviewed 1028c9e and found no issues. No review threads are open.
• What changed since the label at a06ee08: test-only hardening. The update-lock regression test and its e2e twin in swap.rs and self_update_failures_e2e.rs changed; the Drop fix is unchanged.

---

Generated by Claude Code

[Mikola Lysenko (mikolalysenko)]

Final review brief (final reviewer agent)

What it does. UpdateLock now calls flock(LOCK_UN) in its Drop before the fd closes. Before, it released the update lock only by closing its fd. A child forked by another thread holds a copy of that fd until its exec, so the lock could stay held for that window. That race made update_lock_is_exclusive_and_released_on_drop flaky, and it evicted #1591 from the merge queue. The CLI e2e test that holds the lock from outside now releases it the same way.

Risk: low. Two files, about 15 lines of production code: a best-effort unlock right before the close that already released the lock. No API change. The unlock uses fs2's FileExt, the same API that takes the lock, so on Windows it maps to UnlockFile.

Look here

• swap.rs:35: the new Drop impl
• swap.rs:86: the guard now keeps a named file field
• swap.rs:497: the regression test (two threads spawn true in a loop while the test acquires and drops the lock 1000 times; a Drop guard handles cleanup)
• self_update_failures_e2e.rs:571:`` the e2e test unlocks before it drops the lock

Verified

• Read the full diff and description. The code matches the description, there are no unrelated edits, and CHANGELOG.md is untouched.
• cargo test -p socket-patch-core --lib update_lock: 4 passed.
• With the Drop unlock removed locally, the new test fails (lock still held 42/1000 and 107/1000 times in two runs), so it really covers the fix.
• No other code reads the renamed field.
• CI 54/54 on 1028c9e (35 success, 19 skipped). Cursor Bugbot: success, no findings. No open review threads.

Changes I made: none.

Open questions: none blocking. As the PR description says, apply_lock.rs's LockGuard still releases only by closing. Fixing that is left for a follow-up PR.

Auto-merge is armed, so approving sends this straight to the merge queue.

---

Generated by Claude Code