mirror of
https://github.com/block/buzz.git
synced 2026-08-18 06:50:31 +02:00
npub17jjz49l9jjmhhk7cac63j8yt9z555n9cw8vk7v5jz4vzw4ppld5qgj57cc
7772018add
Review (Wren, ratified by Eva) found the same "an admitted insert must not
outlive its protection" invariant failing at a second boundary. Round 1 closed
the lock boundary; this is the lifetime boundary.
`CacheResidency::admit()` stamped `deadline = now + AUTHZ_CACHE_TTL`, read
establishment, and returned a bool. All three callers inserted into moka only
afterwards, and moka starts its own identical TTL at that later instant. So the
residency window that protects an entry started *before* the entry's own clock
— under-covering it by the call gap at minimum, and by an unbounded amount if
the thread is preempted in between. The lost interleaving: admit(C) renews
residency to T+10s and returns true; the caller is delayed; after T+10s the
reconciler prunes residency and correctly withdraws establishment and
unsubscribes; the caller resumes and inserts an entry readable for nearly a
full TTL with no invalidation channel behind it.
The gate no longer hands out a bool for the caller to act on.
`CommunityTopics::with_established(community, f)` runs `f` inside the
establishment read lock, and `CacheResidency::admit_and_insert(community,
insert)` performs three ordered steps in one call:
1. residency is recorded before the establishment read (round 1's invariant,
unchanged): withdrawal re-reads desire under the matching write lock, so
either this renewal is seen and the subscription survives, or the withdrawal
already happened and nothing is admitted;
2. `insert()` runs inside the read lock, so no withdrawal can interleave
between the gate and the entry landing;
3. the residency deadline is re-stamped after `insert()` returns and still
inside that section, so it is based at or after the entry's own clock start
and therefore outlives it.
`admit()` is deleted rather than deprecated, so read-then-act is
unrepresentable rather than warned against — the same structural exclusion that
removed `remove_established` in round 1. The three call sites cache three
different value types (membership, accessible-channels, channel-visibility), so
the insert travels as a closure rather than making the API generic over the
cache. TTL padding was rejected: it narrows the window without closing
arbitrary descheduling.
The tests assert the exclusion *structurally*, with no threads and no sleeps.
An earlier draft of this commit parked a withdrawal thread and slept 50ms from
inside the critical section. That is deterministic on correct code, but its
*mutation bite* is not: under the unlocked mutation a late-scheduled withdrawer
still loses the race and the test passes vacuously. Wren caught it pre-push. A
probabilistic detector is not a deterministic test, so both concurrency tests
were rewritten to assert `try_write()`/`WouldBlock` from inside the critical
section — the lock state itself rather than a racing thread's outcome.
`CommunityTopics::try_withdraw_undesired_for_test` is the seam for that; it
shares `withdraw_locked` with the real `withdraw_undesired` so the twin cannot
drift from the path it stands in for.
`residency_outlives_the_entry_it_protects` previously compared TTL constants
and was green while the bug was live — equal durations do not nest when their
clocks start in the wrong order. It now records `Instant::now()` from inside
the insert closure and asserts `deadline >= inserted_at + AUTHZ_CACHE_TTL`, the
nesting relation itself.
Mutation results, each re-run to confirm determinism rather than a lucky pass:
* releasing the establishment lock before calling `f` (round 2's read-then-act,
restored) fails `no_entry_can_land_after_its_subscription_is_withdrawn` and
`with_established_holds_the_gate_for_the_whole_closure` on 5 of 5 runs, and
leaves `residency_outlives_the_entry_it_protects` green;
* dropping the step-3 re-stamp fails both relay tests on 3 of 3 runs. It bites
`no_entry_can_land...` too because that test's post-insert assertion — the
next withdrawal must withdraw *nothing* — is exactly what the re-stamp buys;
the lock alone does not survive a residency that lapsed during the insert.
So the lock and the re-stamp are separately load-bearing: the first mutation
isolates the lock, the second shows the re-stamp is required even with the lock
intact.
Also resolves the `rustdoc::private_intra_doc_links` warning introduced in the
previous commit: `cargo doc -p buzz-pubsub --no-deps` emitted 4 warnings at
c05cdb15d and emits 3 here, the remainder pre-existing and unrelated.
Co-authored-by: npub17jjz49l9jjmhhk7cac63j8yt9z555n9cw8vk7v5jz4vzw4ppld5qgj57cc <f4a42a97e594b77bdbd8ee35191c8b28a94a4cb871d96f32921558275421fb68@buzz.block.builderlab.xyz>
Signed-off-by: npub17jjz49l9jjmhhk7cac63j8yt9z555n9cw8vk7v5jz4vzw4ppld5qgj57cc <f4a42a97e594b77bdbd8ee35191c8b28a94a4cb871d96f32921558275421fb68@buzz.block.builderlab.xyz>