fix(components): correctly go through shutdown sequence (#157)

* fix(components): correctly go through shutdown sequence * fix: join execute thread on destruction
aica-technology · Oct 9, 2024 · cbb682f · cbb682f
1 parent 1b8d582
commit cbb682f
Show file tree

Hide file tree

Showing 5 changed files with 29 additions and 7 deletions.
diff --git a/CHANGELOG.md b/CHANGELOG.md
@@ -48,6 +48,7 @@ Release Versions:
 - feat!: add support for custom inputs and outputs (#133)
 - release: version v5.0.0-rc0009 (#155)
 - refactor(components): improve component error handling (#138)
+- fix(components): correctly go through shutdown sequence (#157)
 
 ## 4.2.2
 

diff --git a/aica-package.toml b/aica-package.toml
@@ -1,7 +1,7 @@
 #syntax=ghcr.io/aica-technology/package-builder:v1.1.1
 
 [metadata]
-version = "5.0.0-rc0010"
+version = "5.0.0-rc0011"
 description = "Modular ROS 2 extension library for dynamic composition of components and controllers with the AICA robotics framework"
 
 [metadata.collection]

diff --git a/source/modulo_components/include/modulo_components/Component.hpp b/source/modulo_components/include/modulo_components/Component.hpp
@@ -32,9 +32,9 @@ class Component : public rclcpp::Node, public ComponentInterface {
   explicit Component(const rclcpp::NodeOptions& node_options, const std::string& fallback_name = "Component");
 
   /**
-   * @brief Virtual default destructor.
+   * @brief Destructor that joins the thread if necessary.
    */
-  virtual ~Component() = default;
+  virtual ~Component();
 
 protected:
   /**

diff --git a/source/modulo_components/modulo_components/lifecycle_component.py b/source/modulo_components/modulo_components/lifecycle_component.py
@@ -226,25 +226,40 @@ def on_shutdown(self, previous_state: LifecycleState) -> TransitionCallbackRetur
         TRANSITION_CALLBACK_SUCCESS transitions to 'Finalized'
         TRANSITION_CALLBACK_FAILURE, TRANSITION_CALLBACK_ERROR or any uncaught exceptions to 'ErrorProcessing'
         """
+        def error_processing(self):
+            self.get_logger().error("Entering into the error processing transition state.")
+            return TransitionCallbackReturn.ERROR
+
         self.get_logger().debug(f"on_shutdown called from previous state {previous_state.label}.")
         if not self._has_error:
             if previous_state.state_id == State.PRIMARY_STATE_FINALIZED:
                 return TransitionCallbackReturn.SUCCESS
             if previous_state.state_id == State.PRIMARY_STATE_ACTIVE:
                 if not self.__handle_deactivate():
                     self.get_logger().debug("Shutdown failed during intermediate deactivation!")
+                    return error_processing(self)
+                if not self.__handle_cleanup():
+                    self.get_logger().debug("Shutdown failed during intermediate cleanup!")
+                    return error_processing(self)
+                if not self.__handle_shutdown():
+                    return error_processing(self)
+                self._finalize_interfaces()
+                return TransitionCallbackReturn.SUCCESS
             if previous_state.state_id == State.PRIMARY_STATE_INACTIVE:
                 if not self.__handle_cleanup():
                     self.get_logger().debug("Shutdown failed during intermediate cleanup!")
+                    return error_processing(self)
+                if not self.__handle_shutdown():
+                    return error_processing(self)
+                self._finalize_interfaces()
+                return TransitionCallbackReturn.SUCCESS
             if previous_state.state_id == State.PRIMARY_STATE_UNCONFIGURED:
                 if not self.__handle_shutdown():
-                    self.get_logger().error("Entering into the error processing transition state.")
-                    return TransitionCallbackReturn.ERROR
+                    return error_processing(self)
                 self._finalize_interfaces()
                 return TransitionCallbackReturn.SUCCESS
             self.get_logger().warn(f"Invalid transition 'shutdown' from state {previous_state.label}.")
-        self.get_logger().error("Entering into the error processing transition state.")
-        return TransitionCallbackReturn.ERROR
+        return error_processing(self)
 
     def __handle_shutdown(self) -> bool:
         """

diff --git a/source/modulo_components/src/Component.cpp b/source/modulo_components/src/Component.cpp
@@ -18,6 +18,12 @@ Component::Component(const NodeOptions& node_options, const std::string& fallbac
   this->add_predicate("in_error_state", false);
 }
 
+Component::~Component() {
+  if (this->execute_thread_.joinable()) {
+    this->execute_thread_.join();
+  }
+}
+
 void Component::step() {
   try {
     this->evaluate_periodic_callbacks();