Skip to content
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

steady_time module fails to compile on Windows #27

Closed
phil-opp opened this issue Apr 11, 2024 · 5 comments
Closed

steady_time module fails to compile on Windows #27

phil-opp opened this issue Apr 11, 2024 · 5 comments
Assignees
Labels
bug Something isn't working

Comments

@phil-opp
Copy link
Contributor

The steady_time module introduced in a0d8c8e causes compile errors on Windows:

error[E0425]: cannot find function `clock_gettime` in crate `libc`
Error:   --> C:\Users\runneradmin\.cargo\git\checkouts\ros2-client-1852ea698eaf7825\008b76c\src\steady_time.rs:35:27
   |
35 |       assert_eq!(0, libc::clock_gettime(libc::CLOCK_MONOTONIC, &mut ts));
   |                           ^^^^^^^^^^^^^ not found in `libc`

error[E0425]: cannot find value `CLOCK_MONOTONIC` in crate `libc`
Error:   --> C:\Users\runneradmin\.cargo\git\checkouts\ros2-client-1852ea698eaf7825\008b76c\src\steady_time.rs:35:47
   |
35 |       assert_eq!(0, libc::clock_gettime(libc::CLOCK_MONOTONIC, &mut ts));
   |                                               ^^^^^^^^^^^^^^^ not found in `libc`

The clock_gettime POSIX function is not available on Windows.

@Michael-J-Ward
Copy link

Michael-J-Ward commented Apr 13, 2024

rust's std::time::Instant uses the same system call on unix system's that steady_time is using: clock_gettime with CLOCK_MONOTINC.

It's guaranteed to be non-decreasing, and it seems like the implementation is as to monotonic as possible for the given hardware. The monotonicity section of the docs also states (emphasis added):

On all platforms Instant will try to use an OS API that guarantees monotonic behavior if available, which is the case for all tier 1 platforms. In practice such guarantees are – under rare circumstances – broken by hardware, virtualization or operating system bugs.

What does steady_time's implementation offer above what std::time::Instant does?

For reference - the unix implementation:

https://github.com/rust-lang/rust/blob/79424056b05eaa9563d16dfab9b9a0c8f033f220/library/std/src/sys/pal/unix/time.rs#L266-L294

@Michael-J-Ward
Copy link

Michael-J-Ward commented Apr 13, 2024

Digging further down this rabbit hole because this if the first that I've encountered it.

Based on the stackoverflow discussion Does Rust have a steady clock, Rust's Instant and steady_time in c++ are equivalent on rust's tier 1 systems.

However, a big caveat is that CLOCK_MONOTONIC operates differently depending on the platform. On Linux and FreeBSD, it does not include time that the machine was suspended. On Darwin and OpenBSD, it does include time suspended.

This comment on rust's github summarizes the semantics of the different clocks available on the different platforms.

I am a novice to the robot domain, so I don't know how important that caveat is. If it is, someone published an Instant crate that unifies the semantics between the different platforms to always include suspended time - announced in that same rust-lang thread.

@jhelovuo
Copy link
Owner

Thank you for reporting.

We do not routinely test on Windows, so this was released by accident.

The latest master branch changes the steady_time implementation to be a wrapper around std::time::Instant. This should be more portable, as noted in the comments above. As a downside, we lose the ability to convert between steady_time::Time and other representations, as Instant is opaque.

Please check if this works for you and report the result here.

@jhelovuo jhelovuo self-assigned this Apr 15, 2024
@jhelovuo jhelovuo added the bug Something isn't working label Apr 15, 2024
@jhelovuo
Copy link
Owner

Version 0.7.1 also contains the fix.

@phil-opp
Copy link
Contributor Author

I can confirm that this is fixed. Thanks a lot for the quick fix & release!

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
bug Something isn't working
Projects
None yet
Development

No branches or pull requests

3 participants