From: Razvan Becheriu Date: Thu, 18 Feb 2021 09:40:36 +0000 (+0200) Subject: [#899] waitForProcess must be a static function X-Git-Tag: Kea-1.9.5~48 X-Git-Url: http://git.ipfire.org/cgi-bin/gitweb.cgi?a=commitdiff_plain;h=38793cbe3a0342574aaedd7b36c4f414b16cbf2b;p=thirdparty%2Fkea.git [#899] waitForProcess must be a static function --- diff --git a/src/hooks/dhcp/run_script/libloadtests/load_unload_unittests.cc b/src/hooks/dhcp/run_script/libloadtests/load_unload_unittests.cc index dbd864f85c..4eb4eba152 100644 --- a/src/hooks/dhcp/run_script/libloadtests/load_unload_unittests.cc +++ b/src/hooks/dhcp/run_script/libloadtests/load_unload_unittests.cc @@ -45,7 +45,7 @@ public: HooksManager::unloadLibraries(); } - /// @breif Add library to the collection of tested libraries. + /// @brief Add library to the collection of tested libraries. /// /// @param lib The name of the lib added. /// @param params The parameters of the library. diff --git a/src/lib/asiolink/io_service.cc b/src/lib/asiolink/io_service.cc index 2140f2c2d1..374eac0dd5 100644 --- a/src/lib/asiolink/io_service.cc +++ b/src/lib/asiolink/io_service.cc @@ -47,8 +47,9 @@ public: /// \brief The constructor IOServiceImpl() : io_service_(), - work_(new boost::asio::io_service::work(io_service_)) - {}; + work_(new boost::asio::io_service::work(io_service_)) { + }; + /// \brief The destructor. ~IOServiceImpl() {}; //@} @@ -109,12 +110,10 @@ private: boost::shared_ptr work_; }; -IOService::IOService() { - io_impl_ = new IOServiceImpl(); +IOService::IOService() : io_impl_(new IOServiceImpl()) { } IOService::~IOService() { - delete io_impl_; } void diff --git a/src/lib/asiolink/io_service.h b/src/lib/asiolink/io_service.h index f6f33c15e2..fcce53c47c 100644 --- a/src/lib/asiolink/io_service.h +++ b/src/lib/asiolink/io_service.h @@ -95,7 +95,7 @@ public: void post(const std::function& callback); private: - IOServiceImpl* io_impl_; + boost::shared_ptr io_impl_; }; /// @brief Defines a smart pointer to an IOService instance. diff --git a/src/lib/asiolink/process_spawn.cc b/src/lib/asiolink/process_spawn.cc index 2090424329..2ae989ef3c 100644 --- a/src/lib/asiolink/process_spawn.cc +++ b/src/lib/asiolink/process_spawn.cc @@ -43,6 +43,12 @@ struct ProcessState { /// identified by PID. typedef std::map ProcessStates; +class ProcessSpawnImpl; + +/// @brief ProcessCollection container which stores all ProcessStates for each +/// instance of @ref ProcessSpawnImpl. +typedef std::map ProcessCollection; + /// @brief Implementation of the @c ProcessSpawn class. /// /// This pimpl idiom is used by the @c ProcessSpawn in this case to @@ -146,10 +152,10 @@ private: /// /// @return true if the processed signal was SIGCHLD or false if it /// was a different signal. - bool waitForProcess(int signum); + static bool waitForProcess(int signum); /// @brief A map holding the status codes of executed processes. - ProcessStates process_state_; + static ProcessCollection process_collection_; /// @brief Path to an executable. std::string executable_; @@ -160,28 +166,34 @@ private: /// @brief An array holding environment variables for the executable. boost::shared_ptr vars_; - /// @breif Typedef for CString pointer. + /// @brief Typedef for CString pointer. typedef boost::shared_ptr CStringPtr; /// @brief An storage container for all allocated C strings. std::vector storage_; + /// @brief Flag to indicate if process status must be stored. + bool store_; + /// @brief Mutex to protect internal state. - boost::shared_ptr mutex_; + static std::mutex mutex_; /// @brief ASIO signal set. IOSignalSetPtr io_signal_set_; }; +ProcessCollection ProcessSpawnImpl::process_collection_; +std::mutex ProcessSpawnImpl::mutex_; + ProcessSpawnImpl::ProcessSpawnImpl(IOServicePtr io_service, const std::string& executable, const ProcessArgs& args, const ProcessEnvVars& vars) : executable_(executable), args_(new char*[args.size() + 2]), - vars_(new char*[vars.size() + 1]), mutex_(new std::mutex), + vars_(new char*[vars.size() + 1]), store_(false), io_signal_set_(new IOSignalSet(io_service, std::bind(&ProcessSpawnImpl::waitForProcess, - this, ph::_1))) { + ph::_1))) { io_signal_set_->add(SIGCHLD); // Conversion of the arguments to the C-style array we start by setting @@ -203,6 +215,10 @@ ProcessSpawnImpl::ProcessSpawnImpl(IOServicePtr io_service, ProcessSpawnImpl::~ProcessSpawnImpl() { io_signal_set_->remove(SIGCHLD); + if (store_) { + lock_guard lk(mutex_); + process_collection_.erase(this); + } } std::string @@ -255,8 +271,9 @@ ProcessSpawnImpl::spawn(bool dismiss) { // We're in the parent process. if (!dismiss) { try { - lock_guard lk(*mutex_); - process_state_.insert(std::pair(pid, ProcessState())); + lock_guard lk(mutex_); + store_ = true; + process_collection_[this].insert(std::pair(pid, ProcessState())); } catch(...) { pthread_sigmask(SIG_SETMASK, &osset, 0); throw; @@ -268,9 +285,10 @@ ProcessSpawnImpl::spawn(bool dismiss) { bool ProcessSpawnImpl::isRunning(const pid_t pid) const { - lock_guard lk(*mutex_); - ProcessStates::const_iterator proc = process_state_.find(pid); - if (proc == process_state_.end()) { + lock_guard lk(mutex_); + ProcessStates::const_iterator proc; + if (process_collection_.find(this) == process_collection_.end() || + (proc = process_collection_[this].find(pid)) == process_collection_[this].end()) { isc_throw(BadValue, "the process with the pid '" << pid << "' hasn't been spawned and it status cannot be" " returned"); @@ -280,11 +298,12 @@ ProcessSpawnImpl::isRunning(const pid_t pid) const { bool ProcessSpawnImpl::isAnyRunning() const { - lock_guard lk(*mutex_); - for (ProcessStates::const_iterator proc = process_state_.begin(); - proc != process_state_.end(); ++proc) { - if (proc->second.running_) { - return (true); + lock_guard lk(mutex_); + if (process_collection_.find(this) != process_collection_.end()) { + for (auto proc : process_collection_[this]) { + if (proc.second.running_) { + return (true); + } } } return (false); @@ -292,9 +311,10 @@ ProcessSpawnImpl::isAnyRunning() const { int ProcessSpawnImpl::getExitStatus(const pid_t pid) const { - lock_guard lk(*mutex_); - ProcessStates::const_iterator proc = process_state_.find(pid); - if (proc == process_state_.end()) { + lock_guard lk(mutex_); + ProcessStates::const_iterator proc; + if (process_collection_.find(this) == process_collection_.end() || + (proc = process_collection_[this].find(pid)) == process_collection_[this].end()) { isc_throw(InvalidOperation, "the process with the pid '" << pid << "' hasn't been spawned and it status cannot be" " returned"); @@ -317,20 +337,22 @@ ProcessSpawnImpl::allocateInternal(const std::string& src) { bool ProcessSpawnImpl::waitForProcess(int) { - lock_guard lk(*mutex_); + lock_guard lk(mutex_); for (;;) { int status = 0; pid_t pid = waitpid(-1, &status, WNOHANG); if (pid <= 0) { break; } - ProcessStates::iterator proc = process_state_.find(pid); - /// Check that the terminating process was started - /// by our instance of ProcessSpawn - if (proc != process_state_.end()) { - // In this order please - proc->second.status_ = status; - proc->second.running_ = false; + for (auto instance : process_collection_) { + auto proc = instance.second.find(pid); + /// Check that the terminating process was started + /// by our instance of ProcessSpawn + if (proc != instance.second.end()) { + // In this order please + proc->second.status_ = status; + proc->second.running_ = false; + } } } @@ -343,8 +365,10 @@ ProcessSpawnImpl::clearState(const pid_t pid) { isc_throw(InvalidOperation, "unable to remove the status for the" "process (pid: " << pid << ") which is still running"); } - lock_guard lk(*mutex_); - process_state_.erase(pid); + lock_guard lk(mutex_); + if (process_collection_.find(this) != process_collection_.end()) { + process_collection_[this].erase(pid); + } } ProcessSpawn::ProcessSpawn(IOServicePtr io_service, @@ -354,9 +378,6 @@ ProcessSpawn::ProcessSpawn(IOServicePtr io_service, : impl_(new ProcessSpawnImpl(io_service, executable, args, vars)) { } -ProcessSpawn::~ProcessSpawn() { -} - std::string ProcessSpawn::getCommandLine() const { return (impl_->getCommandLine()); diff --git a/src/lib/asiolink/process_spawn.h b/src/lib/asiolink/process_spawn.h index ef37174902..e8dd805d3a 100644 --- a/src/lib/asiolink/process_spawn.h +++ b/src/lib/asiolink/process_spawn.h @@ -72,7 +72,7 @@ public: const ProcessEnvVars& vars = ProcessEnvVars()); /// @brief Destructor. - ~ProcessSpawn(); + ~ProcessSpawn() = default; /// @brief Returns full command line, including arguments, for the process. std::string getCommandLine() const;