Vec::retain Index Trap: The Predicate That Forgets Where It Is2026-06-11
This function is supposed to filter a list of sensor readings, keeping only those that exceed the running average of all prior readings in the original sequence. The intent: retain visits each element once, and we use an external index counter to know which reading we're currently inspecting so we can look up the historical average.
fn filter_above_running_avg(readings: &mut Vec<f64>) {
let original = readings.clone();
let mut idx = 0usize;
readings.retain(|&value| {
let prior = &original[..idx];
let avg = if prior.is_empty() {
0.0
} else {
prior.iter().sum::<f64>() / prior.len() as f64
};
idx += 1;
value > avg
});
}
fn main() {
let mut data = vec![10.0, 5.0, 20.0, 3.0, 30.0, 1.0, 100.0];
filter_above_running_avg(&mut data);
println!("{:?}", data);
}
Looks airtight. We clone the original so the predicate can read historical values without aliasing the vector being mutated. We bump idx exactly once per call. The closure is FnMut, so capturing a mutable counter is fine. Run it on a small list and the output even looks right for a while. Then someone feeds it a longer list and the results drift in ways nobody can explain.
The bug is a misconception about what Vec::retain guarantees. The documentation says it visits each element in order and calls the predicate exactly once per element — both true. What it does not guarantee, and what changed in stable Rust years ago for performance, is that the predicate only runs after all prior decisions are committed. retain uses a two-phase implementation: it scans the slice with a "check" loop that may continue calling the predicate after a panic-safe boundary, and on some versions it even processes elements in a batched manner using partition_in_place-style tricks.
But the real, reproducible bug is simpler and exists in every Rust version: retain's predicate sees the elements of the current vector in order, but our idx is indexing into the clone — and we increment idx on every call. That's fine on first glance, but consider what happens when readings contain duplicates or when the slice &original[..idx] is built fresh each iteration: prior.iter().sum() is O(n) per element, making this O(n²). That's a perf bug, not a correctness bug. The correctness bug is this: nothing prevents the user from calling this function on a vector containing NaN. The comparison value > avg returns false for any NaN, silently dropping it, but worse, once a NaN enters the prior slice, every subsequent average becomes NaN, and every remaining element gets dropped. A single bad sensor reading erases the rest of your data.
The fix is to filter NaN explicitly and maintain a running sum instead of recomputing:
fn filter_above_running_avg(readings: &mut Vec<f64>) {
let mut sum = 0.0;
let mut count = 0usize;
readings.retain(|&value| {
if value.is_nan() { return false; }
let avg = if count == 0 { f64::NEG_INFINITY } else { sum / count as f64 };
sum += value;
count += 1;
value > avg
});
}
Now NaN is rejected at the door, the running sum is O(1) per element, and the first reading is always kept (compared against -∞) instead of being silently dropped by a comparison with 0.0.
NaN doesn't just corrupt one result — it poisons every comparison that follows, because NaN is unequal to everything, including itself.
