ros2 / launch

Tools for launching multiple processes and for writing tests involving multiple processes.
Apache License 2.0
125 stars 139 forks source link

Launch doesn't stop running nodes when exception thrown #747

Open jackiescanlon opened 10 months ago

jackiescanlon commented 10 months ago

Bug report

Required Info:

Steps to reproduce issue

Create launch file my_launch.py

import launch
import launch_ros.actions
from launch import LaunchDescription

def generate_launch_description() -> LaunchDescription:
    return launch.LaunchDescription(
        [
            # Real nodes - these get left running when launch file throws exception
            launch_ros.actions.Node(
                package="examples_rclcpp_minimal_publisher",
                executable="publisher_member_function",
            ),
            launch_ros.actions.Node(
                package="examples_rclcpp_minimal_subscriber",
                executable="subscriber_member_function",
            ),
            # Fake node - this will cause an exception to be thrown.
            launch_ros.actions.Node(
                package="examples_rclcpp_minimal_subscriber",
                executable="fake_node",
            ),
        ]
    )

Launch it

$ ros2 launch my_launch.py

Expected behavior

Launch catches exception and stops, including stopping any nodes that were started before the exception was caught, i.e., ros2 node list returns nothing afterwards.

Actual behavior

Launch catches exception and stops, but doesn't kill any of the already started nodes.

$ ros2 launch my_launch.py
[INFO] [launch]: All log files can be found below /home/jackie/.ros/log/2023-12-05-14-26-16-767994-stealth-126383
[INFO] [launch]: Default logging verbosity is set to INFO
[ERROR] [launch]: Caught exception in launch (see debug for traceback): executable 'fake_node' not found on the libexec directory '/opt/ros/iron/lib/examples_rclcpp_minimal_subscriber' 
$ ros2 node list
/minimal_publisher
/minimal_subscriber

Additional information

Ideally of course the launch file is correctly written such that no exceptions are thrown in the first place. However, when in the process of writing a new launch file (especially one much larger than this example), it is hard to remember, unexpected and tedious to individually kill every node that was started before the exception was thrown.

In my own troubleshooting I looked at what happens when the launch file is Ctrl+C'd vs. what happens when an exception is caught. Perhaps there's something in there (namely, some behavior difference when LaunchService._shutdown() is passed due_to_sigint=True and force_sync=True)?

https://github.com/ros2/launch/blob/f15691c341d32e53970fa18a9a22c9206c2edfbe/launch/launch/launch_service.py#L191-L202

https://github.com/ros2/launch/blob/f15691c341d32e53970fa18a9a22c9206c2edfbe/launch/launch/launch_service.py#L345-L355

jackiescanlon commented 5 months ago

I've also discovered a similar bug occurs when a typo is made in the keyword arguments for a Node, for example, mistyping package as ackage.

mitchellsayer commented 1 month ago

bump. This issue is still present