Unload controller upon sigterm (#3406) (#3427)

Author: mergify[bot]

controller_manager/controller_manager/spawner.py

@@ -115,7 +115,7 @@ def parse_args_advanced(args):
         "-u",
         "--unload-on-kill",
         action="store_true",
-        help="Deactivate the active controllers and unload them on kill",
+        help="Deactivate the active controllers and unload them on SIGINT or SIGTERM",
     )
     global_parser.add_argument("-h", "--help", action="store_true", help="Show help")
 
@@ -253,7 +253,7 @@ def parse_native_args(args):
     parser.add_argument(
         "-u",
         "--unload-on-kill",
-        help="Wait until this application is interrupted and unload controller",
+        help="Wait until this application is interrupted (SIGINT or SIGTERM) and deactivate/unload controllers",
         action="store_true",
     )
     parser.add_argument(
@@ -372,6 +372,13 @@ def get_ros_params_files(argv):
     activate_as_group = global_args.activate_as_group
     unload_on_kill = global_args.unload_on_kill
     node = None
+    lock = None
+
+    def _on_shutdown_signal(signum, frame):
+        raise KeyboardInterrupt()
+
+    signal.signal(signal.SIGINT, _on_shutdown_signal)
+    signal.signal(signal.SIGTERM, _on_shutdown_signal)
 
     if spawner_ros_params_files:
         for controller in controllers:
@@ -604,6 +611,7 @@ def get_ros_params_files(argv):
         # second KeyboardInterrupt during the signal.signal() call itself.
         try:
             signal.signal(signal.SIGINT, signal.SIG_IGN)
+            signal.signal(signal.SIGTERM, signal.SIG_IGN)
         except (KeyboardInterrupt, Exception):
             pass
         if unload_on_kill:
@@ -662,7 +670,7 @@ def get_ros_params_files(argv):
     finally:
         if node:
             node.destroy_node()
-        if lock.is_locked:
+        if lock is not None and lock.is_locked:
             lock.release()
         rclpy.shutdown()
 

controller_manager/doc/userdoc.rst

@@ -187,7 +187,7 @@ There are two scripts to interact with controller manager from launch files:
                             DEPRECATED Namespace for the controller_manager and the controller(s)
       --load-only           Only load the controller and leave unconfigured.
       --inactive            Load and configure the controller, however do not activate them
-      -u, --unload-on-kill  Wait until this application is interrupted and unload controller
+      -u, --unload-on-kill  Wait until this application is interrupted (SIGINT or SIGTERM) and deactivate/unload controllers
       --controller-manager-timeout CONTROLLER_MANAGER_TIMEOUT
                             Time to wait for the controller manager service to be available
       --switch-timeout SWITCH_TIMEOUT
@@ -274,7 +274,7 @@ The ``spawner`` now supports per controller arguments, while parsing the argumen
       --activate-as-group   Activate controllers as a group
       --switch-asap, --no-switch-asap
                             Switch controllers as soon as possible
-      -u, --unload-on-kill  Deactivate the active controllers and unload them on kill
+      -u, --unload-on-kill  Deactivate the active controllers and unload them on SIGINT or SIGTERM
       -h, --help            Show help
 
     Controller Options:

controller_manager/test/test_spawner_unspawner.cpp

@@ -605,6 +605,25 @@ TEST_F(TestLoadController, unload_on_kill_activate_as_group)
   ASSERT_EQ(cm_->get_loaded_controllers().size(), 0ul);
 }
 
+TEST_F(TestLoadController, unload_on_kill_with_sigterm)
+{
+  // When a launch file shuts down because a required sibling process crashes,
+  // it sends SIGINT and escalates to SIGTERM, --unload-on-kill must still
+  // deactivate and unload the controller when SIGTERM is delivered.
+  ControllerManagerRunner cm_runner(this);
+  cm_->set_parameter(rclcpp::Parameter("ctrl_3.type", test_controller::TEST_CONTROLLER_CLASS_NAME));
+  std::stringstream ss;
+  ss << "timeout --signal=TERM 5 "
+     << std::string(coveragepy_script) +
+          " $(ros2 pkg prefix controller_manager)/lib/controller_manager/spawner "
+     << "ctrl_3 -c test_controller_manager --unload-on-kill";
+
+  EXPECT_NE(std::system(ss.str().c_str()), 0)
+    << "timeout should have killed spawner and returned non 0 code";
+
+  ASSERT_EQ(cm_->get_loaded_controllers().size(), 0ul);
+}
+
 TEST_F(TestLoadController, spawner_test_to_check_parameter_overriding)
 {
   const std::string main_test_file_path =