Conversation
SF-Zhou
commented
Jul 16, 2026
SF-Zhou
commented
Owner
- Add a white-box test for pop_lru encountering an unheld entry in the defensive valueless state (removed during the scan and skipped), and extract the shared make_valueless() test helper.
- Replace pop_lru's silent 'if let Ok' on find_entry with .ok().expect(): every node in the LRU list is owned by the table, so a miss is a broken invariant and should panic instead of being ignored. This also removes the untestable Err region from coverage.
…fail fast - Add a white-box test for pop_lru encountering an unheld entry in the defensive valueless state (removed during the scan and skipped), and extract the shared make_valueless() test helper. - Replace pop_lru's silent 'if let Ok' on find_entry with .ok().expect(): every node in the LRU list is owned by the table, so a miss is a broken invariant and should panic instead of being ignored. This also removes the untestable Err region from coverage.
Codecov Report✅ All modified and coverable lines are covered by tests. Additional details and impacted files@@ Coverage Diff @@
## main #46 +/- ##
===========================================
+ Coverage 99.91% 100.00% +0.08%
===========================================
Files 3 3
Lines 2422 2441 +19
===========================================
+ Hits 2420 2441 +21
+ Misses 2 0 -2 ☔ View full report in Codecov by Harness. |
Contributor
There was a problem hiding this comment.
Pull request overview
This PR strengthens LruLockMap’s internal invariants and expands test coverage around defensive “valueless entry” states during LRU eviction.
Changes:
- Add a white-box test to cover
pop_lruskipping an unheld entry in a defensive valueless state. - Extract a shared
make_valueless()test helper to reduce duplication. - Make
pop_lrufail fast if an LRU-list node cannot be found in the backing table (invariant violation).
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
Comment on lines
+1379
to
+1394
| /// White-box helper: puts the unheld entry for `key` into the defensive | ||
| /// valueless state (refcnt == 0, no value) that iteration code must | ||
| /// tolerate but that cannot be produced through the public API. | ||
| fn make_valueless(cache: &LruLockMap<u32, u32>, key: u32) { | ||
| for shard in &cache.shards { | ||
| let inner = shard.inner.lock(); | ||
| for s in inner.table.iter() { | ||
| if s.key == key { | ||
| // SAFETY: the shard lock is held and refcnt == 0, so no | ||
| // guard exists and none can be created concurrently. | ||
| unsafe { (*s.value.get()).take() }; | ||
| s.set_value_state(false); | ||
| } | ||
| } | ||
| } | ||
| } |
Comment on lines
+183
to
+185
| // Every node in the LRU list is owned by the table, so the | ||
| // lookup cannot fail. | ||
| let entry = self |
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
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.