std::time inconsistencies

[debris]

When investigating one of the issues in parity-ethereum repo I noticed several inconsistencies in std::time.

I prepared two short snippets

snippet A

use std::time::{Instant, Duration};

fn main() {
    let now = Instant::now();
    let max_nanoseconds = u64::max_value() / 1_000_000_000;
    let duration = Duration::new(max_nanoseconds, 0);
    println!("{:?}", now + duration);
}

snippet B

use std::time::{Instant, Duration};

fn main() {
    let now = Instant::now();
    let max_nanoseconds = u64::max_value() / 1_000_000_000 + 1;
    let duration = Duration::new(max_nanoseconds, 0);
    println!("{:?}", now + duration);
}

Both of the attached snippets are properly executed on linux (gist), but they panic on macos.

snippet A panics with 'overflow when adding duration to instant'

snippet B panics with 'overflow converting duration to nanoseconds'

---

This is an inconsistent behaviour of standard library and I believe that above snippets should panic (or work) on all platforms in the same way

[sfackler]

Instant is a wrapper around the most precise available time source on each OS. Those sources differ in the size of their supported ranges but you should generally be able to assume they'll work with within a range of 100 or so years.

[vitvakatu]

Moreover, we've run into this issue on Windows too.
The following test passes on Linux, but fails on Windows:

let times = [
            UNIX_EPOCH,
            UNIX_EPOCH + Duration::new(13, 23),
            SystemTime::now(),
            SystemTime::now() + Duration::new(17, 15),
            UNIX_EPOCH + Duration::new(0, u32::max_value()),
            UNIX_EPOCH + Duration::new(i64::max_value() as u64, 0),
            UNIX_EPOCH + Duration::new(i64::max_value() as u64, 999_999_999),
            UNIX_EPOCH + Duration::new(i64::max_value() as u64 - 1, 1_000_000_000),
            UNIX_EPOCH + Duration::new(i64::max_value() as u64 - 4, 4_000_000_000),
            UNIX_EPOCH + Duration::new(i64::max_value() as u64 - 4, u32::max_value()),
        ];

I also think that the behavior must be consistent on all three main platforms, at least.

This issue might be related: #44394

[sfackler]

SystemTime is also a wrapper around the underlying system type. They do not all support times that are 300 billion years in the future.

[vitvakatu]

@sfackler I see your point, but what we're asking for, is to make behaviour consistent on every platform.

[retep998]

The behavior can't be consistent across platforms, because this is inherently platform specific behavior. Different platforms support different ranges and have different precisions.

[varkor]

At the very least, this ought to be better documented. There aren't any explicit warnings that std::time is platform-dependent (apart from the name SystemTime specifically, I don't think this is ever obvious).

On the other hand, unless the documentation regarding Duration is modified from "A Duration type to represent a span of time, typically used for system timeouts." to "A Duration type representing a span of system time." (or equivalent), I don't see why this restriction should be made — people are going to use it for non-system-related tasks at the moment, and as such the inconsistent behaviour is unexpected and poorly abstracted, even if they are in corner cases.

[retep998]

Duration itself is platform independent. It's just Instant and SystemTime which are platform specific, and adding (or subtracting) a Duration to an Instant or SystemTime can cause overflow on some platforms and not others.

But yes, better documentation is a good idea.

[eira-fransham]

This appears to be because on non-mac unix systems Instant is stored as seconds and nanoseconds (independently) whereas on on macOS it's stored as a u64, representing nanos since the unix epoch. A workaround for this would be to use SystemTime instead, which uses stores seconds and nanoseconds seperately on all platforms.

[vitvakatu]

@Vurich no, SystemTime behaves different on different platforms as well.