set thread names by node in component container isolated by Aposhian · Pull Request #2871 · ros2/rclcpp

Aposhian · 2025-06-11T15:45:08Z

Description

Set the thread name of each node's thread in component_container_isolated. This should support POSIX platforms, Apple, and Windows. I tested on Linux only so far.

Fixes #2818

Is this user-facing behavior change?

User's may see an additional warning if the thread name couldn't set for whatever reason. If users are looking at custom thread names, then they will see a difference.

Did you use Generative AI?

Yes with Cursor autocomplete.

Additional Information

Signed-off-by: Adam Aposhian <adam.aposhian@fireflyautomatix.com>

ahcorde

is there any way to add a test?

rclcpp/include/rclcpp/detail/os_thread.hpp

rclcpp/src/rclcpp/detail/os_thread.cpp

Signed-off-by: Adam Aposhian <adam.aposhian@fireflyautomatix.com>

Aposhian · 2025-06-11T16:26:19Z

Add test

Signed-off-by: Adam Aposhian <adam.aposhian@fireflyautomatix.com>

Aposhian · 2025-06-11T17:20:34Z

Ok, I added a get_thread_name method, as well as tests for each platform. I have only tested the Linux test so far.

Signed-off-by: Adam Aposhian <adam.aposhian@fireflyautomatix.com>

fujitatomoya · 2025-06-13T22:22:45Z

rclcpp/include/rclcpp/detail/os_thread.hpp

@@ -0,0 +1,42 @@
+// Copyright 2025 Open Source Robotics Foundation, Inc.


1st of all, i would highly recommend to move this APIs to rcpputils that is where those utilities are maintained in c++ language. so that the user application can use those utilities without rclcpp dependency.

Sounds good.

move os_thread to rcpputils

I will resolve the requested changes here first, and then move it over.

fujitatomoya · 2025-06-13T22:26:47Z

rclcpp/src/rclcpp/detail/os_thread.cpp

+  rc = pthread_setname_np(pthread_self(), truncated_name.c_str());
+#endif
+  if (rc != 0) {
+        // Don't throw since this is not critical


indent is off?

Suggested change

// Don't throw since this is not critical

// Don't throw since this is not critical

fujitatomoya · 2025-06-13T22:32:22Z

rclcpp/src/rclcpp/detail/os_thread.cpp

+#endif
+  if (rc != 0) {
+        // Don't throw since this is not critical
+    RCLCPP_WARN(rclcpp::get_logger("rclcpp"), "Failed to set thread name: %s", name.c_str());


how about calling strerror(errno) to print more information from the system?

Suggested change

RCLCPP_WARN(rclcpp::get_logger("rclcpp"), "Failed to set thread name: %s", name.c_str());

RCLCPP_WARN(rclcpp::get_logger("rclcpp"), "Failed to set thread name(%s): %s", name.c_str(), strerror(errno));

print OS error information

fujitatomoya · 2025-06-13T22:40:38Z

rclcpp_components/include/rclcpp_components/component_manager_isolated.hpp

    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();


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.

You mean to the docstring of set_thread_name or elsewhere?

Document truncation behavior

fujitatomoya · 2025-06-14T04:13:49Z

rclcpp/src/rclcpp/detail/os_thread.cpp

+  // Truncate name to maximum length supported by pthread_setname_np
+  // leaving one character for the null terminator
+  // otherwise pthread_setname_np will return an ERANGE error
+  std::string truncated_name = name.substr(0, MAXTHREADNAMESIZE - 1);


it would be still better to add the warning if truncation happens on this call? so that user application can know the actual thread name to be set to the Executor.

That's a good idea, to log the actual name set if it was truncated.

log truncated name

I ended up removing the log of the truncated name since I wasn't sure how to go about that inside of the rcpputils project. Any suggestions?

Signed-off-by: Adam Aposhian <adam.aposhian@fireflyautomatix.com>

Aposhian · 2025-06-16T20:31:39Z

I copied the thread name utilities and test over to rcpputils: ros2/rcpputils#213.

Signed-off-by: Adam Aposhian <adam.aposhian@fireflyautomatix.com>

ahcorde · 2025-06-17T09:16:03Z

rclcpp_components/include/rclcpp_components/component_manager_isolated.hpp

+      [exec, &thread_initialized, name]() {
+        try {
+          rcpputils::set_thread_name(name);
+        } catch (const std::system_error & e) {


include <system_error>

include system error

Signed-off-by: Adam Aposhian <adam.aposhian@fireflyautomatix.com>

ahcorde · 2025-06-18T05:59:27Z

Pulls: ros2/rcpputils#213, #2871
Gist: https://gist.githubusercontent.com/ahcorde/55c4014e22192d458adca070dac5e92d/raw/879545f9eaba7f535360c0ac4bfb442f4f33868e/ros2.repos
BUILD args: --packages-up-to rclcpp_components rcpputils --packages-above-and-dependencies rclcpp_components rcpputils
TEST args: --packages-select rclcpp_components rcpputils --packages-above rclcpp_components rcpputils
ROS Distro: rolling
Job: ci_launcher
ci_launcher ran: https://ci.ros2.org/job/ci_launcher/16245

Linux
Linux-aarch64
Linux-rhel
Windows

ahcorde · 2025-06-18T13:19:34Z

Pulls: #2871, ros2/rcpputils#213
Gist: https://gist.githubusercontent.com/ahcorde/883710a7fcaac7eb9684fc51a451c8c6/raw/06341f2c44e076f6ce82aad3d2da3d6fdfb3e9b1/ros2.repos
BUILD args: --packages-up-to rclcpp rcpputils --packages-above-and-dependencies rclcpp rcpputils
TEST args: --packages-select rclcpp rcpputils --packages-above rclcpp rcpputils
ROS Distro: rolling
Job: ci_launcher
ci_launcher ran: https://ci.ros2.org/job/ci_launcher/16259

Linux
Linux-aarch64
Linux-rhel
Windows

set thread names by node in component container isolated

6cb3050

Signed-off-by: Adam Aposhian <adam.aposhian@fireflyautomatix.com>

ahcorde requested changes Jun 11, 2025

View reviewed changes

rclcpp/include/rclcpp/detail/os_thread.hpp Outdated Show resolved Hide resolved

rclcpp/src/rclcpp/detail/os_thread.cpp Outdated Show resolved Hide resolved

fix formatting

3967b72

Signed-off-by: Adam Aposhian <adam.aposhian@fireflyautomatix.com>

Aposhian force-pushed the 2818-custom-thread-names branch from 165183b to 3967b72 Compare June 11, 2025 16:11

Aposhian force-pushed the 2818-custom-thread-names branch from 6fdf8e5 to 27dfcd2 Compare June 11, 2025 17:19

add get_thread_name and tests

cc18cde

Signed-off-by: Adam Aposhian <adam.aposhian@fireflyautomatix.com>

Aposhian force-pushed the 2818-custom-thread-names branch from 27dfcd2 to cc18cde Compare June 11, 2025 17:19

remove unnecessary include

26a8a9b

Signed-off-by: Adam Aposhian <adam.aposhian@fireflyautomatix.com>

Aposhian force-pushed the 2818-custom-thread-names branch from 5f02599 to 26a8a9b Compare June 11, 2025 18:19

fix cpplint errors

4adb111

Signed-off-by: Adam Aposhian <adam.aposhian@fireflyautomatix.com>

fujitatomoya requested changes Jun 14, 2025

View reviewed changes

Aposhian added 2 commits June 16, 2025 08:04

fix formatting

3dc401d

Signed-off-by: Adam Aposhian <adam.aposhian@fireflyautomatix.com>

throw with os error info

408d136

Signed-off-by: Adam Aposhian <adam.aposhian@fireflyautomatix.com>

Aposhian mentioned this pull request Jun 16, 2025

add thread naming utilities ros2/rcpputils#213

Merged

use rcpputils thread name utils

d2ab71c

Signed-off-by: Adam Aposhian <adam.aposhian@fireflyautomatix.com>

ahcorde requested changes Jun 17, 2025

View reviewed changes

add system_error include

7bf01ce

Signed-off-by: Adam Aposhian <adam.aposhian@fireflyautomatix.com>

ahcorde approved these changes Jun 18, 2025

View reviewed changes

ahcorde merged commit 1e95407 into ros2:rolling Jun 18, 2025
2 of 3 checks passed

		@@ -0,0 +1,42 @@
		// Copyright 2025 Open Source Robotics Foundation, Inc.

	// Don't throw since this is not critical
	// Don't throw since this is not critical

	RCLCPP_WARN(rclcpp::get_logger("rclcpp"), "Failed to set thread name: %s", name.c_str());
	RCLCPP_WARN(rclcpp::get_logger("rclcpp"), "Failed to set thread name(%s): %s", name.c_str(), strerror(errno));

Conversation

Aposhian commented Jun 11, 2025

Description

Is this user-facing behavior change?

Did you use Generative AI?

Additional Information

Uh oh!

ahcorde left a comment

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Uh oh!

Aposhian commented Jun 11, 2025 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

Aposhian commented Jun 11, 2025

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Aposhian Jun 16, 2025 • 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!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Aposhian Jun 16, 2025 • 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!

Choose a reason for hiding this comment

Uh oh!

Aposhian commented Jun 16, 2025

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Aposhian Jun 17, 2025 • 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!

ahcorde commented Jun 18, 2025

Uh oh!

ahcorde commented Jun 18, 2025

Uh oh!

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

3 participants

Aposhian commented Jun 11, 2025 •

edited

Loading

Aposhian Jun 16, 2025 •

edited

Loading

Aposhian Jun 16, 2025 •

edited

Loading

Aposhian Jun 17, 2025 •

edited

Loading