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

unix/mphal: use CLOCK_MONOTONIC for ticks_ms/us #4715

Closed
wants to merge 1 commit into from

Conversation

mzakharocsc
Copy link
Contributor

Using gettimeofday for ticks_ms/us is not guaranteed to be increasing, which is contrary to the documentation. Using CLOCK_MONOTONIC fixes this.

@pfalcon
Copy link
Contributor

pfalcon commented Apr 23, 2019

While gettimeofday() isn't guaranteed to be increasing, clock_gettime() isn't guaranteed to exist at all.

@mzakharocsc
Copy link
Contributor Author

While gettimeofday() isn't guaranteed to be increasing, clock_gettime() isn't guaranteed to exist at all.

Is there an alternative? gettimeofday() coupled with NTP on a system can cause random unintended sideffects for code that assumes ticks_ms() is always increasing. clock_gettime() is available on Linux/FreeBSD/Mac OSX since 10.12. What other environments does the codebase need to support?

@pfalcon
Copy link
Contributor

pfalcon commented Apr 23, 2019

gettimeofday() coupled with NTP on a system can cause random unintended sideffects for code that assumes ticks_ms() is always increasing.

What specific issue are you facing?

clock_gettime() is available on Linux/FreeBSD/Mac OSX since 10.12. What other environments does the codebase need to support?

Unix port of MicroPython is intended to support as wide selection of Unix-like OSes as possible. E.g. that XENIX or AIX versions of 1985 or something.

@mzakharocsc
Copy link
Contributor Author

What specific issue are you facing?

In our application, we use ticks_ms() to measure time since last heartbeat from a child processes. If NTP changes system time into the future, this triggers heartbeat timeout and forces us to restart as a recovery measure.

@pfalcon
Copy link
Contributor

pfalcon commented Apr 23, 2019

Thanks,

So:

Is there an alternative?

The alternative is to select gettimeofday() vs clock_gettime() via #ifdef.

@mzakharocsc
Copy link
Contributor Author

The alternative is to select gettimeofday() vs clock_gettime() via #ifdef.

How should that #ifdef look like?

@pfalcon
Copy link
Contributor

pfalcon commented Apr 23, 2019

It should allow to use either gettimeofday() of clock_gettime() for implementation of those functions.

@dpgeorge
Copy link
Member

It might work to use the following to select the implementation (untested):

#if _POSIX_TIMERS > 0 && defined(_POSIX_MONOTONIC_CLOCK)
// use clock_gettime
#else 
// use gettimeofday
#endif 

@dpgeorge
Copy link
Member

@mzakharocsc I'm happy to make the #if changes and merge if you aren't able to.

@mzakharocsc
Copy link
Contributor Author

Updated with #ifdefs

@dpgeorge
Copy link
Member

Thanks for updating the PR. Merged in ced340d

@dpgeorge dpgeorge closed this Jun 26, 2019
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

3 participants