From 1ef1837da79cf23125b623ce9f10cccdd83dd221 Mon Sep 17 00:00:00 2001 From: mich-pest Date: Tue, 8 Sep 2026 12:16:11 +0200 Subject: [PATCH 01/11] layer: protected app data access --- .../include/dls2/application/layer.hpp | 6 +++++ .../include/dls2/application/layer.tpp | 26 +++++++++++++++---- 2 files changed, 27 insertions(+), 5 deletions(-) diff --git a/modules/application/include/dls2/application/layer.hpp b/modules/application/include/dls2/application/layer.hpp index d7e1f6d3..7d1f5494 100644 --- a/modules/application/include/dls2/application/layer.hpp +++ b/modules/application/include/dls2/application/layer.hpp @@ -57,6 +57,12 @@ namespace dls template void checkAppData(const Map& app_data); + template + void checkAppData(const Map& app_data, std::mutex& mutex); + + template + void checkAppDataImpl(const Map& app_data, std::mutex* mutex); + // BEGIN critical section mutable std::mutex components_mutex; std::map components; diff --git a/modules/application/include/dls2/application/layer.tpp b/modules/application/include/dls2/application/layer.tpp index 666caf0b..c57d3846 100644 --- a/modules/application/include/dls2/application/layer.tpp +++ b/modules/application/include/dls2/application/layer.tpp @@ -2,17 +2,33 @@ #define LAYER_TPP_H7JRIVPM #include "dls2/application/layer.hpp" +#include namespace dls { template void Layer::checkAppData(const Map& app_data) +{ + checkAppDataImpl(app_data, nullptr); +} + +template +void Layer::checkAppData(const Map& app_data, std::mutex& mutex) +{ + checkAppDataImpl(app_data, &mutex); +} + +template +void Layer::checkAppDataImpl(const Map& app_data, std::mutex* mutex) { using Ptr = typename Map::mapped_type; using Data = typename Ptr::element_type; static_assert(std::is_base_of_v, "must store AppData-derived"); + std::vector stopped; + std::unique_lock lock; + if (mutex) lock = std::unique_lock(*mutex); for(const auto& [key, data] : app_data) { if(!data || !data->proc) @@ -24,14 +40,14 @@ void Layer::checkAppData(const Map& app_data) { if (this->safety_layer_config_->enable_process_died) { - this->robust_event_notifier.notify( - EventID::PROCESS_DIED, - EventSeverity::ERROR, - this->getID() + ": " + key + " is not running" - ); + stopped.push_back(key); } } } + if (lock.owns_lock()) lock.unlock(); + for (const auto& key : stopped) + this->robust_event_notifier.notify(EventID::PROCESS_DIED, EventSeverity::ERROR, + this->getID() + ": " + key + " is not running"); } } // end namespace dls From dde639b7aa119a4a36a3b9b51bb5983bb03db63e Mon Sep 17 00:00:00 2001 From: mich-pest Date: Tue, 8 Sep 2026 13:34:02 +0200 Subject: [PATCH 02/11] control_layer: controllers and motion_generators vector protected access --- .../dls2/core_framework/control_layer.hpp | 5 + .../core_framework/src/control_layer.cpp.in | 201 ++++++++++-------- 2 files changed, 119 insertions(+), 87 deletions(-) diff --git a/modules/core_framework/include/dls2/core_framework/control_layer.hpp b/modules/core_framework/include/dls2/core_framework/control_layer.hpp index 22ed9bd0..08b2480f 100644 --- a/modules/core_framework/include/dls2/core_framework/control_layer.hpp +++ b/modules/core_framework/include/dls2/core_framework/control_layer.hpp @@ -103,6 +103,11 @@ class ControlLayer : public Layer std::mutex motion_mutex; // END critical section + // Serialize operations without holding map locks across DDS or shutdown waits. + std::mutex controllers_operations_mutex; + std::mutex motion_operations_mutex; + std::atomic_bool closing_{false}; + std::shared_ptr ddsSignalLink; /// Default controller spline-in diff --git a/modules/core_framework/src/control_layer.cpp.in b/modules/core_framework/src/control_layer.cpp.in index 2effc374..cfffb56b 100644 --- a/modules/core_framework/src/control_layer.cpp.in +++ b/modules/core_framework/src/control_layer.cpp.in @@ -90,7 +90,9 @@ ControlLayer::ControlLayer(std::string ID, std::string robot_name) "Unload a motion generator", std::function([&](std::string s)->bool { - if(this->unloadMotionGenerator(s) && this->motion_generators.size() == 0) + const bool unloaded = this->unloadMotionGenerator(s); + std::lock_guard lock(motion_mutex); + if(unloaded && this->motion_generators.empty()) return true; else // either the unload was not successful or there are still motion generators running return false; @@ -129,7 +131,9 @@ ControlLayer::ControlLayer(std::string ID, std::string robot_name) "Unload a controller", std::function([&](std::string s)->bool { - if(this->unloadController(s) && this->controllers.size() == 0) + const bool unloaded = this->unloadController(s); + std::lock_guard lock(controllers_mutex); + if(unloaded && this->controllers.empty()) return true; else // either the unload was not successful or there are still controllers running return false; @@ -144,7 +148,9 @@ ControlLayer::ControlLayer(std::string ID, std::string robot_name) "Unload an external controller", std::function([&](std::string s)->bool { - if(this->unloadExternalController(s) && this->controllers.size() == 0) + const bool unloaded = this->unloadExternalController(s); + std::lock_guard lock(controllers_mutex); + if(unloaded && this->controllers.empty()) return true; else // either the unload was not successful or there are still controllers running return false; @@ -166,24 +172,22 @@ ControlLayer::ControlLayer(std::string ID, std::string robot_name) if(s == "cont") { std::stringstream ss; - // std::lock_guard lock(this->controllers_mutex); - ss << "\n"; - for(auto &elem : this->controllers) { - ss << elem.first << "\n"; + std::lock_guard lock(controllers_mutex); + ss << "\n"; + for (const auto& [name, data] : controllers) ss << name << "\n"; } - this->app_logger.info(ss.str()); + app_logger.info(ss.str()); } else if(s == "motion") { - std::stringstream ss; - // std::lock_guard lock(this->motion_mutex); - ss << "\n"; - for(auto &elem : this->motion_generators) + std::stringstream ss; { - ss << elem.first << "\n"; + std::lock_guard lock(motion_mutex); + ss << "\n"; + for (const auto& [name, data] : motion_generators) ss << name << "\n"; } - this->app_logger.info(ss.str()); + app_logger.info(ss.str()); } else { @@ -200,10 +204,6 @@ ControlLayer::ControlLayer(std::string ID, std::string robot_name) ControlLayer::~ControlLayer() { - for(auto pair : this->controllers){ - pair.second = nullptr; - } - void *res = nullptr; // do not need to save the thread exit status pthread_join(controlSignalGatherThread, &res); } @@ -242,6 +242,7 @@ void *ControlLayer::controlSignalGather(void *data) ControlLayer *layer = (ControlLayer*) data; boost::circular_buffer delta(100); + std::vector> controllers_snapshot; while(!layer->sm.isRaised(layer->sm.deactivation_request) && !layer->sm.isRaised(layer->sm.quit_request)) { auto now = std::chrono::high_resolution_clock::now(); @@ -255,7 +256,15 @@ void *ControlLayer::controlSignalGather(void *data) bool active_impedence_controller = false; bool control_mode_set_once = false; - for(auto [_, controller_data] : layer->controllers) + // Snapshot lifetime is independent of concurrent extraction/unload. + controllers_snapshot.clear(); + { + std::lock_guard lock(layer->controllers_mutex); + controllers_snapshot.reserve(layer->controllers.size()); + for (const auto& [name, controller] : layer->controllers) + controllers_snapshot.push_back(controller); + } + for(const auto& controller_data : controllers_snapshot) { controller_data->reader_control_signal.read(); const auto& controller_msg = controller_data->reader_control_signal.msg; @@ -343,17 +352,23 @@ void *ControlLayer::controlSignalGather(void *data) void ControlLayer::monitor() { - this->checkAppData(this->controllers); - this->checkAppData(this->motion_generators); + this->checkAppData(this->controllers, controllers_mutex); + this->checkAppData(this->motion_generators, motion_mutex); } void ControlLayer::close() -{ - for(auto pair : this->motion_generators) - this->unloadMotionGenerator(pair.first); - - for(auto pair : this->controllers) - this->unloadController(pair.first); +{ + closing_.store(true); + decltype(motion_generators) generators_to_stop; + decltype(controllers) controllers_to_stop; + { + std::scoped_lock operations(controllers_operations_mutex, motion_operations_mutex); + std::scoped_lock maps(controllers_mutex, motion_mutex); + generators_to_stop.swap(motion_generators); + controllers_to_stop.swap(controllers); + } + for (auto& [name, data] : generators_to_stop) unloadMotionGenerator(data); + for (auto& [name, data] : controllers_to_stop) unloadController(data); } std::string ControlLayer::where() @@ -362,7 +377,7 @@ std::string ControlLayer::where() // Where Controllers { - // std::lock_guard lock(this->controllers_mutex); + std::lock_guard lock(this->controllers_mutex); if(controllers.size() > 0) { ss << "Active controllers:\n"; @@ -378,7 +393,7 @@ std::string ControlLayer::where() } // Where Motion Generators { - // std::lock_guard lock(this->motion_mutex); + std::lock_guard lock(this->motion_mutex); if(motion_generators.size() > 0) { ss << "Active Motion Generator:\n"; @@ -409,12 +424,16 @@ std::string ControlLayer::where() // ============================================================================= bool ControlLayer::loadController(const std::string& lib_name) { - // std::lock_guard lock(this->controllers_mutex); - - if(this->controllers.find(lib_name) != this->controllers.end()) + std::lock_guard operation(controllers_operations_mutex); + if (closing_.load()) return false; + bool already_loaded; { - this->app_logger.error("Controller " + lib_name + " already loaded"); - return false; + std::lock_guard lock(controllers_mutex); + already_loaded = controllers.find(lib_name) != controllers.end(); + } + if (already_loaded) { + app_logger.error("Controller " + lib_name + " already loaded"); + return false; } std::shared_ptr pData = std::make_shared( @@ -467,16 +486,25 @@ bool ControlLayer::loadController(const std::string& lib_name) // } // } pData->proc->detach(); - this->controllers.emplace(pData->getID(), pData); + { + std::lock_guard lock(controllers_mutex); + this->controllers.emplace(pData->getID(), pData); + } return true; } bool ControlLayer::loadExternalController(const std::string& lib_name) { - if(this->controllers.find(lib_name) != this->controllers.end()) + std::lock_guard operation(controllers_operations_mutex); + if (closing_.load()) return false; + bool already_loaded; { - this->app_logger.error("Controller " + lib_name + " already loaded"); - return false; + std::lock_guard lock(controllers_mutex); + already_loaded = controllers.find(lib_name) != controllers.end(); + } + if (already_loaded) { + app_logger.error("Controller " + lib_name + " already loaded"); + return false; } std::shared_ptr pData = std::make_shared( @@ -491,13 +519,18 @@ bool ControlLayer::loadExternalController(const std::string& lib_name) this->app_logger.info("CONTROL LAYER IS LISTENING CONTROLLER " + pData->getID() + " ON TOPIC " + pData->getID()); - this->controllers.emplace(pData->getID(), pData); + { + std::lock_guard lock(controllers_mutex); + this->controllers.emplace(pData->getID(), pData); + } return true; } bool ControlLayer::unloadController(std::shared_ptr pData) { + // Entry was extracted under the map lock; external controllers have no process. + if (!pData->proc) return true; // // spline down the controller // if(pData->pSpline_out) // { @@ -515,7 +548,6 @@ bool ControlLayer::unloadController(std::shared_ptr pData) // } // } - // std::lock_guard lock(this->controllers_mutex); command_manager.callCommand("shutdown", {}, pData->getID()); @@ -531,48 +563,38 @@ bool ControlLayer::unloadController(std::shared_ptr pData) } pData->proc = nullptr; - this->controllers.erase(pData->getID()); std::cout << "unloaded controller " << pData->getID() << std::endl; return true; } -bool ControlLayer::unloadController(const std::string &ID) +bool ControlLayer::unloadController(const std::string& ID) { - decltype(this->controllers.find(ID)) pair_it; + std::lock_guard operation(controllers_operations_mutex); + std::shared_ptr data; { - // std::lock_guard lock(this->controllers_mutex); - pair_it = this->controllers.find(ID); - - if(pair_it == this->controllers.end()) - { - this->app_logger.error("controller " + ID + " not loaded"); - return false; - } + std::lock_guard lock(controllers_mutex); + auto it = controllers.find(ID); + if (it == controllers.end()) return false; + data = it->second; + controllers.erase(it); } - - return this->unloadController(pair_it->second); + return unloadController(data); } -bool ControlLayer::unloadExternalController(const std::string &ID) +bool ControlLayer::unloadExternalController(const std::string& ID) { - decltype(this->controllers.find(ID)) pair_it; + std::lock_guard operation(controllers_operations_mutex); + std::shared_ptr data; { - // std::lock_guard lock(this->controllers_mutex); - pair_it = this->controllers.find(ID); - - if(pair_it == this->controllers.end()) - { - this->app_logger.error("controller " + ID + " not loaded"); - return false; - } + std::lock_guard lock(controllers_mutex); + auto it = controllers.find(ID); + if (it == controllers.end() || it->second->proc) return false; + data = it->second; + controllers.erase(it); } - - this->controllers.erase(ID); - - std::cout << "unloaded controller " << ID << std::endl; - return true; + return true; } // ----------------------------------------------------------------------------- @@ -580,15 +602,19 @@ bool ControlLayer::unloadExternalController(const std::string &ID) // ----------------------------------------------------------------------------- bool ControlLayer::loadMotionGenerator(const std::string& lib_name) { - // std::lock_guard lock(this->motion_mutex); - - if(this->motion_generators.find(lib_name) != this->motion_generators.end()) + std::lock_guard operation(motion_operations_mutex); + if (closing_.load()) return false; + bool already_loaded; { - this->app_logger.error("motion generation " + lib_name + " already loaded" ); - return false; + std::lock_guard lock(motion_mutex); + already_loaded = motion_generators.find(lib_name) != motion_generators.end(); + } + if (already_loaded) { + app_logger.error("Motion generator " + lib_name + " already loaded"); + return false; } - std::shared_ptr pData = std::make_shared(lib_name); + std::shared_ptr pData = std::make_shared(lib_name); pData->proc = std::make_shared(std::vector({ "${DLS_INSTALL_RUNTIME_DIR}/child_process_launcher", @@ -604,13 +630,17 @@ bool ControlLayer::loadMotionGenerator(const std::string& lib_name) } pData->proc->detach(); - this->motion_generators.emplace(pData->getID(), pData); + { + std::lock_guard lock(motion_mutex); + this->motion_generators.emplace(pData->getID(), pData); + } return true; } bool ControlLayer::unloadMotionGenerator(std::shared_ptr pData) { - // std::lock_guard lock(this->motion_mutex); + // Entry was extracted under the map lock and is no longer monitored. + if (!pData->proc) return true; //shutdown motion generation over the dds comunication layer command_manager.callCommand("shutdown", {}, pData->getID()); @@ -632,24 +662,21 @@ bool ControlLayer::unloadMotionGenerator(std::shared_ptr pData) std::cout<< "Motion generation " << pData->getID() << " is unloaded" << std::endl; pData->proc = nullptr; - this->motion_generators.erase(pData->getID()); return true; } bool ControlLayer::unloadMotionGenerator(const std::string& ID) { - // std::lock_guard lock(this->motion_mutex); - - auto pair_it = this->motion_generators.find(ID); - - if(pair_it == this->motion_generators.end()) + std::lock_guard operation(motion_operations_mutex); + std::shared_ptr data; { - this->app_logger.error("motion generation " + ID + " is not loaded"); - return false; + std::lock_guard lock(motion_mutex); + auto it = motion_generators.find(ID); + if (it == motion_generators.end()) return false; + data = it->second; + motion_generators.erase(it); } - - - return this->unloadMotionGenerator(pair_it->second); + return unloadMotionGenerator(data); } std::vector ControlLayer::saturateTorques(const std::vector& req) const From aee10a743f6925adb86c648cd05fe67eff1c5a54 Mon Sep 17 00:00:00 2001 From: mich-pest Date: Tue, 8 Sep 2026 13:37:09 +0200 Subject: [PATCH 03/11] utils: shutdown signal utils added --- .../include/dls2/util/shutdown_signal.hpp | 113 ++++++++++++++++++ 1 file changed, 113 insertions(+) create mode 100644 modules/utils/include/dls2/util/shutdown_signal.hpp diff --git a/modules/utils/include/dls2/util/shutdown_signal.hpp b/modules/utils/include/dls2/util/shutdown_signal.hpp new file mode 100644 index 00000000..eb3cae13 --- /dev/null +++ b/modules/utils/include/dls2/util/shutdown_signal.hpp @@ -0,0 +1,113 @@ +#pragma once + +#include +#include +#include +#include +#include +#include +#include +#include +#include +#include + +namespace dls::utils +{ +// One instance per executable. Notifications received before start() remain queued. +// The handler only writes to a nonblocking pipe; callbacks run on a normal thread. +class ShutdownSignal +{ +public: + ShutdownSignal() + { + if (::pipe2(pipe_, O_NONBLOCK | O_CLOEXEC) != 0) + throw std::system_error(errno, std::generic_category(), "shutdown pipe"); + int expected = -1; + if (!write_fd_.compare_exchange_strong(expected, pipe_[1])) { + ::close(pipe_[0]); + ::close(pipe_[1]); + throw std::logic_error("ShutdownSignal already installed"); + } + struct sigaction action{}; + action.sa_handler = &handle; + ::sigemptyset(&action.sa_mask); + action.sa_flags = SA_RESTART; + if (::sigaction(SIGINT, &action, &old_int_) != 0) { + const int error = errno; + release(); + throw std::system_error(error, std::generic_category(), "SIGINT handler"); + } + if (::sigaction(SIGTERM, &action, &old_term_) != 0) { + const int error = errno; + ::sigaction(SIGINT, &old_int_, nullptr); + release(); + throw std::system_error(error, std::generic_category(), "SIGTERM handler"); + } + } + + ShutdownSignal(const ShutdownSignal&) = delete; + ShutdownSignal& operator=(const ShutdownSignal&) = delete; + + ~ShutdownSignal() + { + stopping_.store(true); + if (worker_.joinable()) worker_.join(); + write_fd_.store(-1); + ::sigaction(SIGINT, &old_int_, nullptr); + ::sigaction(SIGTERM, &old_term_, nullptr); + release(); + } + + // Return false while initialization cannot yet accept the request. + void start(std::function request_shutdown) + { + if (worker_.joinable()) throw std::logic_error("ShutdownSignal already started"); + worker_ = std::thread([this, callback = std::move(request_shutdown)] { + pollfd input{pipe_[0], POLLIN, 0}; + bool pending = false; + while (!stopping_.load()) { + if (::poll(&input, 1, 50) > 0) { + char signals[64]; + if (::read(pipe_[0], signals, sizeof(signals)) > 0) pending = true; + } + if (pending && !stopping_.load() && callback()) return; + } + }); + } + +private: + static_assert(std::atomic::is_always_lock_free); + static_assert(std::atomic::is_always_lock_free); + inline static std::atomic write_fd_{-1}; + inline static std::atomic handlers_{0}; + + static void handle(int) noexcept + { + const int saved_errno = errno; + handlers_.fetch_add(1); + const int fd = write_fd_.load(); + if (fd >= 0) { + const char byte = 1; + ssize_t result; + do { result = ::write(fd, &byte, 1); } while (result < 0 && errno == EINTR); + // EAGAIN means an earlier notification is already pending. + } + handlers_.fetch_sub(1); + errno = saved_errno; + } + + void release() noexcept + { + write_fd_.store(-1); + while (handlers_.load() != 0) std::this_thread::yield(); + ::close(pipe_[0]); + ::close(pipe_[1]); + } + + int pipe_[2]; + struct sigaction old_int_{}; + struct sigaction old_term_{}; + std::atomic_bool stopping_{false}; + std::thread worker_; +}; +} From fb08d67332ccd9b2fa4cbc00750524ec01481f60 Mon Sep 17 00:00:00 2001 From: mich-pest Date: Tue, 8 Sep 2026 13:38:56 +0200 Subject: [PATCH 04/11] utils: owned process header added --- .../utils/include/dls2/util/owned_process.hpp | 104 ++++++++++++++++++ 1 file changed, 104 insertions(+) create mode 100644 modules/utils/include/dls2/util/owned_process.hpp diff --git a/modules/utils/include/dls2/util/owned_process.hpp b/modules/utils/include/dls2/util/owned_process.hpp new file mode 100644 index 00000000..d0bb3d58 --- /dev/null +++ b/modules/utils/include/dls2/util/owned_process.hpp @@ -0,0 +1,104 @@ +#pragma once + +#include +#include +#include +#include +#include +#include +#include +#include +#include +#include +#include + +namespace dls::utils +{ +// Descendants inherit this dedicated group, including exec'd Python plugins. +// The owning executable must enable PR_SET_CHILD_SUBREAPER to reap orphaned +// descendants after a layer exits before its plugins. +class OwnedProcess +{ +public: + // The interactive console must retain the terminal's foreground group. + explicit OwnedProcess(const std::vector& args, bool own_group = true) + : proc(own_group ? boost::process::child(args, group_) : boost::process::child(args)), + own_group_(own_group) {} + + bool running() + { + if (finished_) return false; + if (!leader_exited_) { + std::error_code error; + leader_exited_ = !proc.running(error); + if (error) throw std::system_error(error, "checking owned child"); + } + if (!own_group_) return !leader_exited_; + if (leader_exited_) { + int status; + while (::waitpid(-group_.native_handle(), &status, WNOHANG) > 0) {} + } + if (::kill(-group_.native_handle(), 0) == 0 || errno != ESRCH) return true; + finished_ = true; + group_.detach(); + return false; + } + + void signal(int value) + { + if (!finished_ && ::kill(own_group_ ? -group_.native_handle() : proc.id(), value) != 0 && errno != ESRCH) + throw std::system_error(errno, std::generic_category(), "signalling owned process group"); + } + + int id() const { return proc.id(); } + void detach() { proc.detach(); group_.detach(); } + +private: + boost::process::group group_; +public: + boost::process::child proc; +private: + bool leader_exited_{false}; + bool finished_{false}; + bool own_group_; +}; + +using OwnedProcesses = std::map>; + +inline bool shutdownProcesses(OwnedProcesses& processes, std::chrono::milliseconds grace) +{ + using namespace std::chrono_literals; + const auto wait = [&](std::chrono::milliseconds duration) { + const auto deadline = std::chrono::steady_clock::now() + duration; + for (;;) { + bool any_running = false; + for (auto& [name, process] : processes) any_running |= process->running(); + if (!any_running) return true; + if (std::chrono::steady_clock::now() >= deadline) return false; + std::this_thread::sleep_for(20ms); + } + }; + const auto signal = [&](int value, bool report) { + for (auto& [name, process] : processes) { + if (!process->running()) continue; + if (report) std::cerr << "Shutdown: " << name << " (PID/PGID " << process->id() + << ") still running; sending signal " << value << std::endl; + process->signal(value); + } + }; + signal(SIGINT, false); + if (wait(grace)) return true; + signal(SIGTERM, true); + if (wait(2s)) return true; + signal(SIGKILL, true); + if (wait(2s)) return true; + for (auto& [name, process] : processes) { + if (process->running()) { + std::cerr << "Shutdown: " << name << " (PID/PGID " << process->id() + << ") did not exit after SIGKILL" << std::endl; + process->detach(); // Do not turn a bounded shutdown into a destructor wait. + } + } + return false; +} +} From 1d8f2bf37ba230a55959ded9cbfe596679dc9764 Mon Sep 17 00:00:00 2001 From: mich-pest Date: Tue, 8 Sep 2026 13:43:01 +0200 Subject: [PATCH 05/11] child_process_launcher: using shutdown signal class --- .../src/child_process_launcher.cpp.in | 30 +++++++------------ 1 file changed, 10 insertions(+), 20 deletions(-) diff --git a/modules/child_process/src/child_process_launcher.cpp.in b/modules/child_process/src/child_process_launcher.cpp.in index a121e01c..a3e27ad6 100644 --- a/modules/child_process/src/child_process_launcher.cpp.in +++ b/modules/child_process/src/child_process_launcher.cpp.in @@ -23,6 +23,7 @@ // #include "dls2/service/service_base.hpp" #include "dls2/log/event_logger.hpp" #include "dls2/application/layer.hpp" +#include "dls2/util/shutdown_signal.hpp" #include "robotlib/robot_factory.hpp" @@ -30,20 +31,6 @@ // Using Declarations // ============================================================================= using namespace dls; -std::string process_name(""); -CommandManager stopper("process_launcher"); -void shutdown(int) -{ - bool stop_wait = false; - if (process_name.compare("")==0) - exit(EXIT_FAILURE); - else if(stopper.waitCommand(process_name, "shutdown", stop_wait)) - stopper.callCommand("shutdown", {}, process_name); - else{ - std::cerr << process_name <<"::shutdown not found. Process is brutally killed."; - exit(EXIT_FAILURE); - } -} // ============================================================================= // Globals @@ -94,14 +81,11 @@ void change_process_name(char **argv, const std::string &name) // ============================================================================= int main(int argc, char **argv) { - // When the user presses CTRL+C, make sure that all layers are shutdown - signal( - SIGINT, - shutdown); + std::shared_ptr pComponent; + utils::ShutdownSignal shutdown_signal; // give time to the logstrem connect to the logger layer Args args = parse_args(argc, argv); - std::shared_ptr pComponent; // Load the component try @@ -206,7 +190,13 @@ int main(int argc, char **argv) exit(EXIT_FAILURE); } - process_name = pComponent->getID(); + shutdown_signal.start([pComponent] { + // Initialization has no quit transition. Retain an early signal until idle. + const auto state = pComponent->sm.getStateName(); + if (state == "initialization") return false; + if (state != "quit") pComponent->stop(); + return true; + }); change_process_name(argv, args.component_name.c_str()); From d8c72619d8149b7efa3a5f47d617e8ff8f9221ec Mon Sep 17 00:00:00 2001 From: mich-pest Date: Tue, 8 Sep 2026 13:55:50 +0200 Subject: [PATCH 06/11] run_dls2: shutdown refactor --- modules/main/include/dls2/main/run_dls2.hpp | 11 ++- modules/main/src/run_dls2.cpp.in | 99 +++++++++------------ 2 files changed, 50 insertions(+), 60 deletions(-) diff --git a/modules/main/include/dls2/main/run_dls2.hpp b/modules/main/include/dls2/main/run_dls2.hpp index 57f254fe..d1e088be 100644 --- a/modules/main/include/dls2/main/run_dls2.hpp +++ b/modules/main/include/dls2/main/run_dls2.hpp @@ -5,6 +5,8 @@ #include "dls2/state_machine/state_machine_watcher.hpp" #include "dls2/command/command_manager.hpp" #include "dls2/application/app_data.hpp" +#include "dls2/util/owned_process.hpp" +#include "dls2/util/shutdown_signal.hpp" namespace dls { @@ -25,16 +27,17 @@ namespace dls void launchServers(); void launchSingleServer(const std::string& ip, int port); - static void shutdown_all(int); - static CommandManager command_manager; - static bool should_quit; - std::map> layers; + static std::atomic_bool should_quit; + utils::OwnedProcesses layers; + utils::OwnedProcesses discovery_servers; //! List of discovery servers std::vector> servers; // add state machine watcher state_machine::StateMachineWatcher sm_watcher; DDSParticipant ddspart; + // Destroy/join the callback before the other instance members. + utils::ShutdownSignal shutdown_signal; }; } #endif diff --git a/modules/main/src/run_dls2.cpp.in b/modules/main/src/run_dls2.cpp.in index bc61fdc2..c0fd4c38 100644 --- a/modules/main/src/run_dls2.cpp.in +++ b/modules/main/src/run_dls2.cpp.in @@ -9,21 +9,15 @@ namespace dls { CommandManager RunDLS2::command_manager("dls_framework"); - bool RunDLS2::should_quit(false); - - void RunDLS2::shutdown_all(int) - { - should_quit = true; - } + std::atomic_bool RunDLS2::should_quit(false); RunDLS2::RunDLS2(int argc, char **argv) : sm_watcher("sm_watcher_runDLS2"), ddspart("ddspart_runDLS2", dls::domains::layers, eprosima::fastdds::rtps::DiscoveryProtocol::SUPER_CLIENT) { - // When the user presses CTRL+C, make sure that all layers are shutdown - signal( - SIGINT, - shutdown_all); + shutdown_signal.start([] { should_quit.store(true); return true; }); + if (prctl(PR_SET_CHILD_SUBREAPER, 1) != 0) + throw std::system_error(errno, std::generic_category(), "enabling child subreaper"); change_process_name(argv, "dls_framework"); @@ -103,32 +97,31 @@ namespace dls launchLayers(); } - // Hanging on this executable to intercept CTRL+C. If no layers are running, the executable will exit - int num_layers = 1; - while (!should_quit || (num_layers > 0)) - { - std::this_thread::sleep_for(std::chrono::milliseconds(500)); - if(should_quit) - command_manager.callCommand("shutdown", {"all"}); - - // Check if at least one layer is running - auto layers = ddspart.getParticipants(); - num_layers = std::count_if(layers.begin(), layers.end(), [](std::string s) - { return s.find("Layer") != std::string::npos; }); - } - - // Before exiting, wait the closing of layers. It is not necessary, but it is used to print the information at the end only when all the layers are closed - std::cout << "\n"<< std::endl; - for (auto layer : layers) - { - std::cout << "Waiting for " << layer.first << " to exit..." << std::endl; - layer.second->proc->wait(); // wait for the process to exit + while (!should_quit.load()) { + bool any_running = false; + for (auto& [name, process] : layers) any_running |= process->running(); + if (!layers.empty() && !any_running) break; + std::this_thread::sleep_for(std::chrono::milliseconds(50)); } - // close servers - if(system("pkill -SIGINT fast-disc")!=0){ - throw std::runtime_error("CANNOT SHUTDOWN FASTDDS SERVERS"); + std::chrono::milliseconds grace(10000); + if (const char* value = std::getenv("DLS_SHUTDOWN_TIMEOUT_MS")) { + try { + std::size_t used = 0; + const int timeout = std::stoi(value, &used); + if (value[used] != '\0' || timeout <= 0 || timeout > 600000) + throw std::invalid_argument("out of range"); + grace = std::chrono::milliseconds(timeout); + } catch (const std::exception&) { + std::cerr << "Invalid DLS_SHUTDOWN_TIMEOUT_MS; using 10000 ms" << std::endl; + } } + // OS signals reach owned descendants even when DDS discovery is unavailable. + // Discovery servers remain available throughout component cleanup. + const bool layers_stopped = utils::shutdownProcesses(layers, grace); + const bool servers_stopped = utils::shutdownProcesses(discovery_servers, std::chrono::milliseconds(2000)); + if (!layers_stopped || !servers_stopped) + throw std::runtime_error("Framework shutdown left processes alive; see PID/PGID diagnostics"); std::cout << "=== FRAMEWORK NODE STOPPED ===" << std::endl; } @@ -145,29 +138,21 @@ namespace dls bool RunDLS2::runLayer(const std::string &lib, const std::string &ID) { - std::shared_ptr pData = std::make_shared(ID); - - pData->proc = std::make_shared(std::vector( + if (should_quit.load()) return false; + auto process = std::make_shared(std::vector( {"${DLS_INSTALL_RUNTIME_DIR}/child_process_launcher", - pData->getID(), + ID, lib, "layer", - Options::robot_name})); - - if (pData->proc == nullptr) - { - std::cout << "Layer " << lib << " failed to launch: nullptr" << std::endl; - return false; - } + Options::robot_name}), lib != "console"); - pData->proc->detach(); - layers.emplace(pData->getID(), pData); + layers.emplace(ID, process); // activate layer - sm_watcher.waitState(pData->getID(), "idle", should_quit); - if(command_manager.waitCommand(pData->getID(), "activate", should_quit)) - command_manager.callCommand("activate", {}, pData->getID()); - sm_watcher.waitState(pData->getID(), "run", should_quit); + sm_watcher.waitState(ID, "idle", should_quit); + if(command_manager.waitCommand(ID, "activate", should_quit)) + command_manager.callCommand("activate", {}, ID); + sm_watcher.waitState(ID, "run", should_quit); return true; } @@ -197,12 +182,11 @@ namespace dls void RunDLS2::launchSingleServer(const std::string& ip, int port){ static size_t server_id = 0; - std::string command = "setsid fastdds discovery -p " +std::to_string(port)+ " --server-id " + std::to_string(server_id) + " -l "+ip+"&"; - server_id++; - std::cout << command << std::endl; - if(system(command.c_str())!=0){ - throw std::runtime_error("CANNOT EXECUTE COMMAND " + command); - } + if (should_quit.load()) return; + const auto id = std::to_string(server_id++); + discovery_servers.emplace("DiscoveryServer" + id, std::make_shared( + std::vector{"/usr/bin/env", "fastdds", "discovery", + "-p", std::to_string(port), "--server-id", id, "-l", ip})); } void RunDLS2::runStartup(const std::string &startup_file) @@ -276,6 +260,7 @@ namespace dls // launch hardware first so dependent apps do not race missing inputs for (auto hardware : applications["hardwares"]) { + if (should_quit.load()) return; command_manager.callCommand(app_to_loading_command["hardwares"], {hardware}, app_to_layer["hardwares"]); sm_watcher.waitState(hardware, "idle", should_quit); @@ -289,6 +274,7 @@ namespace dls } // loadModel does not have effect on the real robot because the HAL is directly loaded there. + if (should_quit.load()) return; sleep(1); // if the model is spawned too fast (in gazebo) the simulation breaks if(command_manager.waitCommand(hardware, "loadModel", should_quit, 10000)) command_manager.callCommand("loadModel", {Options::robot_name, std::to_string(Options::robot_spawning_height)}, hardware); @@ -302,6 +288,7 @@ namespace dls for (auto name : app_names) { + if (should_quit.load()) return; command_manager.callCommand(app_to_loading_command[app_type], {name}, app_to_layer[app_type]); sm_watcher.waitState(name, "idle", should_quit); if(std::find(active_apps.begin(), active_apps.end(), name) != active_apps.end()) From 6b85039146777d35c0942aecaae0ff3326ebc44e Mon Sep 17 00:00:00 2001 From: mich-pest Date: Tue, 8 Sep 2026 16:15:32 +0200 Subject: [PATCH 07/11] cmake: shutdown timeout env var added --- scripts/CMakeLists.txt | 1 + scripts/launch_framework.in | 1 + 2 files changed, 2 insertions(+) diff --git a/scripts/CMakeLists.txt b/scripts/CMakeLists.txt index 9f3973b4..f1c8d7a4 100644 --- a/scripts/CMakeLists.txt +++ b/scripts/CMakeLists.txt @@ -18,6 +18,7 @@ set(DLS_CHILD_PROCESS_LAUNCHER "${DLS_INSTALL_RUNTIME_DIR}/child_pr set(DLS_LAUNCH_SCRIPT_PATH "/usr/local/bin:/usr/bin:\${PATH}") set(DLS_LAUNCH_SCRIPT_LIBRARY_PATH "/usr/local/lib:/usr/lib/dls2:${DLS_LD_LIBRARY_PATH}:\${LD_LIBRARY_PATH}") set(DLS_LUANCH_SCRIPT_RUN_FRAMEWORK_COMMAND "${DLS_INSTALL_RUNTIME_DIR}/dynamic_legged_systems_framework \"$@\"") +set(DLS_SHUTDOWN_TIMEOUT_MS "\${DLS_SHUTDOWN_TIMEOUT_MS:-5000}") dls_configure(launch_framework.in LAUNCH_FRAMEWORK) # Configure the version of the launch script that is used to launch the framework diff --git a/scripts/launch_framework.in b/scripts/launch_framework.in index b311d564..9004014d 100644 --- a/scripts/launch_framework.in +++ b/scripts/launch_framework.in @@ -1,6 +1,7 @@ #!/usr/bin/env bash export DLS_CHILD_PROCESS_LAUNCHER=@DLS_CHILD_PROCESS_LAUNCHER@ +export DLS_SHUTDOWN_TIMEOUT_MS="@DLS_SHUTDOWN_TIMEOUT_MS@" export PATH="@DLS_LAUNCH_SCRIPT_PATH@" export LD_LIBRARY_PATH="@DLS_LAUNCH_SCRIPT_LIBRARY_PATH@" From d2e3c265e357cdaa496ce743cfae9f681f5987b2 Mon Sep 17 00:00:00 2001 From: mich-pest Date: Tue, 8 Sep 2026 16:30:43 +0200 Subject: [PATCH 08/11] run_dls2: fix discovery servers not killed on ctrl+c --- modules/core_framework/src/options.cpp.in | 2 +- modules/main/src/run_dls2.cpp.in | 26 +++++++---------------- 2 files changed, 9 insertions(+), 19 deletions(-) diff --git a/modules/core_framework/src/options.cpp.in b/modules/core_framework/src/options.cpp.in index 6d283ce0..d16f47a4 100644 --- a/modules/core_framework/src/options.cpp.in +++ b/modules/core_framework/src/options.cpp.in @@ -245,7 +245,7 @@ void Options::printUsage() "| layers | l | a comma-separated list of layers to launch |\n" "| super | c | launch the supervisor (c is for core) |\n" "| startup | s | launch the startup procedure |\n" - "| kill | k | force kill dls2 |\n" + "| kill | k | request graceful dls2 shutdown |\n" "| version | v | print the version and exit |\n" "| help | h | print this help and exit |\n" "| docs | d | show development documentation |\n" diff --git a/modules/main/src/run_dls2.cpp.in b/modules/main/src/run_dls2.cpp.in index c0fd4c38..40d7f0a2 100644 --- a/modules/main/src/run_dls2.cpp.in +++ b/modules/main/src/run_dls2.cpp.in @@ -5,6 +5,7 @@ #include #include #include +#include namespace dls { @@ -57,25 +58,14 @@ namespace dls if (Options::kill_dls2) { - int ret = system( - "pkill -9 dls_framework;\ - pkill -9 ControlLayer;\ - pkill -9 ConsoleLayer;\ - pkill -9 ControlLayer;\ - pkill -9 LogLayer;\ - pkill -9 HardwareLayer;\ - pkill -9 EstimationLayer;\ - pkill -9 dls_param_serve;\ - pkill -9 Supervisor;\ - pkill -9 gazebo_sim;\ - pkill -9 gz"); - if (ret == -1) + // Let each framework stop its owned layers, then its discovery servers. + const int ret = system("pkill -TERM -x dls_framework"); + // pkill returns 1 when no matching process could be signalled. + if (ret == -1 || !WIFEXITED(ret) || WEXITSTATUS(ret) > 1) { - throw std::runtime_error("Error executing system command to show the documentation."); + throw std::runtime_error("Failed to request framework shutdown."); } - - command_manager.callCommand("shutdown", {"all"}); - exit(EXIT_SUCCESS); + return; } // Run startup procedure if requested by the user if (Options::run_startup) @@ -300,4 +290,4 @@ namespace dls } } } -} \ No newline at end of file +} From da3b7934dae9bc1a20a796ac0fb49ca025883ab4 Mon Sep 17 00:00:00 2001 From: mich-pest Date: Tue, 8 Sep 2026 16:51:32 +0200 Subject: [PATCH 09/11] console_layer: console killing framework and viceversa --- .../src/child_process_launcher.cpp.in | 4 ++- .../dls2/core_framework/console_layer.hpp | 5 +++- .../core_framework/src/console_layer.cpp.in | 28 +++++++++++-------- modules/main/src/run_dls2.cpp.in | 2 ++ 4 files changed, 26 insertions(+), 13 deletions(-) diff --git a/modules/child_process/src/child_process_launcher.cpp.in b/modules/child_process/src/child_process_launcher.cpp.in index a3e27ad6..6307b72b 100644 --- a/modules/child_process/src/child_process_launcher.cpp.in +++ b/modules/child_process/src/child_process_launcher.cpp.in @@ -190,10 +190,12 @@ int main(int argc, char **argv) exit(EXIT_FAILURE); } - shutdown_signal.start([pComponent] { + shutdown_signal.start([pComponent, is_console = args.component_type == "layer" && args.lib_name == "console"] { // Initialization has no quit transition. Retain an early signal until idle. const auto state = pComponent->sm.getStateName(); if (state == "initialization") return false; + // Ctrl+C in a separate console must also stop the connected framework. + if (is_console) pComponent->command_manager.callCommand("shutdown", {"all"}); if (state != "quit") pComponent->stop(); return true; }); diff --git a/modules/core_framework/include/dls2/core_framework/console_layer.hpp b/modules/core_framework/include/dls2/core_framework/console_layer.hpp index 8c442eda..66fc931b 100644 --- a/modules/core_framework/include/dls2/core_framework/console_layer.hpp +++ b/modules/core_framework/include/dls2/core_framework/console_layer.hpp @@ -6,6 +6,7 @@ #include #include #include +#include namespace dls { @@ -53,11 +54,13 @@ namespace dls // needed to unblock the console from the readline void stop() override; + bool shutdownRequested() const { return shutdown_requested_.load(); } private: + std::atomic_bool shutdown_requested_{false}; // Map with "load[Layer]" commands and associated installation folders const std::map load_layers_paths_; }; } // namespace dls -#endif \ No newline at end of file +#endif diff --git a/modules/core_framework/src/console_layer.cpp.in b/modules/core_framework/src/console_layer.cpp.in index 300b424d..755052a7 100644 --- a/modules/core_framework/src/console_layer.cpp.in +++ b/modules/core_framework/src/console_layer.cpp.in @@ -225,7 +225,12 @@ namespace dls rl_reset_line_state(); } } - int event(void){return 0;} + int event(void) + { + // Readline state is accessed only by the thread running readline. + if (console_layer && console_layer->shutdownRequested()) rl_done = 1; + return 0; + } } // namespace readline_completion // dls::ConsoleLayer implementation @@ -242,8 +247,8 @@ namespace dls { // Assign the dls::readline_completion::ConsoleLayer pointer to the dls::ConsoleLayer object readline_completion::console_layer = this; - // Make readline reading rl_done variable, set when shutting down the control layer in the stop() function. - // When rl_done variable is set to a value !=0, readline return immediately. So the console layer will not wait for a character before exiting + // Keep the launcher's signal handler and unblock input on a shutdown request. + rl_catch_signals = 0; rl_event_hook = readline_completion::event; command_manager.addCommand<> @@ -286,10 +291,7 @@ namespace dls ConsoleLayer::~ConsoleLayer() { - // Free the ConsoleLayer pointer ("dls::readline_completion" namespace) allocated memory - delete readline_completion::console_layer; - - // Make the dangling pointer point to "null" + // This is a non-owning pointer to this object. readline_completion::console_layer = nullptr; } @@ -352,12 +354,11 @@ namespace dls std::cout << std::endl; } - void ConsoleLayer::close(){} + void ConsoleLayer::close(){ shutdown_requested_.store(true); } void ConsoleLayer::stop() { - // When rl_done variable is set to a value !=0, readline return immediately. So the console layer will not wait for a character before exiting - rl_done = 1; + shutdown_requested_.store(true); sm.raiseEvent(sm.quit_request); } @@ -368,11 +369,16 @@ namespace dls } void ConsoleLayer::monitor(){ + if (shutdownRequested()) return; rl_attempted_completion_function = readline_completion::command_completion; rl_completion_display_matches_hook = readline_completion::display_matches; // The line "readline" returned is allocated with "malloc". It is freed (manually) when we are done with it char *line = readline(std::string("> ").c_str()); + if (shutdownRequested()) { + free(line); + return; + } if (line != nullptr) { @@ -440,4 +446,4 @@ namespace dls free(line); } } -} // namespace dls \ No newline at end of file +} // namespace dls diff --git a/modules/main/src/run_dls2.cpp.in b/modules/main/src/run_dls2.cpp.in index 40d7f0a2..1132922e 100644 --- a/modules/main/src/run_dls2.cpp.in +++ b/modules/main/src/run_dls2.cpp.in @@ -106,6 +106,8 @@ namespace dls std::cerr << "Invalid DLS_SHUTDOWN_TIMEOUT_MS; using 10000 ms" << std::endl; } } + // Separate console/framework terminals share DDS, not an OS process group. + command_manager.callCommand("shutdown", {"all"}); // OS signals reach owned descendants even when DDS discovery is unavailable. // Discovery servers remain available throughout component cleanup. const bool layers_stopped = utils::shutdownProcesses(layers, grace); From 0eb50e3d9bbd5d1c2f3b669c2fe7ea8d87527af4 Mon Sep 17 00:00:00 2001 From: mich-pest Date: Tue, 8 Sep 2026 16:51:52 +0200 Subject: [PATCH 10/11] utils: owned_process ControlLayer bug fix --- .../utils/include/dls2/util/owned_process.hpp | 18 ++++++++++++++++-- 1 file changed, 16 insertions(+), 2 deletions(-) diff --git a/modules/utils/include/dls2/util/owned_process.hpp b/modules/utils/include/dls2/util/owned_process.hpp index d0bb3d58..fee00f0d 100644 --- a/modules/utils/include/dls2/util/owned_process.hpp +++ b/modules/utils/include/dls2/util/owned_process.hpp @@ -1,6 +1,7 @@ #pragma once #include +#include #include #include #include @@ -10,6 +11,7 @@ #include #include #include +#include #include namespace dls::utils @@ -22,8 +24,20 @@ class OwnedProcess public: // The interactive console must retain the terminal's foreground group. explicit OwnedProcess(const std::vector& args, bool own_group = true) - : proc(own_group ? boost::process::child(args, group_) : boost::process::child(args)), - own_group_(own_group) {} + : proc(own_group ? boost::process::child(args, + boost::process::extend::on_exec_setup = [](auto& executor) { + // Keep inherited stdin/stdout, but detach from terminal job control. + // setpgid alone lets keyboard I/O suspend the entire layer group. + if (::setsid() == -1) + executor.set_error(std::error_code(errno, std::generic_category()), "setsid"); + }) : boost::process::child(args)), + own_group_(own_group) + { + if (own_group_) { + auto pgid = proc.id(); // setsid creates a group with the child's PID. + group_ = boost::process::group(pgid); + } + } bool running() { From d6b849ca4cf4c58ad5f5a0618d35fbc075f38ecb Mon Sep 17 00:00:00 2001 From: MMarcus95 Date: Mon, 14 Sep 2026 15:50:55 +0200 Subject: [PATCH 11/11] fix merging of main --- modules/main/src/run_dls2.cpp.in | 12 ++++++------ 1 file changed, 6 insertions(+), 6 deletions(-) diff --git a/modules/main/src/run_dls2.cpp.in b/modules/main/src/run_dls2.cpp.in index 7720b3f1..ef7600cc 100644 --- a/modules/main/src/run_dls2.cpp.in +++ b/modules/main/src/run_dls2.cpp.in @@ -153,10 +153,10 @@ namespace dls for (int elapsed_ms = 0; elapsed_ms < STARTUP_STATE_TIMEOUT_MS && !should_quit; elapsed_ms += 5000) { - if (sm_watcher.waitState(pData->getID(), state, should_quit, false)) + if (sm_watcher.waitState(ID, state, should_quit, false)) return true; } - std::cerr << "Layer " << pData->getID() << " did not reach " << state + std::cerr << "Layer " << ID << " did not reach " << state << " within " << STARTUP_STATE_TIMEOUT_MS / 1000 << " seconds" << std::endl; return false; }; @@ -167,16 +167,16 @@ namespace dls for (int elapsed_ms = 0; elapsed_ms < STARTUP_DISCOVERY_TIMEOUT_MS && !should_quit; elapsed_ms += STARTUP_RETRY_INTERVAL_MS) { - if (command_manager.find(pData->getID(), "activate").size() != 1) + if (command_manager.find(ID, "activate").size() != 1) { std::this_thread::sleep_for(std::chrono::milliseconds(STARTUP_RETRY_INTERVAL_MS)); continue; } - if (command_manager.callCommand("activate", {}, pData->getID()) == 1) + if (command_manager.callCommand("activate", {}, ID) == 1) return waitForState("run"); } - std::cerr << "Layer " << pData->getID() + std::cerr << "Layer " << ID << " did not expose a unique activate command within " << STARTUP_DISCOVERY_TIMEOUT_MS / 1000 << " seconds" << std::endl; return false; @@ -337,7 +337,7 @@ namespace dls return false; // loadModel does not have effect on the real robot because the HAL is directly loaded there. - if (should_quit.load()) return; + if (should_quit.load()) return false; sleep(1); // if the model is spawned too fast (in gazebo) the simulation breaks if (!callRequiredCommand(hardware, "loadModel", {Options::robot_name, std::to_string(Options::robot_spawning_height)}))