Subtracting Duration from Instant on Mac is panic-prone

[matklad]

The following program:

fn main() {
    let t = std::time::Instant::now();
    let d = std::time::Duration::from_secs(60);
    eprintln!("{:?}", (t - d).elapsed());
}

works on Linux, but could panic on Mac. The reason for that is that, as implemented, Instant on mac starts at zero at reboot, so it can't represent times before the machine started.

This is pretty unfortunate -- it means that impl Sub<Duration> for Instant isn't really usable on mac, as subtracting even a small duration can panic. And subtracing duration is pretty useful and natural too implement tasks like "periodically, cleanup data crated earlier than Instant::now() - Duration::from_secs(60)".

While technically we don't give any guarantees on Instant range and document this as system-dependent, it is really unfortunate that subtracting API exists, works fine on Linux, and fails occasionally on macs. I think we should just go and patch the time on our side, to get some sort of a reasonable range bothways.

I think just

diff --git a/library/std/src/sys/unix/time.rs b/library/std/src/sys/unix/time.rs
index f99c453a3a8..994320564c4 100644
--- a/library/std/src/sys/unix/time.rs
+++ b/library/std/src/sys/unix/time.rs
@@ -168,7 +168,7 @@ pub fn now() -> Instant {
             extern "C" {
                 fn mach_absolute_time() -> u64;
             }
-            Instant { t: unsafe { mach_absolute_time() } }
+            Instant { t: unsafe { mach_absolute_time() } + u64::MAX / 2 }
         }
 
         pub fn checked_sub_instant(&self, other: &Instant) -> Option<Duration> {

would do the trick, but I am wondering if there's a better way to do this.

Manual for mach_absolute_time (https://developer.apple.com/documentation/kernel/1462446-mach_absolute_time) also suggests using clock_gettime_nsec_np(CLOCK_UPTIME_RAW)instead, so we can perhaps switch to that while we are at it.

[nmain]

Is the situation on Windows also concerning? It uses QueryPerformanceCounter which doesn't guarantee a particular epoch.

[ChrisDenton]

To be honest, I think I agree that Add<Duration> and Sub<Duration> should probably not have been implemented for Instant. It doesn't make sense given the current documentation (and implementations) of Instant as they can only really be compared with other Instants.

That said, there is definitely a use case for adding/subtracting precise times. But couldn't that be satisfied by SystemTime? Maybe with a new method if necessary.

[the8472]

And subtracing duration is pretty useful and natural too implement tasks like "periodically, cleanup data crated earlier than Instant::now() - Duration::from_secs(60)".

You can do this instead: foo.created_at + Duration::from_secs(60) < Instant::now()

[dimo414]

This is really surprising behavior. Instant is basically impossible to use safely given the existing implementation on OSX etc. It's understandable (if unfortunate) that arithmetic breaks down in platform-specific ways when operating on times in the distant past/future, but if you can't reliably construct an instant that's just a few minutes older than now() the entire type is fundamentally unsafe to use.

If Add/Sub on Instant are in fact mis-features it would be helpful to deprecate their use and document the intended alternatives. It's not at all obvious to me what someone's supposed to use instead of Instant if they want to refer to a moment in time but avoid the limitations of SystemTime (who's documentation recommends using Instant for safe elapsed time calculations), so clear documentation would be very helpful.

[Rodrigodd]

An alternative is to store a signed integer inside Instant. I believe that Linux's clock_gettime returns a signed value (or it is unspecified, not sure) and Windows' QueryPerformanceCounter does the same. Although now I see that mach_absolute_time returns a uint64_t.

This is a worse problem when the implementation for a given system uses the time since the program start. See sebcrozet/instant#45, for example.

[JakkuSakura]

It is worth noting that subtracting two u64 converted from Instant/SystemTime may also cause issues due to the non-monotone of Instance.
gluesql/gluesql#1499