Skip to content

set thread names by node in component container isolated #2871

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Open
wants to merge 9 commits into
base: rolling
Choose a base branch
from
+11 −2
Open

set thread names by node in component container isolated #2871

Changes from all commits
Commits
Show all changes
9 commits
Select commit Hold shift + click to select a range
6cb3050
set thread names by node in component container isolated
Aposhian Jun 11, 2025
3967b72
fix formatting
Aposhian Jun 11, 2025
cc18cde
add get_thread_name and tests
Aposhian Jun 11, 2025
26a8a9b
remove unnecessary include
Aposhian Jun 11, 2025
4adb111
fix cpplint errors
Aposhian Jun 11, 2025
3dc401d
fix formatting
Aposhian Jun 16, 2025
408d136
throw with os error info
Aposhian Jun 16, 2025
d2ab71c
use rcpputils thread name utils
Aposhian Jun 16, 2025
7bf01ce
add system_error include
Aposhian Jun 17, 2025
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
13 changes: 11 additions & 2 deletions rclcpp_components/include/rclcpp_components/component_manager_isolated.hpp
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -22,9 +22,10 @@
#include <utility>
#include <vector>
#include <unordered_map>
#include <system_error>

#include "rclcpp_components/component_manager.hpp"

#include "rcpputils/thread_name.hpp"

namespace rclcpp_components
{
Expand Down Expand Up @@ -77,8 +78,16 @@ class ComponentManagerIsolated : public rclcpp_components::ComponentManager
DedicatedExecutorWrapper & wrapper = result.first->second;
wrapper.executor = exec;
auto & thread_initialized = wrapper.thread_initialized;
auto name = node_wrappers_[node_id].get_node_base_interface()->get_name();
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

we would want to add the docstring about this behavior including truncating the node name into 16bytes depending on the platform? it also can mention that if using MultiThreadedExecutor, all the threads in that will be named same.

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

You mean to the docstring of set_thread_name or elsewhere?

  • Document truncation behavior

// Copy name so that it doesn't deallocate before the thread is started
wrapper.thread = std::thread(
[exec, &thread_initialized]() {
[exec, &thread_initialized, name]() {
try {
rcpputils::set_thread_name(name);
} catch (const std::system_error & e) {
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

include <system_error>

Copy link
Contributor Author

@Aposhian Aposhian Jun 17, 2025 •
edited
Loading

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

  • include system error

RCLCPP_WARN(rclcpp::get_logger("rclcpp"), "Failed to set thread name: %s (%s)", e.what(),
e.code().message().c_str());
}
thread_initialized = true;
exec->spin();
});
Expand Down