alkblobs/tasks/f2-cancellation-guards.md

id, name, status, depends_on, scope, risk, impact, level, tags

id	name	status	depends_on	scope	risk	impact	level	tags
f2-cancellation-guards	F-2 + N-6 — cancellation/panic Drop guards: PgJointTx, the sweep lease's windows, the in-process lease	completed		moderate	medium	component	implementation	wave-9 review-001 correctness postgres cancellation

Description

Review-001 F-2 with N-6 riding (same hazard class — the plan's §Wave 9 pairs them). Post-split homes:

• PgJointTx has no Drop guard (src/postgres/mod.rs:328; the impl lives in src/postgres/joint.rs): the fleet joint-tx driver's defense against idle-in-transaction poisoning of pooled sessions is the store core's catch-propagate-terminate on error paths — but a cancellation (task abort, select!, a timeout dropping the future between begin_joint_tx and pin_commit) drops the PgJointTx without terminating: the checked-out Object returns to the pool holding an open transaction, poisoning every later checkout from that session (deadpool's Fast recycle confirmed not to reset tx state — the recorded pitfall). PgLoStaging already solves this exact hazard with a best-effort spawned-ROLLBACK Drop (src/pg_lo/staging.rs:278) — give PgJointTx the same guard, aliasing that defense.
• The sweep lease's cancellation windows (src/postgres/pins.rs:282-331): cancelled between pg_try_advisory_lock and *lease = Some(client) — or after take() before the unlock completes — leaves the advisory lock held on an idle pooled session. Same treatment (a Drop guard or equivalent defense; the exact shape is the task's design point, recorded).
• N-6 — the sweep leases are not panic-safe: a panic inside the sweep body (a panicking consumer-supplied protect callback is the plausible route) skips end_sweep() (the in-process lease sticks true forever — src/gc.rs:258/569 and the sweep body in src/store/sweep.rs) and the fleet sweep_lock_release (a pooled session keeps the advisory lock). Drop-guards for both.

The test (the plan's named shape): cancel-the-future-mid-joint-tx — abort a task holding a begun joint tx; assert a subsequent checkout runs BEGIN/COMMIT cleanly. Plus tests for the lease windows and the panic-safety guards that the harness can drive (a panicking protect callback driven through a sweep; the lease-release-on-panic path).

Coverage note: PgLoStaging's existing Drop body is 0%-covered in the live run — the one drop-cancellation test runs but does not reach the spawned-thread fallback the Drop body actually executes. The new tests should actually exercise the guard bodies where the harness allows; residue that genuinely cannot be reached is consciously recorded (not silently uncovered) — D-2's discipline.

Live harness

The joint-tx and lease shapes run against the local dockerized postgres container (pglo-poc, postgres:16-alpine on :15432, running for dev/testing):

export ALKBLOBS_PG_HOST=localhost
export ALKBLOBS_PG_PORT=15432
export ALKBLOBS_PG_USER=postgres
export ALKBLOBS_PG_PASSWORD=poc
export ALKBLOBS_PG_DB=blobs

Server-less, the pg tests skip cleanly; acceptance requires green against the live harness.

Acceptance Criteria

• PgJointTx carries a Drop guard aliasing PgLoStaging's spawned-ROLLBACK defense; the guard's trigger conditions documented on the type
• The sweep lease's acquire/release cancellation windows defended (cancelled acquire/release cannot strand the advisory lock on a pooled session); the shape recorded
• N-6: the in-process lease releases on a panic inside the sweep body; the fleet sweep_lock_release runs on panic (Drop guards)
• The cancel-the-future-mid-joint-tx test: an aborted task holding a begun joint tx leaves the next checkout running BEGIN/COMMIT cleanly
• The panic-path tests driven where the harness allows; any unreachable residue consciously recorded
• Gates green: cargo test (default), cargo test --all-features server-less, cargo test --all-features + live harness, clippy --all-targets -- -D warnings across the feature variants, cargo fmt --check

References

• docs/reviews/001-storage-surface-general-review.md (F-2; N-6; D-2's uncovered-set entries — postgres.rs:969-992/1104, pg_lo.rs:967-978 in the review's pre-split coordinates)
• docs/plans/implementation.md (§Wave 9 — "N-6's panic-safety Drop guards … ride here — same hazard class")
• src/postgres/mod.rs, src/postgres/joint.rs (PgJointTx)
• src/pg_lo/staging.rs (PgLoStaging's Drop — the alias source)
• src/postgres/pins.rs (sweep_lock_acquire/release)
• src/gc.rs, src/store/sweep.rs (the in-process lease, end_sweep, the sweep body)

Notes

The shipped shape

• Shared defense, one body — PgLoStaging's Drop body was factored into pg_common::detached_batch_execute(client, sql) (the spawn-on-the-runtime / one-shot-thread-fallback posture); the PgLoStaging Drop, the new PgJointTx Drop, and the new lease guard all alias it. "Aliasing the defense" is literal: one body, three call sites.
• PgJointTx guard (src/postgres/mod.rs, on the type): the client became Option<Object> — Some exactly while the tx is open. pin_commit consumes it (it takes the client before issuing COMMIT/ROLLBACK on every path), and every driver-side termination (store/fleet.rs A1 terminate_joint_tx) likewise ends with the client taken — so "checkout present at drop" is exactly "open tx stranded", the guard's trigger condition, documented on the type. The guard's body is the shared spawned-ROLLBACK. The accessor became client() -> Result<&Object, Error> (the defensively-typed consumed state, unreachable on live paths).
• The lease windows (src/postgres/pins.rs): one guard type, LeaseUnlockGuard — a pooled checkout whose advisory unlock is owed if dropped. In sweep_lock_acquire it owns the checkout from the pool checkout through the pg_try_advisory_lock round trip to the lease-store line (armed across the try — the answer's flight time is exactly the "locked server-side, unseen" window; a false answer disarms via release_unheld, and the grant stores into the lease record via into_hold); in sweep_lock_release it owns the taken checkout across the unlock round trip. Drop = spawned best-effort SELECT pg_advisory_unlock(key) (key inlined as the derived i64 — no injection surface). The lease-store line is the await-free seam, so completed paths defuse structurally.
• N-6 — GcHome::begin_sweep_guarded (gc.rs) returns the in-process SweepLeaseGuard; Store::sweep uses it (end_sweep on every exit — panic, cancellation, return). The fleet release got FleetSweepLeaseGuard (store/sweep.rs): armed at lease-acquired, disarmed only after the normal path's awaited release; a Drop past that spawns sweep_lock_release best-effort (same detached posture, sweep_lock_release's no-op-safety making the overlap harmless).

Test determinism

The abort-once-at-a-parked-point trick: each test-instrumented window is a _park point (#[cfg(test)], keyed by the test's own schema — fresh-schema-per-test keeps arming exclusive; an armed task parks forever until its test aborts it). The tests pre-verify the hazard state via a read-only pg_locks probe (advisory_held — the 64-bit key reads as its two 32-bit halves classid/objid, compared as text) rather than a try-lock probe: a try-lock probe can steal the lock ahead of the one-shot acquirer — the failure mode surfaced under the instrumented coverage run (first probe iteration colliding with the acquirer's single try-lock) and is why the probe reads the catalog, never takes the lock.

Coverage (live-harness run, the honest number; --test-threads=2

because the coverage-instrumented parallel run flaked unrelated TTL-cadence timing tests)

94.50% lines / 94.26% functions (the review's pre-fix live number was 95.70% on a smaller tree; the drops here are the consciously-uncovered defensive residue below plus the new test-only helpers). The f2-neighborhood files: postgres/mod.rs 0 missed lines (one defensive fn), postgres/joint.rs 97.93%, gc.rs 98.74%, postgres/pins.rs 94.02%, pg_lo/staging.rs 95.16%, store/sweep.rs 96.03%. PgJointTx's Drop body and both LeaseUnlockGuard windows are exercised by the new tests.

Consciously-uncovered residue (D-2 discipline)

• detached_batch_execute's thread-fallback arm (pg_common.rs:162, formerly PgLoStaging's never-covered arm) — every Drop in the test model happens inside a live runtime, including unwind-driven drops; the arm is the runtime-gone case, unreachable by any test runner.
• FleetSweepLeaseGuard::drop's thread-fallback arm (store/sweep.rs:841) — same shape, same verdict.
• The defensive already-consumed arms: pin_commit's (postgres/pins.rs:126), PgJointTx::client()'s, and LeaseUnlockGuard::session()'s — unreachable by invariant; each degrades to a typed error, never a panic.
• pin_commit's three real-media-failure ROLLBACK arms (postgres/pins.rs:142-164) — the review's D-2 residue names this exact family; still driven only by the injection test, still recorded.

Summary

F-2 + N-6 landed: review-001's cancellation/panic hazard class closed with drop-guard defenses and the regression tests that drive them.

• src/pg_common.rs — the shared best-effort checkout-cleanup (detached_batch_execute: spawned on the live runtime, or a one-shot thread's runtime after it); src/postgres/mod.rs — PgJointTx (the Option client, the Drop guard, the trigger conditions documented on the type); src/postgres/pins.rs — the joint-tx bodies over the taken client, the lease windows (LeaseUnlockGuard) and its guards; src/pg_lo/staging.rs — the Drop aliased onto the shared helper; src/store/fleet.rs (A1) adjusted for the accessor's shape; src/gc.rs + src/store/sweep.rs — begin_sweep_guarded/SweepLeaseGuard (the in-process lease) and FleetSweepLeaseGuard (the fleet release).

Verification: cargo test (default) 186 passed; cargo test --all-features server-less 409 passed (pg columns skip cleanly); cargo test --all-features + live harness (pglo-poc) 409 passed (three green full runs; one unrelated TTL-cadence flake observed once under the coverage-instrumented run and not since); cargo clippy --all-targets -- -D warnings clean on the default and --all-features variants; cargo fmt --check clean; cargo doc --no-deps zero warnings. Every new test red/green-verified: with the corresponding guard's Drop body disabled, the test fails against the live harness (the stranded tx surfaces through the reused session's COMMIT; the stranded advisory lock survives the poll). One pre-existing flake observed (and reproduced on the pre-change tree): ops::tests::the_paused_fleet_token_expires_reaps_and_the_reput_backstop_never_loses_silently under full-parallel-suite load — a paused-clock TTL-cadence test, unrelated to this task's changes; it passes both trees when isolated.

The four named test shapes, against the live harness:

• cancel_mid_joint_tx_leaves_the_next_checkout_clean (postgres/joint.rs) — abort a task holding a begun joint tx (BEGIN
  • entry put inside it, parked before pin_commit) on a max_size: 1 pool; the guard's spawned ROLLBACK lands and the very same session runs a clean joint tx BEGIN/COMMIT: the stranded put publishes nothing, the clean tx's pin row is the only residue.
• cancelled_acquire_cannot_strand_the_advisory_lock / cancelled_release_cannot_strand_the_advisory_lock (postgres/pins.rs) — abort at the recorded windows (a locked then-parked acquire; a taken but not-yet-unlocked release), asserted free again by the pg_locks probe and a successful re-acquire.
• sweep_panic_through_the_protect_consult_releases_the_in_process_lease (store/tests.rs, mem) — the panicking protect callback rides a spawned sweep; the recovery sweep reports no SweeperLock.
• panicking_protect_cannot_leave_the_sweep_leases_held (store/sweep.rs, postgres fleet) — the same panic route under both leases; the advisory lease re-acquires after the guard's spawned release lands, and the recovery sweep gets both domains granted.