Potential RT safety concern: publish_activity() called from update() loop with mutex acquisition

[shlok-mehndiratta]

Context

While reviewing the controller_manager architecture, I noticed what appears to be a real-time safety concern. Verified on commit a03be840 of ros-controls/ros2_control:master.

Problem

In controller_manager.cpp:L3369, publish_activity() is called from within the RT update() loop during fallback controller activation. However, publish_activity() acquires a lock_guard on rt_controllers_wrapper_.controllers_lock_ at L4083.

Since ros2_control_node runs the update loop on a SCHED_FIFO thread (ros2_control_node.cpp:L107), this mutex acquisition in the critical path could lead to priority inversion or unexpected jitter.

Proposed Fix

I've drafted a potential fix locally following the AsyncFunctionHandler pattern already used in the codebase:

1. Introduce std::atomic<bool> status_update_needed_ flag
2. RT thread sets the flag instead of calling publish_activity() directly
3. A timer in the non-RT executor_ thread polls the flag and performs the actual publishing

Is this a known issue? And if a fix is welcome - is the std::atomic flag approach preferred, or would realtime_tools::RealtimeBuffer be more appropriate for the RT-to-non-RT signaling here?

@destogl @saikishor @bmagyar - would appreciate your thoughts on this.

[saikishor]

@shlok-mehndiratta Can you explain how did you find the issue? and what bottlenecks you are having?
Please walk us through the process

[shlok-mehndiratta]

@shlok-mehndiratta Can you explain how did you find the issue? and what bottlenecks you are having? Please walk us through the process

Thanks for the quick response!

I was looking into publish_activity() specifically because I'm planning a contribution around Mission Control for my GSoC proposal. A reliable status stream is a core requirement for any higher-level coordination, so I needed to understand if the existing mechanism could serve as a stable foundation.

While tracing its call sites, I noticed it gets called at L3369 inside update() during fallback activation. That itself seemed worth flagging, but when I followed the implementation to L4083 and saw the recursive_mutex acquisition, it became a real concern - anything I build on top of this would inherit the same RT safety risk.

The design question I'm stuck on is the signaling pattern. I have a local draft using std::atomic<bool> to decouple the RT thread from the actual publishing, but I'm not sure if that's the right approach here or if realtime_tools::RealtimeBuffer would be more appropriate. A buffer feels slightly heavy for a simple lifecycle trigger, but I want to make sure it aligns with how the codebase handles this kind of thing.

Does the atomic flag approach seem reasonable, or is there a preferred pattern for RT-to-non-RT handoff here?

[shlok-mehndiratta]

@saikishor Adding some more details to my previous reply based on further tracing and a small PoC I worked on. This should help clarify the bottleneck I was referring to.

How I encountered this

While analyzing publish_activity() for a GSoC-related feature (Mission Control / status aggregation), I traced its call sites to evaluate whether it’s safe to rely on as a status stream. That’s where I noticed it being invoked inside the update() loop.

---

1. RT Critical Path Trace (Potential Issue)

During fallback activation, the following path is executed:

void ControllerManager::update() { // SCHED_FIFO thread
  ...
  if (fallback_active) {
    activate_fallback_controllers(); // L3369
    publish_activity();              // <-- in RT path
  }
}

void ControllerManager::publish_activity() {
  std::lock_guard<std::recursive_mutex> guard(
    rt_controllers_wrapper_.controllers_lock_); // L4083
}

Concern (summary):
This introduces a mutex acquisition in the RT loop. Since the same lock is also used in non-RT contexts (e.g., executor/service paths), this could lead to priority inversion or jitter, especially during fallback (a critical phase).

---

2. Proposed RT-safe Signaling (PoC)

To decouple RT from non-RT work, I tested a minimal signaling approach:

// ControllerManager
std::atomic<bool> status_update_needed_{false};

// RT thread (update)
status_update_needed_.store(true, std::memory_order_relaxed);

// Non-RT executor/timer
if (status_update_needed_.exchange(false, std::memory_order_acquire)) {
  publish_activity();
}

Rationale (summary):

• Keeps RT loop constant-time and lock-free
• Moves all mutex usage to non-RT context

---

3. Design Question

For this kind of RT → non-RT signaling:

• std::atomic<bool> seems suitable for event-style triggers
• realtime_tools::RealtimeBuffer appears more suited for data transfer

Does this align with the intended design patterns in ros2_control, or would RealtimeBuffer be preferred here for consistency?

---

If this direction looks reasonable, I’d be happy to turn this into a proper PR with tests and benchmarks.