From 529531726fca07e0a624462838104388e89d029d Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Leo=20Bl=C3=B6cher?= Date: Fri, 12 Jul 2024 09:39:23 +0200 Subject: [PATCH] Fix AtomicPosition::reset storing wrong value (#650) 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. --- src/state.rs | 19 +++++++++++++++---- 1 file changed, 15 insertions(+), 4 deletions(-) diff --git a/src/state.rs b/src/state.rs index fc31f5cb..136e2307 100644 --- a/src/state.rs +++ b/src/state.rs @@ -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; @@ -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)); @@ -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); } @@ -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); + } }