Skip to content

Commit a4ecc92

Browse files
committed
Prevent overflow when all entries are visited
This fixes a correctness bug where `insert()` could exceed the configured capacity if the cache was full and every entry had `visited = true`. This also adds a test showing how to trigger the now-fixed bug. The `evict()` implementation can return `None` after a full scan that only clears `visited` bits (first pass), without actually evicting an item. `insert()` calls `evict()` once and then proceeds, which leaves `len() == capacity` and allows the subsequent push to grow past capacity. In the SIEVE paper, Algorithm 1 scans while clearing `visited` and *wraps once*, then evicts the next unvisited entry in the same operation. Practically, this is equivalent to "two passes": first clears bits, second finds a candidate. Capacity is never exceeded. In `insert()`, if the first `evict()` returns `None` (meaning only bits were cleared), we now call `evict()` a second time and assert that it evicts one entry. This guarantees a free slot before insertion and aligns behavior with the SIEVE pseudocode. The fix was applied in `insert()` as that's the only code path able to exceed capacity. Doing this in `evict()` is another possible approach here.
1 parent a78672f commit a4ecc92

File tree

1 file changed

+25
-1
lines changed

1 file changed

+25
-1
lines changed

src/lib.rs

Lines changed: 25 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -420,14 +420,24 @@ impl<K: Eq + Hash + Clone, V> SieveCache<K, V> {
420420

421421
// Evict if at capacity
422422
if self.nodes.len() >= self.capacity {
423-
self.evict();
423+
let item = self.evict();
424+
// When the cache is full and *all* entries are marked `visited`, our `evict()` performs
425+
// a first pass that clears the `visited` bits but may return `None` without removing
426+
// anything. We still must free a slot before inserting, so we call `evict()` a second
427+
// time. This mirrors the original SIEVE miss path, which keeps scanning (wrapping once)
428+
// until it finds an item to evict after clearing bits.
429+
if item.is_none() {
430+
let item = self.evict();
431+
debug_assert!(item.is_some(), "evict() must remove one entry when at capacity");
432+
}
424433
}
425434

426435
// Add new node to the front
427436
let node = Node::new(key.clone(), value);
428437
self.nodes.push(node);
429438
let idx = self.nodes.len() - 1;
430439
self.map.insert(key, idx);
440+
debug_assert!(self.nodes.len() < self.capacity);
431441
true
432442
}
433443

@@ -1186,3 +1196,17 @@ fn test_recommended_capacity() {
11861196
"Capacity should not go below min_factor of current capacity"
11871197
);
11881198
}
1199+
1200+
#[test]
1201+
fn insert_never_exceeds_capacity_when_all_visited() {
1202+
let mut c = SieveCache::new(2).unwrap();
1203+
c.insert("a".to_string(), 1);
1204+
c.insert("b".to_string(), 2);
1205+
// Mark all visited
1206+
assert!(c.get("a").is_some());
1207+
assert!(c.get("b").is_some());
1208+
// This would exceed capacity
1209+
c.insert("c".to_string(), 3);
1210+
// This is our an invariant
1211+
assert!(c.len() <= c.capacity());
1212+
}

0 commit comments

Comments
 (0)