]> git.ipfire.org Git - thirdparty/kea.git/commitdiff
[#899] waitForProcess must be a static function
authorRazvan Becheriu <razvan@isc.org>
Thu, 18 Feb 2021 09:40:36 +0000 (11:40 +0200)
committerRazvan Becheriu <razvan@isc.org>
Thu, 18 Feb 2021 17:23:57 +0000 (19:23 +0200)
src/hooks/dhcp/run_script/libloadtests/load_unload_unittests.cc
src/lib/asiolink/io_service.cc
src/lib/asiolink/io_service.h
src/lib/asiolink/process_spawn.cc
src/lib/asiolink/process_spawn.h

index dbd864f85cd751b4ce365e2e6634e5f58ba61cd2..4eb4eba1521ccb3292c8b59084f447ce513b471b 100644 (file)
@@ -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.
index 2140f2c2d1fcc73fb258018f66412e36a000c6b0..374eac0dd5da65521355d5ce666532d36de31780 100644 (file)
@@ -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<boost::asio::io_service::work> work_;
 };
 
-IOService::IOService() {
-    io_impl_ = new IOServiceImpl();
+IOService::IOService() : io_impl_(new IOServiceImpl()) {
 }
 
 IOService::~IOService() {
-    delete io_impl_;
 }
 
 void
index f6f33c15e293d9b6c01bb1ebed435fb776f02386..fcce53c47cd8ce1fdc39844da35ceac0979000e7 100644 (file)
@@ -95,7 +95,7 @@ public:
     void post(const std::function<void ()>& callback);
 
 private:
-    IOServiceImpl* io_impl_;
+    boost::shared_ptr<IOServiceImpl> io_impl_;
 };
 
 /// @brief Defines a smart pointer to an IOService instance.
index 2090424329e07ffcdc0e87de1ce9f5a6ca65f768..2ae989ef3cbfa0c7ea2507fccfab8e38296deca4 100644 (file)
@@ -43,6 +43,12 @@ struct ProcessState {
 /// identified by PID.
 typedef std::map<pid_t, ProcessState> ProcessStates;
 
+class ProcessSpawnImpl;
+
+/// @brief ProcessCollection container which stores all ProcessStates for each
+/// instance of @ref ProcessSpawnImpl.
+typedef std::map<const ProcessSpawnImpl*, ProcessStates> 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<char*[]> vars_;
 
-    /// @breif Typedef for CString pointer.
+    /// @brief Typedef for CString pointer.
     typedef boost::shared_ptr<char[]> CStringPtr;
 
     /// @brief An storage container for all allocated C strings.
     std::vector<CStringPtr> storage_;
 
+    /// @brief Flag to indicate if process status must be stored.
+    bool store_;
+
     /// @brief Mutex to protect internal state.
-    boost::shared_ptr<std::mutex> 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<std::mutex> 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<std::mutex> lk(*mutex_);
-            process_state_.insert(std::pair<pid_t, ProcessState>(pid, ProcessState()));
+            lock_guard<std::mutex> lk(mutex_);
+            store_ = true;
+            process_collection_[this].insert(std::pair<pid_t, ProcessState>(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<std::mutex> lk(*mutex_);
-    ProcessStates::const_iterator proc = process_state_.find(pid);
-    if (proc == process_state_.end()) {
+    lock_guard<std::mutex> 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<std::mutex> lk(*mutex_);
-    for (ProcessStates::const_iterator proc = process_state_.begin();
-         proc != process_state_.end(); ++proc) {
-        if (proc->second.running_) {
-            return (true);
+    lock_guard<std::mutex> 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<std::mutex> lk(*mutex_);
-    ProcessStates::const_iterator proc = process_state_.find(pid);
-    if (proc == process_state_.end()) {
+    lock_guard<std::mutex> 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<std::mutex> lk(*mutex_);
+    lock_guard<std::mutex> 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<std::mutex> lk(*mutex_);
-    process_state_.erase(pid);
+    lock_guard<std::mutex> 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());
index ef37174902d52bfbc9981ecdb6db8cfa3199feae..e8dd805d3ada22207e40083b6f2f3f259930edd2 100644 (file)
@@ -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;