Refactor component containers + Add option for CBG Executor by skyegalaxy · Pull Request #3134 · ros2/rclcpp

skyegalaxy · 2026-04-19T05:01:38Z

Description

Refactors component_container.cpp to be the single entrypoint for launching component managers of all types.

CLI parsing of executor type and isolated mode
Added new constructor to ComponentManagerIsolated which consumes an ExecutorOptions and num_threads. these are stored in the class and used when spawning new nodes in isolated executors
Defaults to SingleThreadedExecutor in non-isolated mode
Adds support for the new Callback Group Events Executor introduced in feat: Added callback group events executor #3097
Adds a debug message detailing what executor the component container was spawned with, if it's in isolated mode, and the number of threads if applicable

Is this user-facing behavior change?

When users use component_container_mt/component_container_event/component_container_isolated, they will see a warning message
When using component_container without any arguments, it will default to non isolated + single threaded, so users will probably not notice this change
Users will be able to use the Callback Group Events Executor in component containers
Users will be able to specify the number of threads used for MT / CBG executors in isolated component containers, making the refactored executable more featureful than component_container_isolated.cpp which just uses the max possible threads for each isolated executor

Did you use Generative AI?

@mini-1235 's changes were generated by github copilot. My subsequent changes were initially generated by Claude Sonnet 4.6, and then manually cleaned up afterwards.

Additional Information

Depends on #3097
Successor PR to #3129
Supersedes #3055
Companion PR which adds frontend tests to launch_ros: ros2/launch_ros#536

mini-1235

Thank you so much @skyegalaxy, this looks good to me!

mjcarroll · 2026-04-19T15:02:24Z

+    exec = std::make_shared<rclcpp::executors::SingleThreadedExecutor>();
+    switch (args.executor_type) {
+      case ExecutorType::MultiThreaded:
+        node = std::make_shared<


So we have already created a node to read the thread_num param. In this case, std::make_shared will make another node before moving it into node. There could be a very small window where two component manager nodes exist here. Can we just defer here somehow?

Claude originally tried to get around this by creating a --num-threads cli arg which was shadowed by the ROS param, but I felt like having the two ways of configuring threads left component_container.cpp feeling cluttered and that it may be another source of confusion. I figured the cost of destroying and rebuilding the node was negligible for the sake of cleaner reading code

@mjcarroll - how about 0bd014e ?

Following up on this, I think the creation of a temporary ComponentManager to get thread_num created a race condition bug.

When several isolated containers start concurrently and each has a LoadComposableNodes request waiting on it (the normal launch case), some composables silently never load. It's timing-dependent, so it shows up under real load (on hardware / busy CI) and is easy to miss on a warm single-host machine.

My hypothesis on what's happening: ComponentManager's constructor advertises ~/_container/load_node under the container's remapped name. So the load service flaps: advertised (throwaway) → destroyed → re-advertised (real, only served at spin()). A LoadComposableNodes client that is already waiting can have wait_for_service() catch the throwaway, then call_async() lands in the teardown gap and the request is dropped — the node never loads.

I Claude-coded a fix that doesn't involve creating a new ComponentManager and that seems to fix the issue at hand:
rolling...botsandus:rclcpp:race-condition-load-composable
I didn't spend more time validating the approach though.

I also made a test in launch_ros to reproduce the issue:
ros2/launch_ros#563
Note that it will only reproduce under heavy CPU load, alternatively, you could use that branch in rclcpp: rolling...botsandus:rclcpp:add-delay-composable
which adds a sleep, simulating heavy CPU load and making it more likely to occur.

@skyegalaxy would you be able to follow up on this?

FYI @doisyg

thanks for reporting this @tonynajjar. I took a look at your branch, and aside from one comment that feels a bit claude-y (about why we're NOT creating two component containers) I think it looks sound. Could you open a pull request to rolling with your branch?

Thanks for the prompt look, will open a PR tomorrow

mjcarroll · 2026-04-19T15:05:25Z

+
+  if (args.executor_type == ExecutorType::SingleThreaded &&
+    num_threads > 0 &&
+    num_threads < std::thread::hardware_concurrency())


If user sets the number of threads more than hardware_concurrency, the check will fail. Why would someone do that? IDK, hilarious.

The intention here was basically "if the user is manually specifying thread_num and they're using the SingleThreadedExecutor, give a warning". Since the thread_num ROS param ranges from 1 to hardware_concurrency and defaults to hardware_concurrency, specifying 0 as a default wouldn't work (even though for any executors with multiple threads, 0 resolves to hardware_concurrency anyway) and since the param is always defaulted to hardware_concurrency, it would give the warning every time you use the SingleThreadedExecutor regardless of if thread_num was specified.

I can modify the check to num_threads != std::thread::hardware_concurrency() and that ought to achieve the same effect though

mjcarroll

Two small nits here

knmcguire · 2026-04-19T15:14:29Z

Hope this is the correct way?

Pulls: #3134, ros2/launch_ros#536
Gist: https://gist.githubusercontent.com/knmcguire/324f76b97c2a23fbcce926452befc3f9/raw/39fc1c78ff8c571f56bef07d4c6f241912c63f93/ros2.repos
BUILD args: --continue-on-error --packages-above-and-dependencies rclcpp launch_ros
TEST args: --packages-above rclcpp launch_ros
ROS Distro: rolling
Job: ci_launcher
ci_launcher ran: https://ci.ros2.org/job/ci_launcher/19001

Linux
Linux-aarch64
Linux-rhel
Windows

skyegalaxy · 2026-04-19T15:24:24Z

Hope this is the correct way?

Pulls: #3134, ros2/launch_ros#536 Gist: https://gist.githubusercontent.com/knmcguire/324f76b97c2a23fbcce926452befc3f9/raw/39fc1c78ff8c571f56bef07d4c6f241912c63f93/ros2.repos BUILD args: --continue-on-error --packages-above-and-dependencies rclcpp launch_ros TEST args: --packages-above rclcpp launch_ros ROS Distro: rolling Job: ci_launcher ci_launcher ran: https://ci.ros2.org/job/ci_launcher/19001

* Linux [![Build Status](https://camo.githubusercontent.com/f8c647fe9ec2e755107378607f523b07a5abe581e7b4b32e5ba2dd38594c4c15/687474703a2f2f63692e726f73322e6f72672f6275696c645374617475732f69636f6e3f6a6f623d63695f6c696e7578266275696c643d3238373635)](http://ci.ros2.org/job/ci_linux/28765/)

* Linux-aarch64 [![Build Status](https://camo.githubusercontent.com/b277bb25541e829f326a09ae9fa6c9e464b9cc5250fea6ef45303801238d0f7f/687474703a2f2f63692e726f73322e6f72672f6275696c645374617475732f69636f6e3f6a6f623d63695f6c696e75782d61617263683634266275696c643d3231353834)](http://ci.ros2.org/job/ci_linux-aarch64/21584/)

* Linux-rhel [![Build Status](https://camo.githubusercontent.com/23655a2be9fc7cee6c625f96b86f42186795cda7730a5f881d086ca764ec7853/687474703a2f2f63692e726f73322e6f72672f6275696c645374617475732f69636f6e3f6a6f623d63695f6c696e75782d7268656c266275696c643d38363037)](http://ci.ros2.org/job/ci_linux-rhel/8607/)

* Windows [![Build Status](https://camo.githubusercontent.com/c9ba0e3a02930c32a4acf843e3dfc78dd560faf85ef39befe1ef5842e6af4c9b/687474703a2f2f63692e726f73322e6f72672f6275696c645374617475732f69636f6e3f6a6f623d63695f77696e646f7773266275696c643d3237363332)](http://ci.ros2.org/job/ci_windows/27632/)

@knmcguire - that is the correct way, but I'm waiting on @jmachowinski 's CBG Executor PR to merge first because this one is dependent on it

knmcguire · 2026-04-19T15:29:12Z

ah oke then they will probably fail. I'll stop them to offload ci

Signed-off-by: mini-1235 <mauricepurnawan@gmail.com>

…er_isolated Signed-off-by: Skyler Medeiros <skye@polymathrobotics.com>

Signed-off-by: Skyler Medeiros <skye@polymathrobotics.com>

…ram for backwards compatibility Signed-off-by: Skyler Medeiros <skye@polymathrobotics.com>

Signed-off-by: Skyler Medeiros <skye@polymathrobotics.com>

skyegalaxy · 2026-04-19T19:24:13Z

Pulls: #3134, ros2/launch_ros#536
Gist: https://gist.githubusercontent.com/knmcguire/324f76b97c2a23fbcce926452befc3f9/raw/39fc1c78ff8c571f56bef07d4c6f241912c63f93/ros2.repos
BUILD args: --continue-on-error --packages-above-and-dependencies rclcpp launch_ros
TEST args: --packages-above rclcpp launch_ros
ROS Distro: rolling
Job: ci_launcher
ci_launcher ran: https://ci.ros2.org/job/ci_launcher/19005/

Linux
Linux-aarch64
Linux-rhel
Windows

fujitatomoya

@skyegalaxy thanks a lot for this PR! this has been off the top of my head for a long time, but now it is much cleaner 👍

* Refactor component containers Signed-off-by: mini-1235 <mauricepurnawan@gmail.com> * Format Signed-off-by: mini-1235 <mauricepurnawan@gmail.com> * add new constructor, node_options_ and thread_num_ to component_manager_isolated Signed-off-by: Skyler Medeiros <skye@polymathrobotics.com> * make linter happy Signed-off-by: Skyler Medeiros <skye@polymathrobotics.com> * replace experimental with EventsCBG Signed-off-by: Skyler Medeiros <skye@polymathrobotics.com> * add make_executor helper Signed-off-by: Skyler Medeiros <skye@polymathrobotics.com> * add num_threads as a cli arg Signed-off-by: Skyler Medeiros <skye@polymathrobotics.com> * refactor main to use executor helper func. preserve thread_num ros param for backwards compatibility Signed-off-by: Skyler Medeiros <skye@polymathrobotics.com> * update help msg Signed-off-by: Skyler Medeiros <skye@polymathrobotics.com> * unused include Signed-off-by: Skyler Medeiros <skye@polymathrobotics.com> * update deprecation warning Signed-off-by: Skyler Medeiros <skye@polymathrobotics.com> * remove --num-threads CLI arg and use the ROS param Signed-off-by: Skyler Medeiros <skye@polymathrobotics.com> * remove redundant warning Signed-off-by: Skyler Medeiros <skye@polymathrobotics.com> * lint Signed-off-by: Skyler Medeiros <skye@polymathrobotics.com> * update help msg Signed-off-by: Skyler Medeiros <skye@polymathrobotics.com> * use regular node constructor for single-threaded Signed-off-by: Skyler Medeiros <skye@polymathrobotics.com> * match param name in warning msg Signed-off-by: Skyler Medeiros <skye@polymathrobotics.com> * lint again Signed-off-by: Skyler Medeiros <skye@polymathrobotics.com> * check for != default instead of < default Signed-off-by: Skyler Medeiros <skye@polymathrobotics.com> * nullify node before reassignment if isolated Signed-off-by: Skyler Medeiros <skye@polymathrobotics.com> --------- Signed-off-by: mini-1235 <mauricepurnawan@gmail.com> Signed-off-by: Skyler Medeiros <skye@polymathrobotics.com> Co-authored-by: mini-1235 <mauricepurnawan@gmail.com> Co-authored-by: Skyler Medeiros <skye@polymathrobotics.com>

skyegalaxy self-assigned this Apr 19, 2026

skyegalaxy mentioned this pull request Apr 19, 2026

Add tests for new component container refactor ros2/launch_ros#536

Merged

skyegalaxy requested a review from mjcarroll April 19, 2026 05:02

skyegalaxy mentioned this pull request Apr 19, 2026

Refactor component containers #3129

Closed

mini-1235 approved these changes Apr 19, 2026

View reviewed changes

mini-1235 mentioned this pull request Apr 19, 2026

Refactor component containers + Add option for CBG Executor ros2/ros2_documentation#6401

Merged

mjcarroll reviewed Apr 19, 2026

View reviewed changes

skyegalaxy force-pushed the skyegalaxy/component-container-refactor-cbg branch from cd1309d to 80629f1 Compare April 19, 2026 15:36

skyegalaxy mentioned this pull request Apr 19, 2026

Add component container for EventsCBGExecutor + Isolated component container for all events based executors #3123

Closed

skyegalaxy requested a review from mjcarroll April 19, 2026 16:19

mini-1235 and others added 15 commits April 19, 2026 12:21

Refactor component containers

f0fb9c9

Signed-off-by: mini-1235 <mauricepurnawan@gmail.com>

Format

ff7e25e

Signed-off-by: mini-1235 <mauricepurnawan@gmail.com>

add new constructor, node_options_ and thread_num_ to component_manag…

7b684e8

…er_isolated Signed-off-by: Skyler Medeiros <skye@polymathrobotics.com>

make linter happy

565b9ad

Signed-off-by: Skyler Medeiros <skye@polymathrobotics.com>

replace experimental with EventsCBG

d59094b

Signed-off-by: Skyler Medeiros <skye@polymathrobotics.com>

add make_executor helper

8172c03

Signed-off-by: Skyler Medeiros <skye@polymathrobotics.com>

add num_threads as a cli arg

3f35892

Signed-off-by: Skyler Medeiros <skye@polymathrobotics.com>

refactor main to use executor helper func. preserve thread_num ros pa…

5aea21f

…ram for backwards compatibility Signed-off-by: Skyler Medeiros <skye@polymathrobotics.com>

update help msg

8b7eadb

Signed-off-by: Skyler Medeiros <skye@polymathrobotics.com>

unused include

27e09ca

Signed-off-by: Skyler Medeiros <skye@polymathrobotics.com>

update deprecation warning

2ad9585

Signed-off-by: Skyler Medeiros <skye@polymathrobotics.com>

remove --num-threads CLI arg and use the ROS param

b125c57

Signed-off-by: Skyler Medeiros <skye@polymathrobotics.com>

remove redundant warning

2bb8a7f

Signed-off-by: Skyler Medeiros <skye@polymathrobotics.com>

lint

44d8b3f

Signed-off-by: Skyler Medeiros <skye@polymathrobotics.com>

update help msg

2ec9c60

Signed-off-by: Skyler Medeiros <skye@polymathrobotics.com>

Skyler Medeiros added 5 commits April 19, 2026 12:21

use regular node constructor for single-threaded

d80709d

Signed-off-by: Skyler Medeiros <skye@polymathrobotics.com>

match param name in warning msg

829209a

Signed-off-by: Skyler Medeiros <skye@polymathrobotics.com>

lint again

a99095c

Signed-off-by: Skyler Medeiros <skye@polymathrobotics.com>

check for != default instead of < default

ad73c4f

Signed-off-by: Skyler Medeiros <skye@polymathrobotics.com>

nullify node before reassignment if isolated

f39a5a2

Signed-off-by: Skyler Medeiros <skye@polymathrobotics.com>

skyegalaxy force-pushed the skyegalaxy/component-container-refactor-cbg branch from 0bd014e to f39a5a2 Compare April 19, 2026 19:21

skyegalaxy requested a review from jmachowinski April 19, 2026 19:21

jmachowinski approved these changes Apr 20, 2026

View reviewed changes

mjcarroll approved these changes Apr 20, 2026

View reviewed changes

skyegalaxy merged commit 72c5209 into rolling Apr 20, 2026
3 of 4 checks passed

fujitatomoya reviewed Apr 21, 2026

View reviewed changes

armaho mentioned this pull request Jun 6, 2026

Add the ability to specify the number of threads for a component when using component_container_isolated #2930

Closed

fujitatomoya mentioned this pull request Jun 8, 2026

Handling dedicated executor for nodes inside ComponentManager itself rather than ComponentManagerIsolated #3080

Closed

tonynajjar mentioned this pull request Aug 11, 2026

Race condition when loading multiple isolated component_container #3222

Closed

Uh oh!

Conversation

skyegalaxy commented Apr 19, 2026 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Description

Is this user-facing behavior change?

Did you use Generative AI?

Additional Information

Uh oh!

mini-1235 left a comment

Choose a reason for hiding this comment

Uh oh!

mjcarroll Apr 19, 2026

Choose a reason for hiding this comment

Uh oh!

skyegalaxy Apr 19, 2026 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Choose a reason for hiding this comment

Uh oh!

skyegalaxy Apr 19, 2026

Choose a reason for hiding this comment

Uh oh!

tonynajjar Aug 11, 2026 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Choose a reason for hiding this comment

Uh oh!

skyegalaxy Aug 11, 2026

Choose a reason for hiding this comment

Uh oh!

tonynajjar Aug 11, 2026

Choose a reason for hiding this comment

Uh oh!

mjcarroll Apr 19, 2026

Choose a reason for hiding this comment

Uh oh!

skyegalaxy Apr 19, 2026 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Choose a reason for hiding this comment

Uh oh!

skyegalaxy Apr 19, 2026

Choose a reason for hiding this comment

Uh oh!

mjcarroll left a comment

Choose a reason for hiding this comment

Uh oh!

knmcguire commented Apr 19, 2026

Uh oh!

skyegalaxy commented Apr 19, 2026 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

knmcguire commented Apr 19, 2026

Uh oh!

skyegalaxy commented Apr 19, 2026

Uh oh!

Uh oh!

fujitatomoya left a comment

Choose a reason for hiding this comment

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

7 participants

skyegalaxy commented Apr 19, 2026 •

edited

Loading

skyegalaxy Apr 19, 2026 •

edited

Loading

tonynajjar Aug 11, 2026 •

edited

Loading

skyegalaxy Apr 19, 2026 •

edited

Loading

skyegalaxy commented Apr 19, 2026 •

edited

Loading