fix(python): no deadlock when a running Task / Timer / TrajectoryPlanner is deleted or left alive at exit

components/trajectory_planner/README.md

@@ -40,6 +40,10 @@ constraints.
   Safe to call `set_target()` and `output()` from within it.
 - **Thread-safe** - `set_target()`, `get_target()`, `output()`, `stop()`, and
   `reset()` are safe to call from any thread concurrently.
+- **Pause / resume** - `stop_task()` stops the planning and callback tasks (the
+  destructor does the same) and `start_task()` starts them again; `is_running()`
+  reports the state. From Python, `stop_task()` is the clean way to shut a
+  planner down before dropping it.
 
 ## Usage
 

components/trajectory_planner/include/trajectory_planner.hpp

@@ -226,22 +226,6 @@ Leave as nullptr to disable. */
    */
   bool is_running() const;
 
-protected:
-  /// The reverse linear velocity limit in force (max_reverse_linear_velocity, or
-  /// max_linear_velocity when it is 0). Call with mutex_ held.
-  float reverse_velocity_limit_locked() const;
-
-  /// Internal motion state tracked between update() calls.
-  struct State {
-    float v = 0.0f;   /**< Current linear velocity (m/s). */
-    float w = 0.0f;   /**< Current angular velocity (rad/s). */
-    float a_v = 0.0f; /**< Linear acceleration (m/s^2) applied on the last update(), in either
-                           profile mode (0 once a stop has completed). */
-    float a_w = 0.0f; /**< Angular acceleration (rad/s^2) applied on the last update(), in either
-                           profile mode (0 once a stop has completed). */
-  };
-  friend struct fmt::formatter<State>;
-
   /**
    * @brief Start the periodic planning and callback timers.
    *
@@ -254,12 +238,31 @@ Leave as nullptr to disable. */
   bool start_task();
 
   /**
-   * @brief Stop the periodic update task.
+   * @brief Stop the periodic planning and callback timers (joins their tasks).
+   *
+   * The planner can be started again with start_task(); the destructor stops
+   * them itself.
    *
    * @return true if the task was stopped, false if it was not running.
    */
   bool stop_task();
 
+protected:
+  /// The reverse linear velocity limit in force (max_reverse_linear_velocity, or
+  /// max_linear_velocity when it is 0). Call with mutex_ held.
+  float reverse_velocity_limit_locked() const;
+
+  /// Internal motion state tracked between update() calls.
+  struct State {
+    float v = 0.0f;   /**< Current linear velocity (m/s). */
+    float w = 0.0f;   /**< Current angular velocity (rad/s). */
+    float a_v = 0.0f; /**< Linear acceleration (m/s^2) applied on the last update(), in either
+                           profile mode (0 once a stop has completed). */
+    float a_w = 0.0f; /**< Angular acceleration (rad/s^2) applied on the last update(), in either
+                           profile mode (0 once a stop has completed). */
+  };
+  friend struct fmt::formatter<State>;
+
   /**
    * @brief Advance the planner by one time step.
    *
@@ -274,7 +277,12 @@ Leave as nullptr to disable. */
   State state_ = {};
   float target_v_ = 0.0f;
   float target_w_ = 0.0f;
-  espp::TrajectoryPlanner::output_callback_t output_callback_ = nullptr;
+  /// The output callback, shared so the callback task can take a snapshot under
+  /// mutex_ without copying the std::function itself. Copying or destroying a
+  /// std::function that wraps a python callable takes the GIL (pybind11), and a
+  /// thread that holds the GIL may be waiting for mutex_ in set_target() /
+  /// output(): no GIL work may ever happen while mutex_ is held.
+  std::shared_ptr<const output_callback_t> output_callback_;
   std::unique_ptr<espp::Timer> timer_ = nullptr;
   std::unique_ptr<espp::Task> callback_task_ =
       nullptr; // woken by CV from update(), falls back to timeout

components/trajectory_planner/src/trajectory_planner.cpp

@@ -80,11 +80,19 @@ bool TrajectoryPlanner::set_config(const Config &config, bool reset_state) {
   }
 
   logger_.info("Updated config: {}", config);
-  std::lock_guard<std::recursive_mutex> lk(mutex_);
-  config_ = config;
-  output_callback_ = config.output_callback;
-  if (reset_state) {
-    reset();
+  // Copy the configuration (and its callback) before taking mutex_, and let the
+  // previous ones die after releasing it: copying or destroying a std::function
+  // that wraps a python callable takes the GIL, and a thread holding the GIL may
+  // be waiting for mutex_ (see output_callback_ in the header).
+  Config new_config = config;
+  auto new_callback = std::make_shared<const output_callback_t>(config.output_callback);
+  {
+    std::lock_guard<std::recursive_mutex> lk(mutex_);
+    std::swap(config_, new_config);
+    output_callback_.swap(new_callback);
+    if (reset_state) {
+      reset();
+    }
   }
   return true;
 }
@@ -244,15 +252,17 @@ bool TrajectoryPlanner::start_task() {
             return true; // stop the task
           callback_wake_flag_ = false;
         }
-        output_callback_t cb;
+        // snapshot the callback by shared_ptr (no std::function copy under mutex_,
+        // see output_callback_) and call it with the mutex released
+        std::shared_ptr<const output_callback_t> cb;
         MotionCommand cmd;
         {
           std::lock_guard<std::recursive_mutex> lk(mutex_);
           cb = output_callback_;
           cmd = MotionCommand{.linear_velocity = state_.v, .angular_velocity = state_.w};
         }
-        if (cb)
-          cb(cmd);
+        if (cb && *cb)
+          (*cb)(cmd);
         return false;
       },
       .task_config = config_.callback_task_config,

lib/README.md

@@ -212,6 +212,17 @@ by hand:
   when called from python, because the blocked-on thread needs the GIL to invoke
   its python callback (see `_GIL_RELEASE_METHODS` in `autogenerate_bindings.py`;
   the hand-written `rtps` bindings apply the same guard).
+- `espp::py_detail::StopOnExitHolder` as the holder of the classes whose
+  **destructor** joins a thread (`Task`, `Timer`, `TrajectoryPlanner`,
+  `ThreadPool`, `FtpServer`, `RtspServer`; `_STOP_ON_EXIT_CLASSES`). A call
+  guard cannot cover `tp_dealloc`, so deleting a running `Task` from python
+  joined its thread with the GIL held and deadlocked the moment that thread
+  needed the GIL for its callback. The holder (`stop_on_exit_holder.h`) deletes
+  with the GIL released, and registers every live object so the module's
+  `atexit` hook (`stop_registry.h`) stops their threads before the interpreter
+  finalises, when a thread could no longer re-acquire the GIL at all.
+  `python/blocking_destructors_test.py` covers delete-while-running and
+  exit-while-running for each class (it is part of the wheel test command).
 
 The litgen dependency is unpinned and recent versions (0.20–0.22) regressed
 nested-scope/template generation, which is why this automation is needed.

lib/autogenerate_bindings.py

@@ -239,6 +239,7 @@ def _fix_rtsp_qualifications(code: str) -> str:
 _GIL_RELEASE_METHODS = {
     "espp::Task": ["start", "stop"],
     "espp::Timer": ["start", "stop", "cancel"],
+    "espp::TrajectoryPlanner": ["start_task", "stop_task"],
     "espp::FtpServer": ["start", "stop"],
     "espp::TcpSocket": ["connect", "transmit", "receive", "accept"],
     "espp::UdpSocket": ["send", "receive", "start_receiving", "stop_receiving"],
@@ -254,6 +255,40 @@ def _fix_rtsp_qualifications(code: str) -> str:
 _GIL_GUARD = "py::call_guard<py::gil_scoped_release>()"
 
 
+# Classes whose destructor joins a thread (a Task's thread, a Timer's, a TrajectoryPlanner's planning /
+# callback tasks, ...). Deleting one from python runs that join with the GIL held, which deadlocks
+# whenever the thread is waiting for the GIL to invoke its python callback, and at interpreter
+# shutdown the thread can no longer get the GIL at all. These classes get the
+# espp::py_detail::StopOnExitHolder holder (lib/python_bindings/stop_on_exit_holder.h): it deletes
+# with the GIL released and registers the live object so the module's atexit hook stops its threads
+# before finalisation. Only classes without a python-visible base can use it (pybind11 requires a
+# derived class to share its base's holder kind); each needs a stop_for_exit() overload in the header.
+_STOP_ON_EXIT_CLASSES = [
+    "Task",
+    "Timer",
+    "TrajectoryPlanner",
+    "ThreadPool",
+    "FtpServer",
+    "RtspServer",
+]
+_STOP_ON_EXIT_INCLUDE = '#include "stop_on_exit_holder.h"'
+
+
+def _use_stop_on_exit_holders(code: str) -> str:
+    for name in _STOP_ON_EXIT_CLASSES:
+        bare = f"py::class_<espp::{name}>"
+        replacement = f"py::class_<espp::{name}, espp::py_detail::StopOnExitHolder<espp::{name}>>"
+        if bare in code:
+            code = code.replace(bare, replacement)
+        elif replacement not in code:
+            print(f"WARNING: stop-on-exit holder for {name} did not match {bare}")
+    if _STOP_ON_EXIT_INCLUDE not in code:
+        anchor = '#include "espp.hpp"\n'
+        assert anchor in code, "generated file does not include espp.hpp"
+        code = code.replace(anchor, anchor + "\n" + _STOP_ON_EXIT_INCLUDE + "\n", 1)
+    return code
+
+
 def _find_call_end(code: str, open_paren: int) -> int:
     """Return the index of the ')' closing the call whose '(' is at open_paren.
 
@@ -365,6 +400,7 @@ def _postprocess_generated(code: str) -> str:
     code = _fix_class_holders(code)
     code = _fix_rtsp_qualifications(code)
     code = _add_gil_release_guards(code)
+    code = _use_stop_on_exit_holders(code)
     return code
 
 

lib/python_bindings/espp/__init__.pyi

@@ -4141,6 +4141,26 @@ class TrajectoryPlanner:
 
         """
         pass
+    def start_task(self) -> bool:
+        """*
+           * @brief Start the periodic planning and callback timers.
+           *
+           * The planning timer calls update() at Config::planning_period.
+           * The callback task fires output_callback whenever update() produces new output.
+           * Has no effect if the timers are already running.
+           *
+           * @return True if both timers were started successfully.
+
+        """
+        pass
+    def stop_task(self) -> bool:
+        """*
+           * @brief Stop the periodic planning and callback timers (joins their tasks).
+           *
+           * @return True if the task was stopped, False if it was not running.
+
+        """
+        pass
 
     def __init__(self) -> None:
         """Auto-generated default constructor"""

lib/python_bindings/module.cpp

@@ -1,5 +1,7 @@
 #include <pybind11/pybind11.h>
 
+#include "stop_registry.h"
+
 #define STRINGIFY(x) #x
 #define MACRO_STRINGIFY(x) STRINGIFY(x)
 
@@ -43,4 +45,10 @@ PYBIND11_MODULE(_espp, m) {
   py_init_odrive_native(m);
   py_init_dispatcher(m);
   py_init_wdi(m);
+
+  // Stop the threads of every live Task / Timer / TrajectoryPlanner / ... before the interpreter
+  // finalises (see stop_registry.h): their destructors run during finalisation, when a thread
+  // waiting for the GIL to run its python callback could never get it, so the join would hang.
+  py::module_::import("atexit").attr("register")(
+      py::cpp_function([]() { espp::py_detail::StopRegistry::instance().stop_all(); }));
 }

lib/python_bindings/pybind_espp.cpp

@@ -13,6 +13,8 @@
 
 #include "espp.hpp"
 
+#include "stop_on_exit_holder.h"
+
 namespace py = pybind11;
 
 // !!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!  AUTOGENERATED CODE !!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!
@@ -346,8 +348,8 @@ void py_init_module_espp(py::module &m) {
 
   ////////////////////    <generated_from:ftp_server.hpp>    ////////////////////
   auto pyClassFtpServer =
-      py::class_<espp::FtpServer>(m, "FtpServer", py::dynamic_attr(),
-                                  "/ \\brief A class that implements a FTP server.")
+      py::class_<espp::FtpServer, espp::py_detail::StopOnExitHolder<espp::FtpServer>>(
+          m, "FtpServer", py::dynamic_attr(), "/ \\brief A class that implements a FTP server.")
           .def(py::init<std::string_view, uint16_t, const std::filesystem::path &>(),
                py::arg("ip_address"), py::arg("port"), py::arg("root"),
                "/ \\brief A class that implements a FTP server.\n/ \note The IP Address is not "
@@ -2147,7 +2149,7 @@ void py_init_module_espp(py::module &m) {
   ////////////////////    </generated_from:udp_socket.hpp>    ////////////////////
 
   ////////////////////    <generated_from:task.hpp>    ////////////////////
-  auto pyClassTask = py::class_<espp::Task>(
+  auto pyClassTask = py::class_<espp::Task, espp::py_detail::StopOnExitHolder<espp::Task>>(
       m, "Task", py::dynamic_attr(),
       "*\n * @brief Task provides an abstraction over std::thread which optionally\n * includes "
       "memory / priority configuration on ESP systems. It allows users to\n * easily stop the "
@@ -2289,7 +2291,7 @@ void py_init_module_espp(py::module &m) {
   ////////////////////    </generated_from:task.hpp>    ////////////////////
 
   ////////////////////    <generated_from:timer.hpp>    ////////////////////
-  auto pyClassTimer = py::class_<espp::Timer>(
+  auto pyClassTimer = py::class_<espp::Timer, espp::py_detail::StopOnExitHolder<espp::Timer>>(
       m, "Timer", py::dynamic_attr(),
       "/ @brief A timer that can be used to schedule tasks to run at a later time.\n/ @details A "
       "timer can be used to schedule a task to run at a later time.\n/          The timer will run "
@@ -2457,7 +2459,8 @@ void py_init_module_espp(py::module &m) {
   ////////////////////    </generated_from:timer.hpp>    ////////////////////
 
   ////////////////////    <generated_from:trajectory_planner.hpp>    ////////////////////
-  auto pyClassTrajectoryPlanner = py::class_<espp::TrajectoryPlanner>(
+  auto pyClassTrajectoryPlanner = py::class_<
+      espp::TrajectoryPlanner, espp::py_detail::StopOnExitHolder<espp::TrajectoryPlanner>>(
       m, "TrajectoryPlanner", py::dynamic_attr(),
       "*\n *  @brief Converts normalized joystick velocity commands into smooth,\n *         "
       "dynamically feasible chassis motion commands (v, w).\n *\n *  The planner is drive-system "
@@ -2684,11 +2687,25 @@ void py_init_module_espp(py::module &m) {
            "* or before re-initialising with a new configuration.\n")
       .def("is_running", &espp::TrajectoryPlanner::is_running,
            "*\n   * @brief Check whether the periodic update task is currently running.\n   * "
-           "@return True if the task is running.\n");
+           "@return True if the task is running.\n")
+      .def("start_task", &espp::TrajectoryPlanner::start_task,
+           "*\n   * @brief Start the periodic planning and callback timers.\n   *\n   * The "
+           "planning "
+           "timer calls update() at Config::planning_period.\n   * The callback task fires "
+           "output_callback whenever update() produces new output.\n   * Has no effect if the "
+           "timers "
+           "are already running.\n   *\n   * @return True if both timers were started "
+           "successfully.\n",
+           py::call_guard<py::gil_scoped_release>())
+      .def("stop_task", &espp::TrajectoryPlanner::stop_task,
+           "*\n   * @brief Stop the periodic planning and callback timers (joins their tasks).\n   "
+           "*\n   * @return True if the task was stopped, False if it was not running.\n",
+           py::call_guard<py::gil_scoped_release>());
   ////////////////////    </generated_from:trajectory_planner.hpp>    ////////////////////
 
   ////////////////////    <generated_from:thread_pool.hpp>    ////////////////////
-  auto pyClassThreadPool = py::class_<espp::ThreadPool>(
+  auto pyClassThreadPool = py::class_<espp::ThreadPool,
+                                      espp::py_detail::StopOnExitHolder<espp::ThreadPool>>(
       m, "ThreadPool", py::dynamic_attr(),
       "*\n * @brief A thread pool that dispatches submitted jobs to a fixed set of worker "
       "threads.\n *\n * Workers are implemented as espp::Task instances. Jobs are queued and\n * "
@@ -3846,7 +3863,8 @@ void py_init_module_espp(py::module &m) {
   ////////////////////    </generated_from:rtsp_client.hpp>    ////////////////////
 
   ////////////////////    <generated_from:rtsp_server.hpp>    ////////////////////
-  auto pyClassRtspServer = py::class_<espp::RtspServer>(
+  auto pyClassRtspServer = py::class_<espp::RtspServer,
+                                      espp::py_detail::StopOnExitHolder<espp::RtspServer>>(
       m, "RtspServer", py::dynamic_attr(),
       "/ Class for streaming MJPEG data from a camera using RTSP + RTP\n/ Starts a TCP socket to "
       "listen for RTSP connections, and then spawns off a\n/ new RTSP session for each "