rxjs: Replay subject/async scheduler susceptible to clock disciplining race conditions

Bug Report

Current Behavior

Reproduction I did not reproduce the bug, but I am confident this will cause bugs.

Consider that a user could simply adjust their clock manually, and the web app would erase chat message(s) or other values pending in a replay subject, prematurely (consider the consuming web app specified these values should remain in the replay subject for 30 seconds by passing the windowTime argument to the constructor of ReplaySubject.)

Other reasons the NTP time must not be used to calculate durations of time, is NTP will slew the clock so 1000ms elapse in 995ms of wall time, and will adjust the time abruptly if the time goes out of sync by over 128ms, yielding negative durations if relied upon to time the duration of something.

Expected behavior Use performance.now() to time how long something took, since it not subject to clock disciplining, and timing the durations with it would be totally transparent to users, aside from fixing the bug.

Environment n/a

Possible Solution I would change Date.now() to performance.now(), in every single case where it is used to time the duration of how long something to took. Users should not suffer from clock drift issues by default.

Additional context/Screenshots Please read https://www.w3.org/TR/hr-time-2/#introduction Also please read http://www.ntp.org/ntpfaq/NTP-s-algo.htm#Q-CLOCK-DISCIPLINE “how will NTP adjust my clock” for additional context https://caniuse.com/#search=performance.now

About this issue

  • Original URL
  • State: closed
  • Created 4 years ago
  • Comments: 17 (14 by maintainers)

Most upvoted comments

In my opinion, this discussion doesn’t need to be distracted to pursue those directions: If rx provides proper configurable single, consistent entrypoint accepts any timestamp provider you want to use? then I guess what should be default value would be discussed separately, esp changing default will be definitely some kind of breaking but configurable entrypoint should not be considered as breaking.

Somewhat similar crux of change we had is https://github.com/ReactiveX/rxjs/pull/3566 - this is not runtime, but compile time enforcement user have to bring dom type definition lib even though their runtime never intend to use browser-like env. In result, we decide to take it out.

If we are going to put thin a wrapper around the default provider

Is reason I thought attached issue might be related. If we have single point of abstracted interface access to timestamp which is configurable, I think that would solve these maybe.