[Yadunund]

Description

Fixes #3342

EventsCBGExecutor timers can stall after backward ROS time jumps or when use_sim_time is enabled at runtime because TimerQueue retains stale deadlines.

This PR uses the new schedule change callback from ros2/rcl#1352 to update timer deadlines. The callback only flags timers for rescheduling to avoid deadlocks.

test_executors_time_jump verifies both scenarios. It fails on Rolling and passes with both fixes.

Depends-On: ros2/rcl#1352

Is this user-facing behavior change?

Yes. Timers continue working after backward /clock jumps (e.g., ros2 bag play --loop --clock) and when use_sim_time is enabled at runtime.

Did you use Generative AI?

Yes. Claude Code (Claude Fable 5.1) assisted with investigation, implementation, and tests. Changes were reviewed and tested manually.

Additional Information

Reusing the on-reset callback causes a deadlock with TimeSource::set_clock().

[github-actions]

ABI Compliance Check

✅ Verdict: compatible

Library	Verdict	Summary
libcomponent_manager.so	✅ compatible	No ABI changes detected.
librclcpp.so	✅ compatible	No ABI changes detected.
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 @ 3637cae

(empty report — no differences printed by abidiff)

✅ librclcpp.so — full abidiff report

Compared:

• Base: lib-base/librclcpp.so
• Head: lib-pr/librclcpp.so @ 3637cae

Functions changes summary: 0 Removed, 0 Changed (112 filtered out), 0 Added (3 filtered out) functions
Variables changes summary: 0 Removed, 0 Changed, 0 Added variable

✅ librclcpp_action.so — full abidiff report

Compared:

• Base: lib-base/librclcpp_action.so
• Head: lib-pr/librclcpp_action.so @ 3637cae

(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 @ 3637cae

(empty report — no differences printed by abidiff)

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

[skyegalaxy]

thanks for catching the issue and for the PR @Yadunund . @jmachowinski and I both looked into this during the client library working group today. My intuition is that this PR is perhaps putting in additional complexity in rclcpp's handling of timers for something that ought to be fixed in the rcl layer.

I think the preference to not invoke on_reset in ros2/rcl#995 has had the unintended consequence of pushing rcl timer logic further up. The ClockManager in rclpy is also doing something similar to this PR's new bookkeeping. So perhaps we could consider either re-using on_reset and notifying at both timer reschedule sites there, or adding something like rcl_timer_set_on_schedule_change_callback. I don't think we'd need to modify existing function signatures and can just add new symbols so we don't have to worry about ABI breaks.

[Yadunund]

@skyegalaxy thanks for the quick feedback! I agree a fix upstream in rcl would be much cleaner. I've opened ros2/rcl#1352 to add a rcl_timer_set_on_schedule_change_callback API.

I had my agent iterate with the on_reset approach and it apparently led to deadlocks in some cases so going with the callback approach and calling it at both sites.

Deadlock with `on_reset`

Calling on_reset from both reschedule sites in `_rcl_timer_time_jump()` with the current `TimerQueue`. It fixes the stall, but it deadlocks the executor's timer thread against the time source. `TimeSource::set_clock()` holds the clock mutex while rcl runs the jump callbacks, so the on_reset handler now runs under it and takes the queue mutex and then the wait mutex in `wakeup_timer_thread()`, while the timer thread takes the wait mutex and then the clock mutex every time `ClockWaiterImpl::wait_until_ros_time()` installs or removes its jump handler. TSAN (`detect_deadlocks=1`) reports the inversion within seconds of backward jumps, and a stress with 64 ROS time timers and a 1 s backward `/clock` jump every 25 samples hangs within a minute when pinned to one CPU (`taskset -c 0`):

Also updated this PR to adopt the new callback.