-
Notifications
You must be signed in to change notification settings - Fork 12.8k
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Instant::now can go backward #56612
Comments
Ok, I'm not sure that SO link was the best reference, but e.g. https://chromium.googlesource.com/chromium/src/base/+/master/time/time_win.cc#14 also claims that QPC is buggy. |
The linked bug appears to be on a virtual machine, Some hypervisors may emulate TSC instructions with coarse granularity and possibly not core-synchronized. Operating systems try to detect the accuracy of the TSCs based on CPU flags (invariant, nonstop tsc). If the hypervisor is not setting those correctly that's basically out-of-spec behavior. |
We've seen things like this in Firefox and have deployed large hammers to fix. Further investigation suggests that the problems may be limited to certain AMD CPUs. |
I've opened #56988 to apply such a hammer to libstd as well |
This commit is an attempt to force `Instant::now` to be monotonic through any means possible. We tried relying on OS/hardware/clock implementations, but those seem buggy enough that we can't rely on them in practice. This commit implements the same hammer Firefox recently implemented (noted in rust-lang#56612) which is to just keep whatever the lastest `Instant::now()` return value was in memory, returning that instead of the OS looks like it's moving backwards. Closes rust-lang#48514 Closes rust-lang#49281 cc rust-lang#51648 cc rust-lang#56560 Closes rust-lang#56612 Closes rust-lang#56940
std: Force `Instant::now()` to be monotonic This commit is an attempt to force `Instant::now` to be monotonic through any means possible. We tried relying on OS/hardware/clock implementations, but those seem buggy enough that we can't rely on them in practice. This commit implements the same hammer Firefox recently implemented (noted in #56612) which is to just keep whatever the lastest `Instant::now()` return value was in memory, returning that instead of the OS looks like it's moving backwards. Closes #48514 Closes #49281 cc #51648 cc #56560 Closes #56612 Closes #56940
std: Force `Instant::now()` to be monotonic This commit is an attempt to force `Instant::now` to be monotonic through any means possible. We tried relying on OS/hardware/clock implementations, but those seem buggy enough that we can't rely on them in practice. This commit implements the same hammer Firefox recently implemented (noted in #56612) which is to just keep whatever the lastest `Instant::now()` return value was in memory, returning that instead of the OS looks like it's moving backwards. Closes #48514 Closes #49281 cc #51648 cc #56560 Closes #56612 Closes #56940
This hammer is a code smell. This means that I can continue to get the wrong time even after a server's internal clock has been corrected by a time synchronizing service such as NTP. We should not be panicking when the diff between two instants is negative. That possibility should merely be stated in the API documentation for functions where it is likely for an inexperienced software engineer to make an assumption of monotonic increase on a system with more than one processor.
https://gist.github.com/timvisee/fcda9bbdff88d45cc9061606b4b923ca |
You already posted exactly the same comment in #56988, please do not spam. Also note that |
As in #51648 (comment).
I've seen "specified instant was later than self" panics in the wild, in particular on Windows (32-bit x86). Googling reveals stuff like https://stackoverflow.com/questions/44020619/queryperformancecounter-on-multi-core-processor-under-windows-10-behaves-erratic which suggests that QueryPerformanceCounter can't really be relied on... frustratingly.
Perhaps libstd should work around this and enforce that
Instant::now()
never go backward (with an atomic global, or something?)The text was updated successfully, but these errors were encountered: