ros2 launch with ComposableNodes doesn't terminate on SIGTERM

[ksuszka]

Bug report

Required Info:

• Operating System:
  • Ubuntu 20.04
• Installation type:
  • official docker ros:foxy and ros:rolling
• Version or commit hash:
  • N/A
• DDS implementation:
  • rmw_fastrtps_cpp
• Client library (if applicable):
  • ros-*-demo-nodes-cpp used as an example

Steps to reproduce issue

1. Create test.launch.py launch file with composable node:

from launch import LaunchDescription
from launch_ros.actions import ComposableNodeContainer
from launch_ros.descriptions import ComposableNode

def generate_launch_description():
    nodes = ComposableNodeContainer(
        name="localization_container",
        namespace="",
        package="rclcpp_components",
        executable="component_container",
        composable_node_descriptions=[
            ComposableNode(
                package='demo_nodes_cpp',
                plugin='demo_nodes_cpp::Talker',
                name='talker')
        ],
        output="screen",
    )
    return LaunchDescription([nodes])

2. Run this file by ros2 launch test.launch.py
3. Find PID of running launch file
4. In second terminal issue command: kill -SIGTERM <pid>

Expected behavior

Launch process is terminated as it happens without composable node.

Actual behavior

Launch process outputs standard termination messages and then hangs.

[ERROR] [launch]: using SIGTERM or SIGQUIT can result in orphaned processes
[ERROR] [launch]: make sure no processes launched are still running
[ERROR] [launch]: run task was canceled

Additional information

I've observed the same behaviour for ros:foxy and ros:rolling images.

[mjeronimo]

I was able to reproduce this with Patch Release 4 of Foxy, but not the latest Rolling release (https://ci.ros2.org/job/ci_packaging_linux/440/).

@jacobperron Any idea what might have been fixed? Seems like something that could be back-ported (or released) to Foxy.

[jacobperron]

I recall there were a bunch of issues with ComposableNodes and launch that I helped fix since Foxy. I tried to start backporting them but ran into conflicts (there were some major refactors). Although, that being said, this sounds like it may be related to launch (not launch_ros). There are a bunch a somewhat recent changes related to signal handling in launch. It's a shot in the dark, but maybe related to ros2/launch#476 ?

[mjeronimo]

@jacobperron Yes, that's the one! I tested on Foxy and that particular commit fixes this issue.

[clalancette]

I believe that this has been fixed in Foxy, so closing this out. If you are still having problems with this, please feel free to reopen.

[ciandonovan]

This is still broken in Foxy and above, Launch doesn't install a POSIX signal handler for SIGTERM, which means the async function to handle it is never called. Normally that means Launch is terminated immediately leaving an orphaned child ROS node, or if it's run in a container, the signal is ignored completely as it's PID1 and no explicit handler was installed.

Instructions to reproduce:
ros2/launch#666

and proposed pull-request to fix it:
ros2/launch#712