Fix CAS and increment/decrement lost-update races in cache-core - #81
Merged
Merged
Conversation
cas() validated the CasToken but then published via update_if_present, which compare-exchanges against a re-read slot word rather than the location the token was checked against. A concurrent set or cas landing between the token check and the publish was silently overwritten — a lost update, the exact race CAS exists to prevent. Publish via cas_location(key, checked_location, new_location) instead, making the slot exchange the linearization point: any concurrent write, relocation, or delete changes the slot and the CAS fails closed with Ok(false). Spurious exchange failures caused by concurrent readers bumping the frequency bits are retried while the key still maps to the checked location (get_item_frequency is freq-neutral). Co-Authored-By: Claude Fable 5 <[email protected]>
increment() and decrement() were get -> parse -> set: two concurrent operations could both read N and both write N+delta, losing an update. Rewrite both as a retry loop over with_value_cas (zero-copy read + CAS token under the item guard) and cas(), so a racing write forces a re-read instead of being overwritten. The missing-key initial case uses add() (insert-if-absent), retrying on a lost creation race. with_value_cas returns None both for an absent key and for an item that is transiently unreadable (segment mid-migration); only treat the key as absent when the hashtable agrees, otherwise retry. The new concurrent stress test (4 threads x 250 increments) loses ~55-70% of updates against the old code and passes exactly with the fix; behavior-preserving semantics (trim/parse, overflow, saturation, initial handling, error variants) are pinned by unit tests. Co-Authored-By: Claude Fable 5 <[email protected]>
CasToken ABA protection depends on try_reserve incrementing the segment generation on every Free -> Reserved transition. That bump exists (try_reserve resets stats and bumps generation) but no test pinned it across a full recycle cycle; add one so a refactor of the reserve path cannot silently drop it. Co-Authored-By: Claude Fable 5 <[email protected]>
- Use checked_div in DiskConfig::segment_count to satisfy the new clippy::manual_checked_ops lint (behavior unchanged: 0 when segment_size is 0). - Bump rustls-webpki 0.103.10 -> 0.103.13 for RUSTSEC-2026-0098, RUSTSEC-2026-0099, and RUSTSEC-2026-0104. Both pre-date this branch; they gate CI as of the latest stable clippy and advisory database. Co-Authored-By: Claude Fable 5 <[email protected]>
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
1 participant
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Summary
Two lost-update races in
TieredCache, found while porting the CasToken design to pelikan-io/cache-rs (pelikan-io/cache-rs#24):cas()published viaupdate_if_present, which compare-exchanges against a re-read slot word rather than the location the token was validated against. A concurrent set or cas landing between the token check and the publish was silently overwritten — the exact race CAS exists to prevent. Now publishes viacas_location(key, checked_location, new_location), making the slot exchange the linearization point; any concurrent write, relocation, or delete fails the CAS closed withOk(false). Spurious exchange failures from concurrent readers bumping the frequency bits are retried while the key still maps to the checked location (freq-neutralget_item_frequency).increment()/decrement()were get → parse → set: two concurrent operations could both read N and write N+delta. Both are nowwith_value_cas+cas()retry loops, withadd()(insert-if-absent) for theinitialcase.with_value_casreturningNoneis only treated as key-absent when the hashtable agrees, since it's also the transient mid-migration result. Semantics preserved: trim/parse, overflow checks, saturation at zero, initial handling, error variants.Also adds a regression test pinning that
try_reservebumps the segment generation on every recycle — the property CasToken's ABA protection depends on (the bump already existed; it just wasn't pinned).Testing
test_increment_concurrent_no_lost_updates(4 threads × 250 increments): against the old code it loses 55–70% of updates (observed 314/1000, 439/1000); with the fix it lands exactly at 1000, repeatedly.test_cas_token_lifecycleandtest_increment_decrement_semanticspin single-threaded behavior;test_segment_generation_bumps_on_recyclepins the recycle bump.cargo test -p cache-core(413 tests),--features loom(44 models), and segcache/heap-cache/slab-cache suites all green.cargo fmt --checkclean. (One pre-existing clippymanual_checked_opshit indisk/config.rsfrom a newer toolchain lint, present on main; workspace io_uring crates don't build on macOS as usual.)Follow-up
HeapCache::cas(cache/heap/src/lib.rs) has the same check-then-act publish shape on its slot storage and needs an analogous fix. Full race-freedom ofcas()against the (astronomically narrow) recycle-during-publish window would additionally need a segment read guard held from the generation check through the slot exchange.🤖 Generated with Claude Code