Skip to content

Fixed event-loop clock rounding - #924

Open
radkesvat wants to merge 1 commit into
ithewei:masterfrom
radkesvat:fix/loop-clock-rounding
Open

radkesvat wants to merge 1 commit into
ithewei:masterfrom
radkesvat:fix/loop-clock-rounding

Conversation

@radkesvat

@radkesvat radkesvat commented Nov 4, 2026 •

Copy link
Copy Markdown
Contributor

hloop_now() can report the previous second even when hloop_now_ms() already reports the next one. hloop_update_time() interprets that rounding error as a system-clock change and unnecessarily recalculates the loop's wall-clock starting point.

Example

Suppose the loop starts at 12:00:00.900, and 200 ms passes. The system clock has not been adjusted:

Starting time:           12:00:00.900
Elapsed time:                 0.200 seconds
Correct current time:   12:00:01.100

The existing seconds calculation truncates the two parts separately:

truncate(starting time) + truncate(elapsed time)
       12:00:00         +          0 seconds
                     = 12:00:00

But adding first and then truncating gives 12:00:01, consistent with both the system clock and hloop_now_ms().

Because hloop_update_time() compares the incorrectly calculated second with time(NULL), it enters its “systemtime changed” path even though both clocks are progressing normally. This can happen repeatedly around second boundaries. Rebuilding the starting point from separately sampled clocks also lets sampling jitter affect later cached timestamps; applications using unsigned elapsed-time subtraction are particularly sensitive to any backward step. This PR addresses the unnecessary recalculation, without claiming that wall-clock timestamps are monotonic.

Controlled backward-step regression

The regression test also demonstrates a backward step using controlled clock samples. Starting from a wall-clock origin of 100,900 ms, it supplies these increasing readings:

High-resolution elapsed time Sampled wall time Old hloop_now_ms() Fixed hloop_now_ms()
100 ms 101,002 ms 101,002 ms 101,000 ms
1,099 ms 101,999 ms 102,001 ms 101,999 ms
1,100 ms 102,000 ms 102,000 ms 102,000 ms

The first sample models a normal 2 ms delay between reading the high-resolution clock and the wall clock. The old code unnecessarily rebases at that point, then rebases again in the final row. Its cached time goes backward by 1 ms, although both input clocks move forward. The unmodified code reproduced this result in the test; the corrected Debug and Release builds passed. These are deterministic test inputs, not measurements from an application workload.

Changes

  • Derive hloop_now() from the combined millisecond timestamp, so it agrees with hloop_now_ms() / 1000.
  • Compare complete millisecond timestamps in hloop_update_time(): the sampled system time and the starting wall time plus elapsed high-resolution time.
  • Recalculate the starting point only when the absolute difference is at least 1,000 ms. Handle both directions without unsigned subtraction wrapping around.
  • Add focused regression coverage for the fractional-second boundary, the backward step above, differences below the threshold, and actual clock adjustments in either direction.

Real system-clock changes still update the wall-clock baseline. hloop_now_hrtime() and the high-resolution deadlines used by ordinary relative timers retain their existing behavior. No public API changes are needed.

Validation

  • Added test against unmodified upstream code: failed as expected, including the 102001 → 102000 ms reversal.
  • Linux static-library CMake builds, Debug and Release: focused test passed, including controlled clock updates and differences of 0, ±1, ±999, ±1000, and ±5000 ms. Checks remain active with NDEBUG.
  • Linux shared-only CMake Release build and Make build: focused test passed for the cached timestamp getters. Clock-source injection is enabled only for Linux static-library CMake builds.

To run the full focused regression on Linux:

cmake -S . -B build/clock-debug \
  -DCMAKE_BUILD_TYPE=Debug -DBUILD_UNITTEST=ON -DBUILD_EXAMPLES=OFF \
  -DBUILD_STATIC=ON -DBUILD_SHARED=OFF -DWITH_HTTP=OFF -DWITH_EVPP=ON
cmake --build build/clock-debug --target hloop_clock_test -j8
build/clock-debug/bin/hloop_clock_test

Repeat with Release and a separate build directory to check optimized behavior. The tests do not adjust the host clock. Other operating systems were not tested.

This change concerns timestamp calculation and clock-change detection. It is separate from the timer lifetime work in #910 and the socket-polling starvation fix in #922.

Thanks to Mahmut Şeyhan, who first discovered this bug and reported it in WaterWall PR #346.

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.

1 participant