check thread whether joinable before join

[uupks]

If rclcpp is inited with rclcpp::SignalHandlerOptions::None, signal_handler_thread will not be created.

Checking thread whether is joinable before join is a solution to this little bug.

[fujitatomoya]

lgtm

[fujitatomoya]

This is clearly bug, that could raise std::system_error with Invalid argument.
Test would be ideal.

[fujitatomoya]

@clalancette could you take a look at this?

[alsora]

Looks correct to me.
+1 on adding a unit test

[clalancette]

This looks good to me, fixes an obvious bug.

While adding a test would be nice, it is not at all clear to me how to do that. We don't have any existing tests for the SignalHandler class, and they would be tricky to write (particularly in a portable way). So I'm fine with this going in as-is; I'll run CI on it next.

[clalancette]

CI:

• Linux
• Linux-aarch64
• Windows

[clalancette]

The one failure on Windows is a known flake, so I'm going to go ahead with this. Thanks for the contribution.

[shuhaowu]

Is there a way to workaround this problem on Humble? The best way I've found is to override the signal handler with a custom one then call rclcpp::shutdown, but this hits this comment:

    // TODO(wjwwood): what happens if someone overrides our signal handler then calls uninstall?
    //   I think we need to assert that we're the current signal handler, and mitigate if not.

So the way to deal with this would be:

1. Register signal handler in rclcpp::init
2. Register your own signal handler which overrides the one in rclcpp::init
3. Let the custom signal handler handle the signal
4. Call rclcpp::shutdown, which will override the custom signal handler but it's OK at this point.

[fujitatomoya]

@shuhaowu that could work, but straightforward way is to backport this to humble?

[fujitatomoya]

@Mergifyio backport humble

[mergify]

backport humble

✅ Backports have been created

• #2275 check thread whether joinable before join (backport #2019) has been created for branch humble