Skip to content

Commit

Permalink
Fix AtomicPosition::reset storing wrong value (#650)
Browse files Browse the repository at this point in the history
The code actually expects this value to be in ns. I reckon the bug got
introduced because the comments are worded a bit ambiguously, so I also
cleared those up.
  • Loading branch information
TheJokr authored Jul 12, 2024
1 parent 022fda7 commit 5295317
Showing 1 changed file with 15 additions and 4 deletions.
19 changes: 15 additions & 4 deletions src/state.rs
Original file line number Diff line number Diff line change
Expand Up @@ -537,7 +537,7 @@ impl AtomicPosition {
}

let mut capacity = self.capacity.load(Ordering::Acquire);
// `prev` is the number of ms after `self.started` we last returned `true`, in ns
// `prev` is the number of ns after `self.started` we last returned `true`
let prev = self.prev.load(Ordering::Acquire);
// `elapsed` is the number of ns since `self.started`
let elapsed = (now - self.start).as_nanos() as u64;
Expand All @@ -551,8 +551,8 @@ impl AtomicPosition {
return false;
}

// We now calculate `new`, the number of ms, in ns, since we last returned `true`,
// and `remainder`, which represents a number of ns less than 1ms which we cannot
// We now calculate `new`, the number of INTERVALs since we last returned `true`,
// and `remainder`, which represents a number of ns less than INTERVAL which we cannot
// convert into capacity now, so we're saving it for later. We do this by
// subtracting this from `elapsed` before storing it into `self.prev`.
let (new, remainder) = ((diff / INTERVAL), (diff % INTERVAL));
Expand All @@ -568,7 +568,7 @@ impl AtomicPosition {

fn reset(&self, now: Instant) {
self.set(0);
let elapsed = (now.saturating_duration_since(self.start)).as_millis() as u64;
let elapsed = (now.saturating_duration_since(self.start)).as_nanos() as u64;
self.prev.store(elapsed, Ordering::Release);
}

Expand Down Expand Up @@ -799,4 +799,15 @@ mod tests {
// Should not panic.
atomic_position.allow(later);
}

#[test]
fn test_atomic_position_reset() {
const ELAPSE_TIME: Duration = Duration::from_millis(20);
let mut pos = AtomicPosition::new();
pos.reset(pos.start + ELAPSE_TIME);

// prev should be exactly ELAPSE_TIME after reset
assert_eq!(*pos.pos.get_mut(), 0);
assert_eq!(*pos.prev.get_mut(), ELAPSE_TIME.as_nanos() as u64);
}
}

0 comments on commit 5295317

Please sign in to comment.