[#3025] re-enable ProcessSpawnTest.isRunningSync

2 files changed, 16 insertions, 24 deletions

diff --git a/src/lib/asiolink/process_spawn.cc b/src/lib/asiolink/process_spawn.cc
index 5ca79fb63f..2dc1d3a84e 100644
--- a/src/lib/asiolink/process_spawn.cc
+++ b/src/lib/asiolink/process_spawn.cc
@@ -1,4 +1,4 @@
-// Copyright (C) 2015-2023 Internet Systems Consortium, Inc. ("ISC")
+// Copyright (C) 2015-2024 Internet Systems Consortium, Inc. ("ISC")
 //
 // This Source Code Form is subject to the terms of the Mozilla Public
 // License, v. 2.0. If a copy of the MPL was not distributed with this
@@ -208,7 +208,7 @@ private:
     /// @param pid the pid to wait for, -1 by default meaning wait
     /// for any child process
     /// @param sync whether this function is called immediately after spawning
-    /// (synchronous) or not (asynchronous, default).:w
+    /// (synchronous) or not (asynchronous, default).
     static void waitForProcess(int signum, pid_t const pid = -1, bool const sync = false);
 
     /// @brief A map holding the status codes of executed processes.
@@ -415,6 +415,8 @@ void
 ProcessSpawnImpl::waitForProcess(int /* signum */,
                                  pid_t const pid /* = -1 */,
                                  bool sync /* = false */) {
+    // In synchronous mode, the lock is taken by the caller function
+    // spawn(), so do not deadlock.
     unique_lock<std::mutex> lk{mutex_, std::defer_lock};
     if (!sync) {
         lk.lock();
@@ -459,8 +461,12 @@ ProcessSpawn::ProcessSpawn(IOServicePtr io_service,
                            const ProcessArgs& args,
                            const ProcessEnvVars& vars,
                            const bool inherit_env /* = false */)
-    : impl_(new ProcessSpawnImpl(
-          io_service, executable, args, vars, inherit_env, /* sync = */ false)) {
+    : impl_(new ProcessSpawnImpl(io_service,
+                                 executable,
+                                 args,
+                                 vars,
+                                 inherit_env,
+                                 /* sync = */ false)) {
 }
 
 ProcessSpawn::ProcessSpawn(const std::string& executable,
diff --git a/src/lib/asiolink/tests/process_spawn_unittest.cc b/src/lib/asiolink/tests/process_spawn_unittest.cc
index af3597e1c6..cad7e7b0dc 100644
--- a/src/lib/asiolink/tests/process_spawn_unittest.cc
+++ b/src/lib/asiolink/tests/process_spawn_unittest.cc
@@ -1,4 +1,4 @@
-// Copyright (C) 2015-2023 Internet Systems Consortium, Inc. ("ISC")
+// Copyright (C) 2015-2024 Internet Systems Consortium, Inc. ("ISC")
 //
 // This Source Code Form is subject to the terms of the Mozilla Public
 // License, v. 2.0. If a copy of the MPL was not distributed with this
@@ -529,11 +529,6 @@ TEST_F(ProcessSpawnTest, invalidExecutableSync) {
 // This test verifies that the full command line for the synchronous process is
 // returned.
 TEST_F(ProcessSpawnTest, getCommandLineSync) {
-    // Note that cases below are enclosed in separate scopes to make
-    // sure that the ProcessSpawn object is destroyed before a new
-    // object is created. Current implementation doesn't allow for
-    // having two ProcessSpawn objects simultaneously as they will
-    // both try to allocate a signal handler for SIGCHLD.
     {
         // Case 1: arguments present.
         ProcessArgs args;
@@ -554,23 +549,14 @@ TEST_F(ProcessSpawnTest, getCommandLineSync) {
     }
 }
 
-// This test verifies that it is possible to check if the synchronous process is
-// running.
-TEST_F(ProcessSpawnTest, DISABLED_isRunningSync) {
-    // Run the process which sleeps for 10 seconds, so as we have
-    // enough time to check if it is running.
-    vector<string> args;
-    args.push_back("-s");
-    args.push_back("10");
-
-    ProcessSpawn process(TEST_SCRIPT_SH, args);
+// This test verifies that the synchronous process reports as not running after
+// it was spawned.
+TEST_F(ProcessSpawnTest, isRunningSync) {
+    ProcessSpawn process(TEST_SCRIPT_SH);
     pid_t pid = 0;
     ASSERT_NO_THROW(pid = process.spawn());
 
-    EXPECT_TRUE(process.isRunning(pid));
-
-    // Kill the process.
-    ASSERT_EQ(0, kill(pid, SIGKILL));
+    EXPECT_FALSE(process.isRunning(pid));
 }
 
 // This test verifies inheritance of environment in a synchronous context.