Skip to content

Use monotonic time for timer deadlines - #345

Open
holmuk wants to merge 1 commit into
freebsd:masterfrom
holmuk:issue-335-monotonic-clock
Open

holmuk wants to merge 1 commit into
freebsd:masterfrom
holmuk:issue-335-monotonic-clock

Conversation

@holmuk

@holmuk holmuk commented Oct 23, 2026 •

Copy link
Copy Markdown
Contributor

The PR introduce datetime::monotonic_timestamp, backed by clock_gettime(CLOCK_MONOTONIC), and use it in the timer scheduler for deadline creation, ordering, remaining-time calculations, and expiration checks.

Monotonic time is preferable for deadlines because they depend on elapsed time rather than calendar time. This follows the clock semantics documented by FreeBSD: CLOCK_MONOTONIC uses an unspecified epoch, while setting the time of day operates on CLOCK_REALTIME. See FreeBSD clock_gettime(2).

The new tests cover:

  • Monotonic timestamp arithmetic and comparisons, rejection of negative intervals
  • calendar_backward: verify that moving the calendar clock backwards does not prevent a timer from firing.
  • calendar_forward: verify that moving the calendar clock forwards does not trigger a premature callback when processing an alarm.

Resolves #335

Introduce monotonic_timestamp backed by CLOCK_MONOTONIC and use it
in the timer scheduler to avoid deadline changes caused by
calendar clock adjustments. Add ./configure checks and new tests
covering timestamp operations, clock source, and timer behavior when
calendar time moves backwards or forwards.

Signed-off-by: Pavel Grakhov <altogetherx@gmail.com>
@ngie-eign

ngie-eign commented Oct 25, 2026 •

Copy link
Copy Markdown
Contributor

@holmuk : I appreciate the submission, but this isn't exactly what I had in mind if you look at the issue set as a whole.
Using clock_gettime with C++11 and later programs is a step backwards: while the underlying implementation for std::chrono might use clock_gettime, there's no guarantee that that is actually how the feature is implemented as desired.
Your suggested change also adds additional autotools integration, which I'm trying to steer the project away from as it's more difficult to maintain than some of the alternatives (cmake, meson).
Using std::chrono is the path forward, but that's just a part of what needs to be done (there are more pieces than just the linked subtask).
That being said, I think your approach about tackling the timers instead of focusing on both the timers and the metadata is spot on for fixing the immediate issue--which is what's generating a ton of the test failures. This can be easily delivered in the next release.

@holmuk

holmuk commented Oct 25, 2026

Copy link
Copy Markdown
Contributor Author

@ngie-eign , Thanks for clarifying the preferred direction. I agree that using std::chrono would simplify the implementation. However, the timer scheduler currently reads the clock and processes deadlines directly from the SIGALRM handler. Unlike clock_gettime(), std::chrono::steady_clock::now() is not guaranteed to be async-signal-safe (C++ [support.signal] section, 3.1 restricts standard library calls from signal handlers to explicitly signal-safe functions and plain lock-free atomic operations).

Anyway, I think this points to a more fundamental architectural issue. sigalrm_handler() directly calls global_state::fire() - this function processes expired deadlines, updates containers, reprograms the timer, and invokes callbacks - all of it happens before the signal handler returns. Consequently, the entire call chain including user callbacks executes in signal-handler context and must be async-signal-safe.

Ideally, the signal handler should only notify the scheduler that work is pending. Deadline processing and callbacks should then run in an explicitly defined, ordinary execution context.

As I see it, we have two options:

  1. Keep using clock_gettime() for now, preserving the existing signal-handling architecture, and defer the migration to std::chrono until the signal-handling code is refactored.
  2. Refactor signal handling first, moving deadline processing out of the signal handler so that std::chrono::steady_clock can be used without relying on undocumented signal-safety guarantees.

The second option would address the underlying architectural problem and probably require a series of PR under an epic issue.

Would you prefer keeping the current PR as an incremental fix for the immediate timer issue, or should we close the current PR and address the architecture problem first?

@ngie-eign ngie-eign left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Why not just inherit datetime::monotonic_timestamp from datetime::timestamp? It would cut down on a ton of duplicate code.

@ngie-eign

ngie-eign commented Oct 27, 2026 •

Copy link
Copy Markdown
Contributor

@ngie-eign , Thanks for clarifying the preferred direction. I agree that using std::chrono would simplify the implementation. However, the timer scheduler currently reads the clock and processes deadlines directly from the SIGALRM handler. Unlike clock_gettime(), std::chrono::steady_clock::now() is not guaranteed to be async-signal-safe (C++ [support.signal] section, 3.1 restricts standard library calls from signal handlers to explicitly signal-safe functions and plain lock-free atomic operations).

Anyway, I think this points to a more fundamental architectural issue. sigalrm_handler() directly calls global_state::fire() - this function processes expired deadlines, updates containers, reprograms the timer, and invokes callbacks - all of it happens before the signal handler returns. Consequently, the entire call chain including user callbacks executes in signal-handler context and must be async-signal-safe.

Ideally, the signal handler should only notify the scheduler that work is pending. Deadline processing and callbacks should then run in an explicitly defined, ordinary execution context.

Bleh. I didn't know about that caveat with std::chrono (the reentrancy problem), but that tracks that the code is calculating times and programming/unprogramming timers in signal handlers which it really shouldn't be sigh.

As I see it, we have two options:

1. Keep using `clock_gettime()` for now, preserving the existing signal-handling architecture, and defer the migration to `std::chrono` until the signal-handling code is refactored.

2. Refactor signal handling first, moving deadline processing out of the signal handler so that `std::chrono::steady_clock` can be used without relying on undocumented signal-safety guarantees.

Let's go for option 1. now and work towards 2.

Thanks!

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.

Replace gettimeofday use for time durations with monotonic clock

2 participants