event loop: tolerate backward monotonic clock jumps #369

pull VijayabaskarR-06 wants to merge 2 commits into bitcoin-core:master from VijayabaskarR-06:fix-nonmonotonic-clock changing 4 files +145 −1
  1. VijayabaskarR-06 commented at 4:43 AM on September 29, 2026: none

    Description

    Cap'n Proto versions before 1.2 assume that successive CLOCK_MONOTONIC values never decrease. In some Linux virtualized environments, however, the clock can briefly move backwards, causing the event-loop wait to fail with a KJ exception ending with:

    "can't advance backwards in time"

    Cap'n Proto addressed this upstream in capnproto/capnproto#2261 and capnproto/capnproto#2296. Newer versions clamp a regressed timestamp to the previous monotonic value instead of raising this error.

    The failing check in older versions is recoverable (KJ_REQUIRE(newTime >= time, "can't advance backwards in time") { return; } in TimerImpl::advanceTo()). This PR installs a kj::ExceptionCallback on the event loop thread for the duration of EventLoop::loop(). The callback logs a warning and returns for this specific error, so the timer keeps its previous time like newer versions do. Every other error is forwarded to the next callback unchanged. The workaround is compiled only for Cap'n Proto versions before 1.2, so builds against newer versions are unchanged.

    Testing

    Adds a Linux-only mpclocktest that overrides clock_gettime() to force a 10 ms CLOCK_MONOTONIC regression while the event loop is waiting. It is also added to the sanitize CI job.

    Tested locally in Linux containers:

    • Cap'n Proto 0.9.2, 1.0.1, 1.1.0 and 1.3.0: all tests pass, and mpclocktest passed 100+ repeated runs on each.
    • With 0.9.2 and the callback removed, the test fails with can't advance backwards in time.
    • 0.9.2 with the olddeps warning flags (-Werror), and clang with the llvm job's flags plus clang-tidy and IWYU on the changed files: clean.
    • TSan build (uninstrumented Cap'n Proto 1.1.0): mpclocktest 50/50 runs with no reports.

    Related

    • bitcoin/bitcoin#36345
    • capnproto/capnproto#2261
    • capnproto/capnproto#2296
  2. DrahtBot commented at 4:43 AM on September 29, 2026: none

    <!--e57a25ab6845829454e8d69fc972939a-->

    The following sections might be updated with supplementary metadata relevant to reviewers and maintainers.

    <!--021abf342d371248e50ceaed478a90ca-->

    Reviews

    See the guideline and AI policy for information on the review process.

    Type Reviewers
    Concept ACK ryanofsky

    If your review is incorrectly listed, please copy-paste <code>&lt;!--meta-tag:bot-skip--&gt;</code> into the comment that the bot should ignore.

    <!--5faf32d7da4f0f540f40219e4f7537a3-->

  3. in src/mp/proxy.cpp:311 in ffc7be8d7b outdated
     307 | +        bool retry{false};
     308 | +        // Unlike try/catch, this works across KJ dynamic library boundaries.
     309 | +        KJ_IF_MAYBE(exception, kj::runCatchingExceptions([&] {
     310 | +            read_bytes = wait_stream->read(&buffer, 0, 1).wait(m_io_context.waitScope);
     311 | +        })) {
     312 | +            // Cap'n Proto versions before 1.2 throw if CLOCK_MONOTONIC briefly
    


    Sjors commented at 8:01 AM on September 29, 2026:

    Can we gate this behind a version check (in addition to the string comparison below)?


    VijayabaskarR-06 commented at 9:41 AM on September 29, 2026:

    So Basically I added them so the workaround is now actually gated with CAPNP_VERSION < 1002000 in addition to the exception type and message check so thats why Cap’n Proto 1.2+ won’t take this path.


    Sjors commented at 4:09 PM on September 29, 2026:

    Thanks, that also makes it easier to drop the workaround if we ever bump the minimum version past this.

  4. in src/mp/proxy.cpp:314 in ffc7be8d7b
     310 | +            read_bytes = wait_stream->read(&buffer, 0, 1).wait(m_io_context.waitScope);
     311 | +        })) {
     312 | +            // Cap'n Proto versions before 1.2 throw if CLOCK_MONOTONIC briefly
     313 | +            // moves backwards. Retrying lets the event loop recover once the
     314 | +            // clock catches up.
     315 | +            if (exception->getType() != kj::Exception::Type::FAILED ||
    


    Sjors commented at 8:03 AM on September 29, 2026:

    style nit: if (!( exception->getType() == kj::Exception::Type::FAILED && exception->getDescription ...)) is more readable imo.


    VijayabaskarR-06 commented at 9:42 AM on September 29, 2026:

    Got it Changed to the positive condition form you suggested. Thanks.

  5. in src/mp/proxy.cpp:303 in ffc7be8d7b outdated
     299 | @@ -300,8 +300,29 @@ void EventLoop::loop()
     300 |  
     301 |      kj::Own<kj::AsyncIoStream>& wait_stream{m_wait_stream};
     302 |      char buffer = 0;
     303 | +    bool clock_regressed{false};
    


    Sjors commented at 8:06 AM on September 29, 2026:

    why do you need both clock_regressed and retry?


    VijayabaskarR-06 commented at 9:43 AM on September 29, 2026:

    Ok so i just removed retry now here!. clock_regressed is only kept to suppress duplicate warnings during consecutive clock regression failures and it gets reset after a successful wait.

  6. Sjors commented at 8:06 AM on September 29, 2026: member

    Are you sure this is the only place in the codebase where can't advance backwards in time or a similar error can happen? In that case the workaround seems simple enough.

    Consider dropping (most of) "Changes" and "Testing" from the PR description.

  7. VijayabaskarR-06 force-pushed on Sep 29, 2026
  8. VijayabaskarR-06 commented at 9:51 AM on September 29, 2026: none

    Hello @Sjors Thankyou very much for you validations ! I completely checked the wait sites in the repository and as pe my knowledge this is the only active KJ wait using m_io_context.waitScope. The other active waits are condition variable waits, so they don't go through KJ's timer handling and can't produce this exception. I've also trimmed the PR description according to the changes as suggested. Thankyou once again would love to hear furthur feedbacks from you!

  9. Sjors commented at 4:40 PM on September 29, 2026: member

    Codex (Astra) suggests the following additional test, to actually simulate the time shift:

    <details><summary>patch</summary>

    diff --git a/test/CMakeLists.txt b/test/CMakeLists.txt
    index fd0aa023d..275454734 100644
    --- a/test/CMakeLists.txt
    +++ b/test/CMakeLists.txt
    @@ -40,3 +40,14 @@ if(BUILD_TESTING AND TARGET CapnProto::kj-test)
       add_dependencies(mptests mptest)
       add_test(NAME mptest COMMAND mptest)
    +
    +  # This test overrides clock_gettime() to force CLOCK_MONOTONIC backwards.
    +  # Use a separate executable so other tests keep using the system clock.
    +  # Linux Cap'n Proto versions before 1.2 throw on these backward readings.
    +  if(CMAKE_SYSTEM_NAME STREQUAL "Linux")
    +    add_executable(mpclocktest mp/test/clock_tests.cpp)
    +    target_link_libraries(mpclocktest PRIVATE Libmultiprocess::multiprocess CapnProto::kj-test Threads::Threads ${CMAKE_DL_LIBS})
    +    add_dependencies(mptests mpclocktest)
    +    add_test(NAME mpclocktest COMMAND mpclocktest)
    +    set_tests_properties(mpclocktest PROPERTIES TIMEOUT 30)
    +  endif()
     endif()
    diff --git a/test/mp/test/clock_tests.cpp b/test/mp/test/clock_tests.cpp
    new file mode 100644
    index 000000000..e79d3d83d
    --- /dev/null
    +++ b/test/mp/test/clock_tests.cpp
    @@ -0,0 +1,93 @@
    +// Copyright (c) The Bitcoin Core developers
    +// Distributed under the MIT software license, see the accompanying
    +// file COPYING or http://www.opensource.org/licenses/mit-license.php.
    +
    +#include <mp/proxy-io.h>
    +#include <mp/proxy.h>
    +
    +#include <capnp/common.h>
    +#include <kj/async-io.h>
    +#include <kj/common.h>
    +#include <kj/debug.h>
    +#include <kj/function.h>
    +#include <kj/memory.h>
    +#include <kj/test.h>
    +#include <kj/time.h>
    +#include <kj/timer.h>
    +#include <kj/units.h>
    +
    +#include <cstdlib>
    +#include <ctime>
    +#include <dlfcn.h>
    +#include <future>
    +#include <string>
    +#include <sys/types.h>
    +#include <thread>
    +
    +namespace {
    +struct ClockRegression
    +{
    +    timespec time{};
    +    unsigned reads{0};
    +    std::promise<void> entering_wait;
    +};
    +
    +// Only the event-loop thread opts into the fake clock. In particular, CTest's
    +// watchdog and the test thread's synchronization keep using the real clock.
    +thread_local ClockRegression* g_regression{nullptr};
    +} // namespace
    +
    +// Interpose both KJ's clock reads and calls from this executable, without
    +// requiring LD_PRELOAD or a modified Cap'n Proto build.
    +extern "C" int clock_gettime(clockid_t clock, struct timespec* time) noexcept
    +{
    +    if (clock == CLOCK_MONOTONIC && g_regression) {
    +        *time = g_regression->time;
    +        if (++g_regression->reads == 1) g_regression->entering_wait.set_value();
    +        return 0;
    +    }
    +    static const auto real_clock_gettime = reinterpret_cast<decltype(&clock_gettime)>(dlsym(RTLD_NEXT, "clock_gettime"));
    +    if (!real_clock_gettime) std::abort();
    +    return real_clock_gettime(clock, time);
    +}
    +
    +KJ_TEST("EventLoop recovers from a backward monotonic clock reading")
    +{
    +    ClockRegression regression;
    +    unsigned warnings{0};
    +    std::promise<mp::EventLoopRef> started;
    +    std::thread thread([&] {
    +        mp::EventLoop loop("mpclocktest", [&](mp::LogMessage log) {
    +            if (log.level == mp::Log::Warning && log.message.find("non-monotonic clock read") != std::string::npos) ++warnings;
    +        });
    +        started.set_value(mp::EventLoopRef{loop});
    +        loop.loop();
    +    });
    +    auto loop = started.get_future().get();
    +
    +    loop->sync([&] {
    +        // Regress relative to KJ's last observed time, so scheduling delays
    +        // cannot cause the real clock to catch up before the fault is observed.
    +        const auto time = loop->m_io_context.provider->getTimer().now() - kj::origin<kj::TimePoint>() - 10 * kj::MILLISECONDS;
    +        regression.time.tv_sec = time / kj::SECONDS;
    +        regression.time.tv_nsec = (time % kj::SECONDS) / kj::NANOSECONDS;
    +        g_regression = &regression;
    +    });
    +
    +    // KJ reads the clock before entering epoll_wait(), and again after waking.
    +    // Wait for the first read before posting work: this ensures the socket read
    +    // is pending, instead of completing synchronously and bypassing KJ's timer.
    +    regression.entering_wait.get_future().wait();
    +    loop->sync([&] { g_regression = nullptr; });
    +
    +    // Check another callback and clean shutdown after restoring the real clock.
    +    bool completed{false};
    +    loop->sync([&] { completed = true; });
    +    loop.reset();
    +    thread.join();
    +
    +    KJ_EXPECT(regression.reads >= 2);
    +    KJ_EXPECT(completed);
    +    // Newer KJ versions clamp internally; older ones need the retry handler.
    +    KJ_EXPECT(warnings == (CAPNP_VERSION < 1002000 ? 1u : 0u));
    +}
    

    </details>

    It's probably overkill though, and I don't fully understand what it's doing.

  10. VijayabaskarR-06 commented at 5:53 AM on September 30, 2026: none

    Thanks @Sjors. So currently I just validated the suggested regression test and added it in here 261d155. The test installs a regressed monotonic clock for the event loop thread and it waits until the event loop performs its first fake clock read, and only then posts work to wake it so the backward time path is actually exercised.So this algorithm can be more accurate as you said !

    I also tested it with Cap’n Proto 0.9.2, 1.0.1, and 1.2.0, and all passed then i ran the regression test 100 consecutive times successfully. So waiting for your review! Thanks for the suggestion this gives the workaround direct regression coverage.

  11. in src/mp/proxy.cpp:16 in 625480017a
      12 | @@ -13,6 +13,7 @@
      13 |  #include <atomic>
      14 |  #include <capnp/capability.h>
      15 |  #include <capnp/common.h> // IWYU pragma: keep
      16 | +#include <capnp/generated-header-support.h>
    


    ryanofsky commented at 3:40 PM on October 1, 2026:

    In commit "event loop: tolerate backward monotonic clock jumps" (625480017a1e1a153c46242fd0dab6f688e21c1f)

    This include is added here and removed again in the test commit, so the change could be dropped from both commits.


    VijayabaskarR-06 commented at 5:44 AM on October 2, 2026:

    Done, dropped the include from both commits.

  12. in src/mp/proxy.cpp:323 in 261d155a41
     319 | +            if (!clock_regressed) {
     320 | +                MP_LOG(*this, Log::Warning) << "EventLoop: retrying after non-monotonic clock read.";
     321 | +                clock_regressed = true;
     322 | +            }
     323 | +            continue;
     324 | +        }
    


    ryanofsky commented at 3:40 PM on October 1, 2026:

    In commit "event loop: tolerate backward monotonic clock jumps" (625480017a1e1a153c46242fd0dab6f688e21c1f)

    Instead of catching the exception and retrying, I think a simpler and less invasive fix would be to install a kj::ExceptionCallback on the event loop thread that ignores this specific error, and leave the wait() call as it is on master.

    This works because the check that fails in old Cap'n Proto versions is a recoverable one: KJ_REQUIRE(newTime >= time, "can't advance backwards in time") { return; } in TimerImpl::advanceTo(). KJ reports it by calling onRecoverableException() on the current thread's ExceptionCallback, and the default callback throws. If the callback returns instead, the { return; } recovery block runs and advanceTo() keeps the previous time, which is basically what the upstream fix in capnproto/capnproto#2296 does.

    The callback is registered just by constructing it. The kj::ExceptionCallback constructor pushes the object onto a thread-local stack of callbacks (saving the previous one as next), and the destructor pops it. So a local variable at the top of EventLoop::loop() applies to everything KJ does on the event loop thread while the loop runs, and to nothing on other threads:

    namespace {
    #if CAPNP_VERSION < 1002000
    //! Ignore "can't advance backwards in time" errors from Cap'n Proto versions
    //! before 1.2, which raise them when CLOCK_MONOTONIC briefly goes backwards
    //! (https://github.com/capnproto/capnproto/issues/2261). Returning lets
    //! TimerImpl::advanceTo() keep the previous time, like newer versions do.
    class ClockErrorCallback : public kj::ExceptionCallback
    {
    public:
        explicit ClockErrorCallback(EventLoop& loop) : m_loop(loop) {}
        void onRecoverableException(kj::Exception&& exception) override
        {
            if (exception.getType() == kj::Exception::Type::FAILED &&
                exception.getDescription().endsWith("can't advance backwards in time")) {
                MP_LOG(m_loop, Log::Warning) << "EventLoop: ignoring non-monotonic clock read.";
                return;
            }
            next.onRecoverableException(kj::mv(exception));
        }
        EventLoop& m_loop;
    };
    #endif
    } // namespace
    
    void EventLoop::loop()
    {
        assert(!CurrentThread().loop_thread);
        CurrentThread().loop_thread = true;
        KJ_DEFER(CurrentThread().loop_thread = false);
    #if CAPNP_VERSION < 1002000
        // Registered with KJ for this thread until loop() returns.
        ClockErrorCallback clock_error_callback{*this};
    #endif
        ...rest of loop() unchanged from master...
    }
    

    Advantages over the current approach:

    • The exception is currently thrown from UnixEventPort::processEpollEvents() after epoll events have been consumed, so unwinding drops its return value, which says whether another thread woke the loop to run kj::Executor events. Nothing here uses kj::Executor so this doesn't break anything now, but it would be easy to miss later. With a callback, nothing is unwound and KJ's state stays the same as if the clock had not moved backwards.
    • It covers every KJ wait and poll on the event loop thread, not only this call site, which answers Sjors's question about other places this could happen.
    • Builds against Cap'n Proto 1.2 and later compile exactly the same code as master.

    The new test should keep working as is, since it only counts warnings. I haven't tried this against an old Cap'n Proto version, so it's possible I'm missing something.


    VijayabaskarR-06 commented at 5:44 AM on October 2, 2026:

    Thanks, switched to the kj::ExceptionCallback approach in f5ce6f8, close to your snippet.

    Tested in Linux containers against Cap'n Proto 0.9.2, 1.0.1, 1.1.0 and 1.3.0. With 0.9.2, removing the callback makes the test fail with can't advance backwards in time, and with it the loop recovers.

    One thing did need changing: the callback can run more than once during a single wakeup (I saw 1 or 2 warnings depending on timing), so the test now checks warnings >= 1 on older versions instead of exactly 1.

  13. in src/mp/proxy.cpp:313 in 261d155a41
     309 | +            read_bytes = wait_stream->read(&buffer, 0, 1).wait(m_io_context.waitScope);
     310 | +        })) {
     311 | +            // Cap'n Proto versions before 1.2 throw if CLOCK_MONOTONIC briefly
     312 | +            // moves backwards. Retrying lets the event loop recover once the
     313 | +            // clock catches up.
     314 | +            if (!(CAPNP_VERSION < 1002000 &&
    


    ryanofsky commented at 3:40 PM on October 1, 2026:

    In commit "event loop: tolerate backward monotonic clock jumps" (625480017a1e1a153c46242fd0dab6f688e21c1f)

    If you keep this approach rather than the ExceptionCallback one suggested above: since CAPNP_VERSION is a compile-time constant, it would be better to check it with #if CAPNP_VERSION < 1002000 around the whole runCatchingExceptions block. Right now, with Cap'n Proto 1.2 and later, every exception thrown from wait() is still caught and rethrown through kj::throwRecoverableException, which changes non-KJ exceptions into kj::Exception for no reason. With #if, newer builds compile the same code as before, and it's clear what to delete when the minimum version is raised.


    VijayabaskarR-06 commented at 5:44 AM on October 2, 2026:

    Resolved by the switch to the callback approach. The class and its registration are both under #if CAPNP_VERSION < 1002000, so builds against 1.2+ compile the same code as master.

  14. in test/mp/test/clock_tests.cpp:93 in 261d155a41
      88 | +    thread.join();
      89 | +
      90 | +    KJ_EXPECT(regression.reads >= 2);
      91 | +    KJ_EXPECT(completed);
      92 | +    // Newer KJ versions clamp internally; older ones need the retry handler.
      93 | +    KJ_EXPECT(warnings == (CAPNP_VERSION < 1002000 ? 1u : 0u));
    


    ryanofsky commented at 3:40 PM on October 1, 2026:

    In commit "test: cover backward monotonic clock recovery" (261d155a41ccbafcd3647d23cf2d5fde580dfe52)

    I think it's worth keeping this test, since otherwise the workaround has no coverage. Note that the only Linux CI job using Cap'n Proto older than 1.2 is olddeps (0.9.2), so that's the job that actually runs the workaround path. The other jobs only check that newer versions clamp the time themselves. It would also be good to confirm the sanitize job (TSan, with Cap'n Proto built with TSan) is happy with the clock_gettime override. I approved the CI runs so we should see results soon.


    VijayabaskarR-06 commented at 5:44 AM on October 2, 2026:

    Kept the test. The previous CI run had three failures, now fixed in 02767e1:

    • sanitize builds only mptest but runs every registered test, so mpclocktest was "Not Run". Added it to that job's BUILD_TARGETS, so the clock_gettime override is now also checked under TSan. Locally, a TSan build passed 50/50 runs with no reports, but that used an uninstrumented Cap'n Proto, so the CI job is the real check.
    • default and llvm failed on IWYU wanting <kj/common.h> and <kj/debug.h> in clock_tests.cpp, now added.
  15. ryanofsky commented at 3:46 PM on October 1, 2026: collaborator

    [Note: review below is entirely from LLM, but I think the approach it suggests seems a bit cleaner and less fragile so would recommend trying it]

    Concept ACK 261d155a41ccbafcd3647d23cf2d5fde580dfe52. Thanks for following up on bitcoin/bitcoin#36345. I suggested there that upgrading Cap'n Proto in CI might be enough, but as maflcko pointed out, the latest Ubuntu LTS only ships 1.1, so a workaround here makes sense for anyone building against distro packages.

    I left a suggestion below for a less invasive way to do this with kj::ExceptionCallback, plus a few smaller comments. It would also be good to move the explanation in the PR description into the first commit message, since both commits currently have empty bodies.

  16. event loop: tolerate backward monotonic clock jumps
    Cap'n Proto versions before 1.2 assume that successive CLOCK_MONOTONIC
    values never decrease. In some Linux virtualized environments the clock
    can briefly move backwards, and the event loop wait then fails with a
    KJ exception ending in "can't advance backwards in time". Cap'n Proto
    fixed this upstream in capnproto/capnproto#2261 and
    capnproto/capnproto#2296, where a regressed timestamp is clamped to the
    previous value instead of raising an error.
    
    The failing check in older versions is recoverable:
    KJ_REQUIRE(newTime >= time, "can't advance backwards in time") { return; }
    in TimerImpl::advanceTo(). KJ reports it by calling
    onRecoverableException() on the current thread's kj::ExceptionCallback,
    and the default callback throws. Install a callback on the event loop
    thread that logs a warning and returns for this specific error, so the
    recovery block runs and the timer keeps its previous time, which matches
    the behavior of newer versions. All other errors are forwarded to the
    next callback unchanged.
    
    The callback only exists while EventLoop::loop() runs, applies only to
    the event loop thread, and covers every KJ wait and poll on that thread.
    It is compiled only for Cap'n Proto versions before 1.2, so builds
    against newer versions are unchanged.
    f5ce6f819f
  17. test: cover backward monotonic clock recovery
    Add a Linux-only mpclocktest executable that overrides clock_gettime()
    so CLOCK_MONOTONIC returns a time 10ms behind KJ's last observed time
    while the event loop is waiting. It checks that the event loop keeps
    running callbacks and shuts down cleanly after the real clock is
    restored.
    
    With Cap'n Proto versions before 1.2, the test expects at least one
    "non-monotonic clock read" warning from the ClockErrorCallback added in
    the previous commit. The callback can run more than once during a single
    wakeup, so the exact count is not checked. With newer versions, KJ
    clamps the time internally and no warning is expected.
    
    The test is a separate executable so other tests keep using the system
    clock. Add it to the sanitize CI job's build targets, since that job
    only builds mptest but runs every registered test, and so the
    clock_gettime() override is also checked under ThreadSanitizer.
    02767e1bbd
  18. VijayabaskarR-06 force-pushed on Oct 2, 2026
  19. VijayabaskarR-06 requested review from Sjors on Oct 2, 2026
  20. VijayabaskarR-06 requested review from ryanofsky on Oct 2, 2026

github-metadata-mirror

This is a metadata mirror of the GitHub repository bitcoin-core/libmultiprocess. This site is not affiliated with GitHub. Content is generated from a GitHub metadata backup.
generated: 2026-10-08 00:30 UTC

This site is hosted by @0xB10C
More mirrored repositories can be found on mirror.b10c.me