From: Razvan Becheriu Date: Wed, 17 Feb 2021 19:50:10 +0000 (+0200) Subject: [#899] clear signal handle after process has finished X-Git-Tag: Kea-1.9.5~49 X-Git-Url: http://git.ipfire.org/cgi-bin/gitweb.cgi?a=commitdiff_plain;h=0aaf01c1918f730dcb0a4e33157c91123caf761a;p=thirdparty%2Fkea.git [#899] clear signal handle after process has finished --- diff --git a/src/hooks/dhcp/run_script/run_script.cc b/src/hooks/dhcp/run_script/run_script.cc index 1a177c4c3e..a7748e95d8 100644 --- a/src/hooks/dhcp/run_script/run_script.cc +++ b/src/hooks/dhcp/run_script/run_script.cc @@ -45,7 +45,7 @@ RunScriptImpl::configure(LibraryHandle& handle) { void RunScriptImpl::runScript(const ProcessArgs& args, const ProcessEnvVars& vars) { ProcessSpawn process(io_service_, name_, args, vars); - process.spawn(); + process.spawn(true); } void diff --git a/src/hooks/dhcp/run_script/run_script.h b/src/hooks/dhcp/run_script/run_script.h index d12612d037..4fd370c7b1 100644 --- a/src/hooks/dhcp/run_script/run_script.h +++ b/src/hooks/dhcp/run_script/run_script.h @@ -195,7 +195,7 @@ public: /// @brief Get name of the target script. /// /// @return The name of the target script. - std::string getName() { + std::string getName() const { return (name_); } @@ -209,7 +209,7 @@ public: /// @brief Get the synchronous call mode for the target script. /// /// @return The synchronous call mode for the target script. - bool getSync() { + bool getSync() const { return (sync_); } diff --git a/src/lib/asiolink/io_service_signal.cc b/src/lib/asiolink/io_service_signal.cc index 44c7658a14..d75fac67c5 100644 --- a/src/lib/asiolink/io_service_signal.cc +++ b/src/lib/asiolink/io_service_signal.cc @@ -20,9 +20,8 @@ namespace isc { namespace asiolink { /// @brief Implementation class of IOSignalSet. -class IOSignalSetImpl : - public boost::enable_shared_from_this, - public boost::noncopyable { +class IOSignalSetImpl : public boost::enable_shared_from_this, + public boost::noncopyable { public: /// @brief Constructor. /// @@ -31,16 +30,21 @@ public: IOSignalSetImpl(IOServicePtr io_service, IOSignalHandler handler); /// @brief Destructor. - ~IOSignalSetImpl(){} + ~IOSignalSetImpl() = default; /// @brief Install the callback on the IO service queue. void install(); - /// @brief Add a signal in the ASIO signal set. + /// @brief Add a signal to the ASIO signal set. /// /// @param signum the signal number. void add(int signum); + /// @brief Remove a signal from the ASIO signal set. + /// + /// @param signum the signal number. + void remove(int signum); + private: /// @brief the ASIO signal set. boost::asio::signal_set signal_set_; @@ -85,8 +89,18 @@ IOSignalSetImpl::add(int signum) { try { signal_set_.add(signum); } catch (const boost::system::system_error& ex) { - isc_throw(isc::Unexpected, "Failed to add signal " << signum - << ": " << ex.what()); + isc_throw(isc::Unexpected, + "Failed to add signal " << signum << ": " << ex.what()); + } +} + +void +IOSignalSetImpl::remove(int signum) { + try { + signal_set_.remove(signum); + } catch (const boost::system::system_error& ex) { + isc_throw(isc::Unexpected, + "Failed to add signal " << signum << ": " << ex.what()); } } @@ -96,14 +110,15 @@ IOSignalSet::IOSignalSet(IOServicePtr io_service, IOSignalHandler handler) : impl_->install(); } -IOSignalSet::~IOSignalSet() { - impl_.reset(); -} - void IOSignalSet::add(int signum) { impl_->add(signum); } +void +IOSignalSet::remove(int signum) { + impl_->remove(signum); +} + } // namespace asiolink } // namespace isc diff --git a/src/lib/asiolink/io_service_signal.h b/src/lib/asiolink/io_service_signal.h index 7efabf9646..5032e8e526 100644 --- a/src/lib/asiolink/io_service_signal.h +++ b/src/lib/asiolink/io_service_signal.h @@ -32,7 +32,7 @@ public: IOSignalSet(asiolink::IOServicePtr io_service, IOSignalHandler handler); /// @brief Destructor. - ~IOSignalSet(); + ~IOSignalSet() = default; /// @brief Add a signal to the list of signals to handle. /// @@ -40,6 +40,12 @@ public: /// @throw Unexpected on error. void add(int signum); + /// @brief Remove a signal from the list of signals to handle. + /// + /// @param signum Signal number. + /// @throw Unexpected on error. + void remove(int signum); + private: /// @brief Pointer to the implementation. boost::shared_ptr impl_; diff --git a/src/lib/asiolink/process_spawn.cc b/src/lib/asiolink/process_spawn.cc index 05854c885c..2090424329 100644 --- a/src/lib/asiolink/process_spawn.cc +++ b/src/lib/asiolink/process_spawn.cc @@ -86,9 +86,11 @@ public: /// or when the executable does not exist. If the process ends successfully /// the EXIT_SUCCESS is returned. /// + /// @param dismiss The flag which indicated if the process status can be + /// disregarded. /// @return PID of the spawned process. /// @throw ProcessSpawnError if forking a current process failed. - pid_t spawn(); + pid_t spawn(bool dismiss); /// @brief Checks if the process is still running. /// @@ -146,9 +148,6 @@ private: /// was a different signal. bool waitForProcess(int signum); - /// @brief ASIO signal set. - IOSignalSetPtr io_signal_set_; - /// @brief A map holding the status codes of executed processes. ProcessStates process_state_; @@ -169,6 +168,9 @@ private: /// @brief Mutex to protect internal state. boost::shared_ptr mutex_; + + /// @brief ASIO signal set. + IOSignalSetPtr io_signal_set_; }; ProcessSpawnImpl::ProcessSpawnImpl(IOServicePtr io_service, @@ -176,10 +178,10 @@ ProcessSpawnImpl::ProcessSpawnImpl(IOServicePtr io_service, const ProcessArgs& args, const ProcessEnvVars& vars) : executable_(executable), args_(new char*[args.size() + 2]), - vars_(new char*[vars.size() + 1]), mutex_(new std::mutex) { - io_signal_set_.reset(new IOSignalSet(io_service, - std::bind(&ProcessSpawnImpl::waitForProcess, - this, ph::_1))); + vars_(new char*[vars.size() + 1]), mutex_(new std::mutex), + io_signal_set_(new IOSignalSet(io_service, + std::bind(&ProcessSpawnImpl::waitForProcess, + this, ph::_1))) { io_signal_set_->add(SIGCHLD); // Conversion of the arguments to the C-style array we start by setting @@ -200,6 +202,7 @@ ProcessSpawnImpl::ProcessSpawnImpl(IOServicePtr io_service, } ProcessSpawnImpl::~ProcessSpawnImpl() { + io_signal_set_->remove(SIGCHLD); } std::string @@ -218,7 +221,7 @@ ProcessSpawnImpl::getCommandLine() const { } pid_t -ProcessSpawnImpl::spawn() { +ProcessSpawnImpl::spawn(bool dismiss) { // Protect us against SIGCHLD signals sigset_t sset; sigset_t osset; @@ -250,12 +253,14 @@ ProcessSpawnImpl::spawn() { } // We're in the parent process. - try { - lock_guard lk(*mutex_); - process_state_.insert(std::pair(pid, ProcessState())); - } catch(...) { - pthread_sigmask(SIG_SETMASK, &osset, 0); - throw; + if (!dismiss) { + try { + lock_guard lk(*mutex_); + process_state_.insert(std::pair(pid, ProcessState())); + } catch(...) { + pthread_sigmask(SIG_SETMASK, &osset, 0); + throw; + } } pthread_sigmask(SIG_SETMASK, &osset, 0); return (pid); @@ -358,8 +363,8 @@ ProcessSpawn::getCommandLine() const { } pid_t -ProcessSpawn::spawn() { - return (impl_->spawn()); +ProcessSpawn::spawn(bool dismiss) { + return (impl_->spawn(dismiss)); } bool diff --git a/src/lib/asiolink/process_spawn.h b/src/lib/asiolink/process_spawn.h index b0b2971d0d..ef37174902 100644 --- a/src/lib/asiolink/process_spawn.h +++ b/src/lib/asiolink/process_spawn.h @@ -87,8 +87,10 @@ public: /// or when the executable does not exist. If the process ends successfully /// the EXIT_SUCCESS is returned. /// + /// @param dismiss The flag which indicated if the process status can be + /// disregarded. /// @throw ProcessSpawnError if forking a current process failed. - pid_t spawn(); + pid_t spawn(bool dismiss = false); /// @brief Checks if the process is still running. /// diff --git a/src/lib/util/util.dox b/src/lib/util/util.dox index 8781f2f555..e4ef04451a 100644 --- a/src/lib/util/util.dox +++ b/src/lib/util/util.dox @@ -50,7 +50,7 @@ The utilities library (libkea-util) provides generic and Kea utilities: - read-write mutex (header only). - - signal set: signal handling (please use @c isc::process::IOSignalSet + - signal set: signal handling (please use @c isc::asiolink::IOSignalSet instead). - staged values.