diff --git a/rclcpp_components/include/rclcpp_components/component_manager_isolated.hpp b/rclcpp_components/include/rclcpp_components/component_manager_isolated.hpp index 4d22d9c943..44d61fe304 100644 --- a/rclcpp_components/include/rclcpp_components/component_manager_isolated.hpp +++ b/rclcpp_components/include/rclcpp_components/component_manager_isolated.hpp @@ -102,7 +102,12 @@ class ComponentManagerIsolated : public rclcpp_components::ComponentManager if constexpr (std::is_same_v) { exec = std::make_shared(executor_options_); } else { - exec = std::make_shared(executor_options_, num_threads_); + // num_threads_ == 0 means "auto": read the `thread_num` parameter declared by + // ComponentManager (defaults to the hardware concurrency). + const size_t num_threads = num_threads_ != 0 ? + num_threads_ : + static_cast(this->get_parameter("thread_num").as_int()); + exec = std::make_shared(executor_options_, num_threads); } exec->add_node(node_wrappers_[node_id].get_node_base_interface()); diff --git a/rclcpp_components/src/component_container.cpp b/rclcpp_components/src/component_container.cpp index b671531123..c2a66625e8 100644 --- a/rclcpp_components/src/component_container.cpp +++ b/rclcpp_components/src/component_container.cpp @@ -143,46 +143,48 @@ int main(int argc, char * argv[]) } std::shared_ptr exec; - std::shared_ptr node = - std::make_shared(); - const int64_t num_threads = (node->has_parameter("thread_num")) ? - node->get_parameter("thread_num").as_int() : - std::thread::hardware_concurrency(); - std::string debug_msg; + std::shared_ptr node; - if (args.executor_type == ExecutorType::SingleThreaded && - num_threads > 0 && - num_threads != std::thread::hardware_concurrency()) - { - RCUTILS_LOG_WARN_NAMED("component_container", - "thread_num is not supported by the SingleThreadedExecutor. Ignoring..."); - } + // Create the manager once, as its final type. The isolated managers resolve the + // `thread_num` parameter themselves when a component is loaded (0 == auto). if (args.isolated) { - // we use the ComponentManager node initially to get the `thread_num` parameter, - // but temporarily delete it here before re-assigning it - // if running with a ComponentManagerIsolated. - // This is to avoid a possible race condition - // where the 2 nodes may be briefly alive at the same time, - node = nullptr; - // The outer executor runs only the container manager's load/unload services. - // Each loaded component gets its own dedicated executor of the requested type. - exec = std::make_shared(); switch (args.executor_type) { case ExecutorType::MultiThreaded: node = std::make_shared< rclcpp_components::ComponentManagerIsolated>( - rclcpp::ExecutorOptions(), num_threads); + rclcpp::ExecutorOptions(), 0); break; case ExecutorType::EventsCBG: node = std::make_shared< rclcpp_components::ComponentManagerIsolated>( - rclcpp::ExecutorOptions(), num_threads); + rclcpp::ExecutorOptions(), 0); break; default: node = std::make_shared< rclcpp_components::ComponentManagerIsolated>(); break; } + } else { + node = std::make_shared(); + } + + const int64_t num_threads = (node->has_parameter("thread_num")) ? + node->get_parameter("thread_num").as_int() : + std::thread::hardware_concurrency(); + + if (args.executor_type == ExecutorType::SingleThreaded && + num_threads > 0 && + num_threads != std::thread::hardware_concurrency()) + { + RCUTILS_LOG_WARN_NAMED("component_container", + "thread_num is not supported by the SingleThreadedExecutor. Ignoring..."); + } + + std::string debug_msg; + if (args.isolated) { + // The outer executor runs only the container manager's load/unload services. + // Each loaded component gets its own dedicated executor of the requested type. + exec = std::make_shared(); debug_msg = "Creating isolated component container with the following per-node settings: " "executor_type: " + executor_type_to_string(args.executor_type); diff --git a/rclcpp_components/test/test_component_manager_api.cpp b/rclcpp_components/test/test_component_manager_api.cpp index a8d1b4e768..48e234c4be 100644 --- a/rclcpp_components/test/test_component_manager_api.cpp +++ b/rclcpp_components/test/test_component_manager_api.cpp @@ -23,6 +23,7 @@ #include "composition_interfaces/srv/list_nodes.hpp" #include "rclcpp/executors/events_cbg_executor/events_cbg_executor.hpp" +#include "rclcpp/executors/multi_threaded_executor.hpp" #include "rclcpp_components/component_manager.hpp" #include "rclcpp_components/component_manager_isolated.hpp" @@ -420,3 +421,33 @@ TEST_F(TestComponentManager, no_throw_remove_node_twice_on_shutdown) manager->remove_all_nodes_from_executor(); EXPECT_NO_THROW(manager.reset()); } + +// An isolated manager templated on a non-single-threaded executor and built +// without an explicit thread count (num_threads_ == 0, "auto") must resolve the +// `thread_num` parameter when loading a component instead of handing 0 to the +// executor. +TEST_F(TestComponentManager, isolated_multi_threaded_auto_thread_num) +{ + auto exec = std::make_shared(); + using IsolatedMultiThreaded = + rclcpp_components::ComponentManagerIsolated; + auto manager = std::make_shared(exec); + auto client_node = rclcpp::Node::make_shared("test_component_manager_isolated_mt"); + + exec->add_node(manager); + exec->add_node(client_node); + + auto composition_client = client_node->create_client( + "/ComponentManager/_container/load_node"); + ASSERT_TRUE(composition_client->wait_for_service(20s)) << "service not available after waiting"; + + auto request = std::make_shared(); + request->package_name = "rclcpp_components"; + request->plugin_name = "test_rclcpp_components::TestComponentFoo"; + auto future = composition_client->async_send_request(request); + ASSERT_EQ(exec->spin_until_future_complete(future, 5s), rclcpp::FutureReturnCode::SUCCESS); + auto result = future.get(); + EXPECT_TRUE(result->success); + EXPECT_EQ(result->error_message, ""); + EXPECT_EQ(result->full_node_name, "/test_component_foo"); +}