Skip to content

Commit ea849b7

Browse files
authored
fix: Make StdClock monotonic using std::time::Instant (#4841)
This Pull Request fixes/closes #4330 . Previously, `StdClock` used `SystemTime` which isn't monotonic—it can jump backward when the system clock adjusts. This broke the guarantees that `JsInstant` is supposed to provide for engine timing. Now `StdClock` uses `std::time::Instant` with a base instant, measuring elapsed time from that point. This makes `JsInstant` truly monotonic like `std::time::Instant`, so time never goes backward for `setTimeout`, `setInterval`, and job scheduling. For `Date` objects that need actual wall-clock time, I added a separate `system_time_millis()` method to the `Clock` trait. Clean separation of concerns. Added tests to verify the monotonic behavior works as expected. --------- Signed-off-by: Abhinav Sharma <abhinavs1920bpl@gmail.com>
1 parent 79e6549 commit ea849b7

File tree

5 files changed

+126
-28
lines changed

5 files changed

+126
-28
lines changed

core/engine/src/builtins/date/mod.rs

Lines changed: 3 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -50,7 +50,7 @@ impl Date {
5050

5151
/// Creates a new `Date` from the current UTC time of the host.
5252
pub(crate) fn utc_now(context: &mut Context) -> Self {
53-
Self(context.clock().now().millis_since_epoch() as f64)
53+
Self(context.clock().system_time_millis() as f64)
5454
}
5555
}
5656

@@ -210,7 +210,7 @@ impl BuiltInConstructor for Date {
210210
// 1. If NewTarget is undefined, then
211211
if new_target.is_undefined() {
212212
// a. Let now be the time value (UTC) identifying the current time.
213-
let now = context.clock().now().millis_since_epoch();
213+
let now = context.clock().system_time_millis();
214214

215215
// b. Return ToDateString(now).
216216
return Ok(JsValue::from(to_date_string_t(
@@ -328,7 +328,7 @@ impl Date {
328328
/// [mdn]: https://developer.mozilla.org/en-US/docs/Web/JavaScript/Reference/Global_Objects/Date/now
329329
#[allow(clippy::unnecessary_wraps)]
330330
pub(crate) fn now(_: &JsValue, _: &[JsValue], context: &mut Context) -> JsResult<JsValue> {
331-
Ok(JsValue::new(context.clock().now().millis_since_epoch()))
331+
Ok(JsValue::new(context.clock().system_time_millis()))
332332
}
333333

334334
/// `Date.parse()`

core/engine/src/builtins/intl/date_time_format/mod.rs

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -257,7 +257,7 @@ impl DateTimeFormat {
257257
// NOTE (nekevss) i64 should be sufficient for a millisecond
258258
// representation.
259259
// a. Let x be ! Call(%Date.now%, undefined).
260-
context.clock().now().millis_since_epoch() as f64
260+
context.clock().system_time_millis() as f64
261261
// 4. Else,
262262
} else {
263263
// NOTE (nekevss) The i64 covers all MAX_SAFE_INTEGER values.

core/engine/src/builtins/temporal/now.rs

Lines changed: 4 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -213,9 +213,10 @@ impl HostHooks for &Context {}
213213

214214
impl HostClock for &Context {
215215
fn get_host_epoch_nanoseconds(&self) -> TemporalResult<EpochNanoseconds> {
216-
Ok(EpochNanoseconds::from(
217-
self.clock().now().nanos_since_epoch() as i128,
218-
))
216+
// Temporal needs actual Unix epoch time, not monotonic time
217+
let millis = self.clock().system_time_millis();
218+
let nanos = i128::from(millis) * 1_000_000;
219+
Ok(EpochNanoseconds::from(nanos))
219220
}
220221
}
221222

core/engine/src/context/mod.rs

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -1201,7 +1201,7 @@ impl ContextBuilder {
12011201
let root_shape = RootShape::default();
12021202

12031203
let host_hooks = self.host_hooks.unwrap_or(Rc::new(DefaultHooks));
1204-
let clock = self.clock.unwrap_or_else(|| Rc::new(StdClock));
1204+
let clock = self.clock.unwrap_or_else(|| Rc::new(StdClock::new()));
12051205
let realm = Realm::create(host_hooks.as_ref(), &root_shape)?;
12061206
let vm = Vm::new(realm);
12071207

core/engine/src/context/time.rs

Lines changed: 117 additions & 20 deletions
Original file line numberDiff line numberDiff line change
@@ -1,17 +1,23 @@
11
//! Clock related types and functions.
22
3+
use crate::sys::time::Instant;
4+
35
/// A monotonic instant in time, in the Boa engine.
46
///
57
/// This type is guaranteed to be monotonic, i.e. if two instants
68
/// are compared, the later one will always be greater than the
7-
/// earlier one. It is also always guaranteed to be greater than
8-
/// or equal to the Unix epoch.
9+
/// earlier one.
10+
///
11+
/// This mirrors the behavior of [`std::time::Instant`] and represents
12+
/// a measurement of elapsed time relative to an arbitrary starting point.
13+
/// It is NOT tied to wall-clock time or the Unix epoch, and system clock
14+
/// adjustments will not affect it.
915
///
1016
/// This should not be used to keep dates or times, but only to
11-
/// measure the current time in the engine.
17+
/// measure monotonic time progression in the engine.
1218
#[derive(Debug, Clone, Copy, Hash, PartialEq, Eq, PartialOrd, Ord)]
1319
pub struct JsInstant {
14-
/// The duration of time since the Unix epoch.
20+
/// The duration of time since an arbitrary starting point.
1521
inner: std::time::Duration,
1622
}
1723

@@ -29,13 +35,21 @@ impl JsInstant {
2935
Self { inner }
3036
}
3137

32-
/// Returns the number of milliseconds since the Unix epoch.
38+
/// Returns the number of milliseconds since the clock's starting point.
39+
///
40+
/// Note: This is NOT a Unix timestamp. It represents elapsed time
41+
/// since an arbitrary starting point and is only meaningful for
42+
/// measuring durations and comparing instants.
3343
#[must_use]
3444
pub fn millis_since_epoch(&self) -> u64 {
3545
self.inner.as_millis() as u64
3646
}
3747

38-
/// Returns the number of nanoseconds since the Unix epoch.
48+
/// Returns the number of nanoseconds since the clock's starting point.
49+
///
50+
/// Note: This is NOT a Unix timestamp. It represents elapsed time
51+
/// since an arbitrary starting point and is only meaningful for
52+
/// measuring durations and comparing instants.
3953
#[must_use]
4054
pub fn nanos_since_epoch(&self) -> u128 {
4155
self.inner.as_nanos()
@@ -131,22 +145,62 @@ impl std::ops::Sub for JsInstant {
131145

132146
/// Implement a clock that can be used to measure time.
133147
pub trait Clock {
134-
/// Returns the current time.
148+
/// Returns the current monotonic time.
149+
///
150+
/// Implementers must ensure this is monotonic and should be used for measuring
151+
/// durations and scheduling timeouts. The engine assumes monotonicity.
135152
fn now(&self) -> JsInstant;
153+
154+
/// Returns the current wall-clock time in milliseconds since the Unix epoch.
155+
///
156+
/// This is NOT monotonic and can go backward if the system clock is adjusted.
157+
/// It should only be used for `Date` objects and other wall-clock time needs.
158+
fn system_time_millis(&self) -> i64;
159+
}
160+
161+
/// A clock that uses the standard monotonic clock.
162+
///
163+
/// This clock is based on the `instant` crate's `Instant` type, which provides
164+
/// cross-platform monotonic time, including WASM support via `performance.now()`.
165+
/// Time measurements are relative to an arbitrary starting point
166+
/// (the first call to `now()`) and are not affected by system clock adjustments.
167+
///
168+
/// This ensures that time never goes backward, which is critical for
169+
/// maintaining the invariants of [`JsInstant`].
170+
#[derive(Debug, Clone, Copy)]
171+
pub struct StdClock {
172+
/// The base instant from which all measurements are relative.
173+
base: Instant,
136174
}
137175

138-
/// A clock that uses the standard system clock.
139-
#[derive(Debug, Clone, Copy, Default)]
140-
pub struct StdClock;
176+
impl Default for StdClock {
177+
fn default() -> Self {
178+
Self::new()
179+
}
180+
}
181+
182+
impl StdClock {
183+
/// Creates a new `StdClock` with the current instant as the base.
184+
#[must_use]
185+
pub fn new() -> Self {
186+
Self {
187+
base: Instant::now(),
188+
}
189+
}
190+
}
141191

142192
impl Clock for StdClock {
143193
fn now(&self) -> JsInstant {
194+
let elapsed = self.base.elapsed();
195+
JsInstant::new_unchecked(elapsed)
196+
}
197+
198+
fn system_time_millis(&self) -> i64 {
144199
let now = std::time::SystemTime::now();
145200
let duration = now
146201
.duration_since(std::time::UNIX_EPOCH)
147202
.expect("System clock is before Unix epoch");
148-
149-
JsInstant::new_unchecked(duration)
203+
duration.as_millis() as i64
150204
}
151205
}
152206

@@ -178,28 +232,36 @@ impl Clock for FixedClock {
178232
((millis % 1000) * 1_000_000) as u32,
179233
))
180234
}
235+
236+
fn system_time_millis(&self) -> i64 {
237+
*self.0.borrow() as i64
238+
}
181239
}
182240

183241
#[test]
184242
fn basic() {
185-
let now = StdClock.now();
186-
assert!(now.millis_since_epoch() > 0);
187-
assert!(now.nanos_since_epoch() > 0);
243+
let clock = StdClock::new();
244+
let now = clock.now();
245+
// Since we're using a relative clock, values are always >= 0 by type
246+
let _millis = now.millis_since_epoch();
247+
let _nanos = now.nanos_since_epoch();
188248

189249
let duration = JsDuration::from_millis(1000);
190250
let later = now + duration;
191251
assert!(later > now);
192252

193-
let earlier = now - duration;
194-
assert!(earlier < now);
253+
// Only subtract if we have enough time elapsed
254+
let duration_small = JsDuration::from_millis(100);
255+
let later_small = now + duration_small;
256+
let earlier = later_small - duration_small;
257+
assert_eq!(earlier, now);
195258

196-
let diff = later - earlier;
197-
assert_eq!(diff.as_millis(), 2000);
259+
let diff = later - now;
260+
assert_eq!(diff.as_millis(), 1000);
198261

199262
let fixed = FixedClock::from_millis(0);
200263
let now2 = fixed.now();
201264
assert_eq!(now2.millis_since_epoch(), 0);
202-
assert!(now2 < now);
203265

204266
fixed.forward(1000);
205267
let now3 = fixed.now();
@@ -212,3 +274,38 @@ fn basic() {
212274
assert_eq!(now4.millis_since_epoch(), u64::MAX);
213275
assert!(now4 > now3);
214276
}
277+
278+
#[test]
279+
fn monotonic_behavior() {
280+
let clock = StdClock::new();
281+
282+
// Verify that time always moves forward
283+
let t1 = clock.now();
284+
std::thread::sleep(std::time::Duration::from_millis(1));
285+
let t2 = clock.now();
286+
std::thread::sleep(std::time::Duration::from_millis(1));
287+
let t3 = clock.now();
288+
289+
// Time must always increase
290+
assert!(t2 > t1, "Time must move forward");
291+
assert!(t3 > t2, "Time must continue moving forward");
292+
assert!(t3 > t1, "Time must be transitive");
293+
294+
// Verify that elapsed time is reasonable
295+
let elapsed = t3 - t1;
296+
assert!(elapsed.as_millis() >= 2, "At least 2ms should have elapsed");
297+
}
298+
299+
#[test]
300+
fn clock_independence() {
301+
// Each clock instance has its own base instant
302+
let clock1 = StdClock::new();
303+
std::thread::sleep(std::time::Duration::from_millis(10));
304+
let clock2 = StdClock::new();
305+
306+
let t1 = clock1.now();
307+
let t2 = clock2.now();
308+
309+
// clock1 started earlier, so it should show more elapsed time
310+
assert!(t1.millis_since_epoch() >= t2.millis_since_epoch());
311+
}

0 commit comments

Comments
 (0)