[finger563]

Summary

Fast follow to #851. Deleting a python-owned espp object whose destructor joins a thread ran that join with the GIL held; when the thread was waiting for the GIL to invoke its python callback, neither side could proceed and del task hung forever. The existing call-guard pass cannot cover this (tp_dealloc is not a .def), and at interpreter shutdown the same join happened during finalisation, when a C++ thread can no longer re-acquire the GIL at all.

• espp::py_detail::StopOnExitHolder (lib/python_bindings/stop_on_exit_holder.h) is now the pybind11 holder for Task, Timer, TrajectoryPlanner, ThreadPool, FtpServer and RtspServer (_STOP_ON_EXIT_CLASSES, applied by a new postprocess pass in autogenerate_bindings.py, idempotent, reapplied to the generated file). It owns the object like unique_ptr, deletes it with the GIL released, and registers every live object in a StopRegistry (stop_registry.h); the module's atexit hook stops all of them with the GIL released before finalisation, so the destructors that run later have nothing left to join. UdpSocket keeps the default holder: pybind11 requires a derived class to share its base's holder kind.
• TrajectoryPlanner::start_task() / stop_task() become public. They are the documented way to pause and resume the planner, the atexit hook needs stop_task(), and a python script can now stop a planner explicitly. Both are bound; stop_task carries the GIL-release guard like Task.stop.
• python/blocking_destructors_test.py runs each case in a child interpreter under a 15 s timeout, so a regression is a failed case rather than a hung test: delete a running Task / Timer / TrajectoryPlanner whose python callback blocks, exit with all three alive, and an unhandled exception with a live planner. Added to the cibuildwheel test command. The lib README documents the holder next to the call-guard note.

Evidence

Against the unmodified main build (host lib + python package, macOS, Python 3.15):

FAIL task_del: timed out after 15 s (hang)
FAIL timer_del: timed out after 15 s (hang)
FAIL planner_del: timed out after 15 s (hang)
PASS exit_with_live_objects (0.25 s)
PASS exception_with_live_planner (0.18 s)
2/5 cases passed

With this change:

PASS task_del (0.26 s): task_del ok 29 0.0
PASS timer_del (0.26 s): timer_del ok 42 0.005
PASS planner_del (0.27 s): planner_del ok 42 0.005
PASS exit_with_live_objects (0.26 s): exit_with_live_objects: leaving True
PASS exception_with_live_planner (0.17 s)
5/5 cases passed

python/dispatcher_test.py and python/rtps_bindings_smoke.py still pass against the rebuilt module; components/trajectory_planner/example builds for esp32 (IDF 6.1).

Second finding: a GIL / mutex lock-order inversion in the planner (pre-existing)

The first CI run of the new test hung on Linux cp311 and on macOS x86_64, and locally in about one run in four. A stack sample of a hung process showed the main thread holding the GIL and blocked on the planner's recursive_mutex inside set_target(), while the planner's callback task held that mutex and was blocked in take_gil: it copied the output_callback std::function under the mutex, and copying or destroying a std::function that wraps a Python callable takes the GIL in pybind11. That inversion exists on main and can deadlock any Python user of the planner, independent of the destructor paths.

Fix (third commit): the callback is held in a shared_ptr; the callback task snapshots the pointer under the mutex (no GIL involved) and calls it with the mutex released, and set_config() copies the new configuration before taking the mutex and lets the old one die after releasing it. No behaviour change. The test's callback-count assertions now only require that callbacks ran at all, since an emulated x86_64 CI runner manages only a handful in the sampling window.

Result: 0 hangs in 50 runs (25 on Python 3.15 from the lib build, 25 on a Python 3.12 wheel of this branch), versus 7 hangs in 23 runs before the fix. The esp32 example still builds.

Merge with main + self-review (at d236ba1)

main (with #851 squash-merged) is merged in; the one conflict was the planner header, where this branch moves start_task() / stop_task() into the public section and #851 changed the protected section right below it (new State docs, reverse_velocity_limit_locked()). Resolved by keeping both: the public pair first, then main's protected section.

Review findings acted on (fourth commit):

• stop_all() ran copied stop functions with the GIL released, so a daemon python thread deleting one of those objects at the same moment would have been a use-after-free. Each registry entry now has its own mutex and alive flag; the holder takes it around the delete, after releasing the GIL (so there is no GIL / mutex inversion), and stop_all() takes it around each stop and skips dead entries.
• A stop function that throws is swallowed at exit.
• No other binding translation unit transfers ownership of these classes (no unique_ptr / shared_ptr of them crosses the boundary), so the custom holder cannot clash with a default-holder cast elsewhere.
• Checked and left alone: Timer and Task invoke their callbacks outside their mutexes, so the inversion was specific to the planner's callback task.

Evidence on the merged tree (macOS arm64): blocking_destructors_test.py 0 hangs in 40 runs (20 on a Python 3.15 lib build, 20 on a Python 3.12 wheel); dispatcher_test.py and rtps_bindings_smoke.py pass; the #850 and #852 scenarios from #851 pass through the Python API with a normal interpreter exit and live planners (previously they needed os._exit()); the planner host test passes and the esp32 example builds.

Generator validation

The committed pybind_espp.cpp / __init__.pyi were updated by applying the new postprocess pass to them directly (it is idempotent), not by regenerating. To validate the pass inside the real pipeline, litgen was installed (Python 3.12; its srcml-caller has no wheel for 3.15) and autogenerate_bindings.py run twice on this branch's headers, once with main's script and once with this one. The two fresh litgen outputs differ in exactly the intended places: the six py::class_<espp::X> → StopOnExitHolder rewrites, the #include "stop_on_exit_holder.h", and the call_guard on TrajectoryPlanner.start_task / stop_task; the stubs are identical.

Worth knowing, pre-existing and not touched here: a fresh regeneration no longer matches the committed file (about 500 lines after clang-format) and does not compile as is (UdpSocket::set_option overload cast, ThreadPool Job, an explicit constructor in copy-initialisation), and the QosBand / Dscp enums have been hand-bound since #735 with capitalised value names where litgen emits lowercase ones. So the committed file is the source of truth and regeneration needs a reconciliation pass before it can be used again; that is a separate job.

Test plan

• blocking-destructor test: 3/5 hang on main, 5/5 pass with the fix
• existing python smoke tests pass
• esp32 example builds
• CI wheels (the test now runs in cibuildwheel on every platform)

🤖 Generated with Claude Code

https://claude.ai/code/session_01DFjjnq3XCTRAXENSxsCxJU

[github-actions]

✅Static analysis result - no issues found! ✅