[skyegalaxy]

Description

The timer manager in #3097 introduced a variant of ClockWaiter and ClockConditionalVariable which could accept a clock on its wait_until calls instead of at construction. This PR unifies both ClockWaiter and ClockConditionalVariable under one set which can do both, and removes the duplicates in timer_manager.

• ClockWaiter and ClockConditionalVariable can now accept a null clock at construction, and both gain a wait_until(lock, clock, until, pred) overload. The existing overloads without a clock are unchanged.
• The clock given to a wait is only used for that call and is never stored, so the timer queue doesn't keep a timer's clock alive.
• ClockConditionalVariable keeps one ClockWaiter for its whole lifetime, so notify_one() takes no lock and a wait performs no allocation.
• TimerQueue uses rclcpp::ClockConditionalVariable directly, now constructed with a null clock.
• A wait with no clock (none given at construction or on the call), or a null clock passed to the new overload, throws std::runtime_error. Before, waiting without a clock dereferenced a null pointer.
• If the timer thread's wait throws while the context is still valid, it now logs an error before stopping that queue's timers. Before, it stopped them silently. During shutdown it stays quiet, as before.

Is this user-facing behavior change?

rclcpp::ClockConditionalVariable::wait_until() used to return true unconditionally. It now returns false if the context was shut down before the call. Callers that ignored the return value need no change.

A wait whose until doesn't match the clock's type now throws before it starts, on every clock type.

Did you use Generative AI?

Yes, Claude Opus 5.5

Additional Information

[github-actions]

ABI Compliance Check

✅ Verdict: compatible (additions only)

Library	Verdict	Summary
libcomponent_manager.so	✅ compatible	No ABI changes detected.
librclcpp.so	✅ additions-only	ABI changed but only with additions (backward-compatible).
librclcpp_action.so	✅ compatible	No ABI changes detected.
librclcpp_lifecycle.so	✅ compatible	No ABI changes detected.

✅ libcomponent_manager.so — full abidiff report

Compared:

• Base: lib-base/libcomponent_manager.so
• Head: lib-pr/libcomponent_manager.so @ 626453d

(empty report — no differences printed by abidiff)

✅ librclcpp.so — full abidiff report

Compared:

• Base: lib-base/librclcpp.so
• Head: lib-pr/librclcpp.so @ 626453d

Functions changes summary: 0 Removed (30 filtered out), 0 Changed (134 filtered out), 2 Added (13 filtered out) functions
Variables changes summary: 0 Removed, 0 Changed, 0 Added variable

2 Added functions:

  [A] 'method bool rclcpp::ClockConditionalVariable::wait_until(std::unique_lock<std::mutex>&, const rclcpp::Clock::SharedPtr&, const rclcpp::Time&, const std::function<bool()>&)'    {_ZN6rclcpp24ClockConditionalVariable10wait_untilERSt11unique_lockISt5mutexERKSt10shared_ptrINS_5ClockEERKNS_4TimeERKSt8functionIFbvEE}
  [A] 'method bool rclcpp::ClockWaiter::wait_until(std::unique_lock<std::mutex>&, const rclcpp::Clock::SharedPtr&, const rclcpp::Time&, const std::function<bool()>&)'    {_ZN6rclcpp11ClockWaiter10wait_untilERSt11unique_lockISt5mutexERKSt10shared_ptrINS_5ClockEERKNS_4TimeERKSt8functionIFbvEE}

✅ librclcpp_action.so — full abidiff report

Compared:

• Base: lib-base/librclcpp_action.so
• Head: lib-pr/librclcpp_action.so @ 626453d

(empty report — no differences printed by abidiff)

✅ librclcpp_lifecycle.so — full abidiff report

Compared:

• Base: lib-base/librclcpp_lifecycle.so
• Head: lib-pr/librclcpp_lifecycle.so @ 626453d

(empty report — no differences printed by abidiff)

_{Updated for commit 626453d · check run · suppressions: /home/runner/work/_temp/ros2-abi-suppressions.txt}

[skyegalaxy]

Pulls: #3401
Gist: https://gist.githubusercontent.com/skyegalaxy/321c7098da935e031b380b66ed453c47/raw/c23e34775d5f4767cc08708d4867337f30471401/ros2.repos
BUILD args: --packages-above-and-dependencies rclcpp
TEST args: --packages-above rclcpp
ROS Distro: rolling
Job: ci_launcher
ci_launcher ran: https://ci.ros2.org/job/ci_launcher/21046

• Linux
• Linux-aarch64
• Linux-rhel
• Windows

[mjcarroll]

With a RCL_ROS_TIME clock and sim time off (and with a RCL_SYSTEM_TIME clock), the old code never compared abs_time to the clock, so wait_until(lk, rclcpp::Time(sec, nsec), pred) worked. rclcpp::Time defaults to system time, so that call now throws. Only the steady path and the active-ROS path threw before, through Time::operator- and operator>=.

Could we keep the old behavior for those paths, or call this out as a breaking change?

[skyegalaxy]

reverted to the old behavior in 708ac97

[mjcarroll]

This overload makes ClockConditionalVariable cv(nullptr); and std::make_unique<ClockConditionalVariable>(nullptr) ambiguous, because nullptr converts to both shared_ptr types.

[skyegalaxy]

1ed6a12

[mjcarroll]

Switching to rclcpp::ClockConditionalVariable means wait_until can now throw for reasons other than the shutdown race: a clock type mismatch, or a null clock. The existing catch (const std::runtime_error &) in the timer thread (around L392) sets running = false without logging, so either of these would stop every timer in the queue without a message. Worth narrowing that catch, or at least logging.

[skyegalaxy]

added logging for when a wait fails af98349

[fujitatomoya]

add one space for alignment?

Suggested change

	* to wait on, for example because the clocks involved may be destroyed
	* between waits.
	* to wait on, for example because the clocks involved may be destroyed
	* between waits.

[skyegalaxy]

fixed in 14fe72c

[fujitatomoya]

this means this must be always with default context? imo it shoule be more like the following with using the stored context in TimerQueue? otherwise, using explicit context in the application will fail here.

Suggested change

	if (rclcpp::ok()) {
	if (rclcpp::ok(context)) {

[skyegalaxy]

626453d