diff --git a/src/lib/asiolink/process_spawn.cc b/src/lib/asiolink/process_spawn.cc index 2dc1d3a84e..36c618c328 100644 --- a/src/lib/asiolink/process_spawn.cc +++ b/src/lib/asiolink/process_spawn.cc @@ -209,7 +209,7 @@ private: /// for any child process /// @param sync whether this function is called immediately after spawning /// (synchronous) or not (asynchronous, default). - static void waitForProcess(int signum, pid_t const pid = -1, bool const sync = false); + static void waitForProcess(int signum, pid_t const wpid = -1, bool const sync = false); /// @brief A map holding the status codes of executed processes. static ProcessCollection process_collection_; @@ -332,7 +332,9 @@ ProcessSpawnImpl::getCommandLine() const { pid_t ProcessSpawnImpl::spawn(bool dismiss) { lock_guard lk(mutex_); - ProcessSpawnImpl::IOSignalSetInitializer::initIOSignalSet(io_service_); + if (!sync_) { + ProcessSpawnImpl::IOSignalSetInitializer::initIOSignalSet(io_service_); + } // Create the child pid_t pid = fork(); if (pid < 0) { @@ -413,7 +415,7 @@ ProcessSpawnImpl::allocateInternal(const std::string& src) { void ProcessSpawnImpl::waitForProcess(int /* signum */, - pid_t const pid /* = -1 */, + pid_t const wpid /* = -1 */, bool sync /* = false */) { // In synchronous mode, the lock is taken by the caller function // spawn(), so do not deadlock. @@ -427,18 +429,37 @@ ProcessSpawnImpl::waitForProcess(int /* signum */, // When called asynchronously, the first IO context event is for // receiving the SIGCHLD signal which itself is blocking, // hence no need to block here too. - pid_t wpid = waitpid(pid, &status, sync ? 0 : WNOHANG); - if (wpid <= 0) { - break; - } - for (auto const& instance : process_collection_) { - auto const& proc = instance.second.find(wpid); - /// 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; + pid_t pid = waitpid(wpid, &status, sync ? 0 : WNOHANG); + if (pid < 0) { + if (!sync) { + break; + } + if (errno == EINTR) { + // On some systems that call sigaction wihout SA_RESTART by default, signals make + // waitpid exit with EINTR. In this situation, if sync mode is enabled, we're + // interested in another round of waitpid. + continue; + } + isc_throw(InvalidOperation, "process with pid " << wpid << " has returned " << pid + << " from waitpid in sync mode, errno: " + << strerror(errno)); + } else if (pid == 0) { + if (!sync) { + break; + } + } else /* if (pid > 0) */ { + for (auto const& instance : process_collection_) { + auto const& proc = instance.second.find(pid); + /// Check that the terminating process was started + /// by our instance of ProcessSpawn + if (proc != instance.second.end()) { + proc->second->status_ = status; + proc->second->running_ = false; + } + } + if (sync) { + // Collected process status. Can exit loop. + break; } } } diff --git a/src/lib/asiolink/tests/process_spawn_unittest.cc b/src/lib/asiolink/tests/process_spawn_unittest.cc index 7314447582..76706543f0 100644 --- a/src/lib/asiolink/tests/process_spawn_unittest.cc +++ b/src/lib/asiolink/tests/process_spawn_unittest.cc @@ -68,7 +68,11 @@ public: io_signal_set_->remove(SIGCHLD); io_signal_set_.reset(); // Make sure the cancel handler for the IOSignalSet is called. - io_service_->poll(); + io_service_->restart(); + try { + io_service_->poll(); + } catch (...) { + } } /// @brief Method used as the IOSignalSet handler. @@ -98,6 +102,7 @@ public: /// @brief Failsafe timer expiration handler. void testTimerHandler() { io_service_->stop(); + FAIL() << "Test Time: " << test_time_ms_ << " expired"; } }; @@ -119,8 +124,6 @@ TEST_F(ProcessSpawnTest, spawnWithArgs) { io_service_->runOne(); // waitForProcess handler. - io_service_->runOne(); - // ProcessSpawnTest::processSignal handler. io_service_->runOne(); @@ -155,8 +158,6 @@ TEST_F(ProcessSpawnTest, spawnWithArgsAndEnvVars) { io_service_->runOne(); // waitForProcess handler. - io_service_->runOne(); - // ProcessSpawnTest::processSignal handler. io_service_->runOne(); @@ -189,14 +190,15 @@ TEST_F(ProcessSpawnTest, spawnTwoProcesses) { io_service_->runOne(); // waitForProcess handler. - io_service_->runOne(); - // ProcessSpawnTest::processSignal handler. io_service_->runOne(); // Poll to be sure. io_service_->poll(); + ASSERT_EQ(1, processed_signals_.size()); + ASSERT_EQ(SIGCHLD, processed_signals_[0]); + pid_t pid2 = 0; ASSERT_NO_THROW(pid2 = process.spawn()); @@ -207,8 +209,6 @@ TEST_F(ProcessSpawnTest, spawnTwoProcesses) { io_service_->runOne(); // waitForProcess handler. - io_service_->runOne(); - // ProcessSpawnTest::processSignal handler. io_service_->runOne(); @@ -246,8 +246,6 @@ TEST_F(ProcessSpawnTest, spawnNoArgs) { io_service_->runOne(); // waitForProcess handler. - io_service_->runOne(); - // ProcessSpawnTest::processSignal handler. io_service_->runOne(); @@ -269,8 +267,6 @@ TEST_F(ProcessSpawnTest, spawnNoArgs) { io_service_->runOne(); // waitForProcess handler. - io_service_->runOne(); - // ProcessSpawnTest::processSignal handler. io_service_->runOne(); @@ -347,8 +343,6 @@ TEST_F(ProcessSpawnTest, isRunning) { io_service_->runOne(); // waitForProcess handler. - io_service_->runOne(); - // ProcessSpawnTest::processSignal handler. io_service_->runOne(); @@ -379,8 +373,6 @@ TEST_F(ProcessSpawnTest, inheritEnv) { io_service_->runOne(); // waitForProcess handler. - io_service_->runOne(); - // ProcessSpawnTest::processSignal handler. io_service_->runOne(); @@ -415,8 +407,6 @@ TEST_F(ProcessSpawnTest, inheritEnvWithParentVar) { io_service_->runOne(); // waitForProcess handler. - io_service_->runOne(); - // ProcessSpawnTest::processSignal handler. io_service_->runOne();