fix(Process): detect defunct processes reliably in isRunning #1097

- Replace optional<atomic<int>> with separate atomic status/flag
  for better portability across compilers
- Fix PID-only isRunningImpl to use waitpid(WNOHANG) before
  falling back to kill(pid, 0), properly detecting zombie children
- Fix missing debug suffix in test binary names
- Rewrite testIsRunningAllowsForTermination to reproduce the
  original defunct-with-pipes scenario from issue #1097
- Add tests: PID-only isRunning, wait-after-isRunning,
  concurrent wait/isRunning thread safety
- Remove unused Mutex.h and optional includes

Author: matejk

Foundation/include/Poco/Process_UNIX.h

@@ -20,13 +20,11 @@
 
 #include "Poco/Foundation.h"
 #include "Poco/Event.h"
-#include "Poco/Mutex.h"
 #include "Poco/RefCountedObject.h"
 #include <unistd.h>
 #include <vector>
 #include <map>
 #include <atomic>
-#include <optional>
 
 
 namespace Poco {
@@ -51,7 +49,8 @@ class Foundation_API ProcessHandleImpl: public RefCountedObject
 	static int statusToExitCode(int status);
 	const std::atomic<pid_t> _pid;
 	mutable Event _event;
-	mutable std::optional<std::atomic<int>> _status;
+	mutable std::atomic<int> _status{0};
+	mutable std::atomic<bool> _hasStatus{false};
 };
 
 

Foundation/src/Process_UNIX.cpp

@@ -71,13 +71,13 @@ int ProcessHandleImpl::wait() const
 	if (wait(0) != _pid)
 		throw SystemException("Cannot wait for process", NumberFormatter::format(_pid));
 
-	return statusToExitCode(_status.value());
+	return statusToExitCode(_status.load());
 }
 
 
 int ProcessHandleImpl::wait(int options) const
 {
-	if (_status.has_value()) return _pid;
+	if (_hasStatus.load()) return _pid;
 	int status;
 	int rc;
 	do
@@ -87,14 +87,15 @@ int ProcessHandleImpl::wait(int options) const
 	while (rc < 0 && errno == EINTR);
 	if (rc == _pid)
 	{
-		_status = status;
+		_status.store(status);
+		_hasStatus.store(true);
 		_event.set();
 	}
 	else if (rc < 0 && errno == ECHILD)
 	{
 		// Another thread reaped the process; wait for it to update the status.
 		// Use a timeout to prevent hanging indefinitely if something goes wrong.
-		if (_event.tryWait(5000) && _status.has_value())
+		if (_event.tryWait(5000) && _hasStatus.load())
 		{
 			rc = _pid;
 		}
@@ -111,13 +112,13 @@ int ProcessHandleImpl::tryWait() const
 		return -1;
 	if (rc != _pid)
 		throw SystemException("Cannot wait for process", NumberFormatter::format(_pid));
-	return statusToExitCode(_status.value());
+	return statusToExitCode(_status.load());
 }
 
 
 bool ProcessHandleImpl::isRunning() const
 {
-	if (_status.has_value())
+	if (_hasStatus.load())
 		return false;
 	return wait(WNOHANG) == 0;
 }
@@ -352,14 +353,14 @@ bool ProcessImpl::isRunningImpl(const ProcessHandleImpl& handle)
 
 bool ProcessImpl::isRunningImpl(PIDImpl pid)
 {
-	if (::kill(pid, 0) == 0)
-	{
-		return true;
-	}
-	else
-	{
+	int status;
+	int rc = ::waitpid(pid, &status, WNOHANG);
+	if (rc == pid)
 		return false;
-	}
+	if (rc == 0)
+		return true;
+	// Not our child or error; fall back to kill check
+	return ::kill(pid, 0) == 0;
 }
 
 

Foundation/testsuite/src/ProcessTest.cpp

@@ -19,6 +19,7 @@
 #include "Poco/Environment.h"
 #include "Poco/ProcessOptions.h"
 #include "Poco/Thread.h"
+#include <atomic>
 #include <csignal>
 #include <iostream>
 
@@ -359,29 +360,145 @@ void ProcessTest::testIsRunningAllowsForTermination()
 #if !defined(_WIN32_WCE)
 	std::string name("TestApp");
 	std::string cmd;
+#if defined(_DEBUG) && (POCO_OS != POCO_OS_ANDROID)
+	name += "d";
+#endif
 
 #if defined(POCO_OS_FAMILY_UNIX)
-	cmd = "./";
 	cmd += name;
 #else
 	cmd = name;
 #endif
 
+	// Launch with pipes to reproduce the original defunct process scenario (issue #1097).
+	// When a process launched with pipes exits, it becomes defunct until reaped.
 	std::vector<std::string> args;
-	ProcessHandle ph = Process::launch(cmd, args, nullptr, nullptr, nullptr);
+	args.push_back("-count");
+	Pipe inPipe;
+	ProcessHandle ph = Process::launch(cmd, args, &inPipe, nullptr, nullptr);
+	PipeOutputStream ostr(inPipe);
+	ostr << std::string(10, 'x');
+	ostr.close();
+	while (Process::isRunning(ph))
+		Thread::sleep(100);
+	// Process should be reaped by now; wait must not hang
+	int rc = ph.wait();
+	assertTrue (rc == 10);
+#endif // !defined(_WIN32_WCE)
+}
+
+
+void ProcessTest::testIsRunningByPidAllowsForTermination()
+{
+#if defined(POCO_OS_FAMILY_UNIX)
+	std::string name("TestApp");
+	std::string cmd;
+#if defined(_DEBUG) && (POCO_OS != POCO_OS_ANDROID)
+	name += "d";
+#endif
+
+	cmd += name;
+
+	// Test the PID-only isRunning overload with a child process
+	std::vector<std::string> args;
+	args.push_back("-count");
+	Pipe inPipe;
+	ProcessHandle ph = Process::launch(cmd, args, &inPipe, nullptr, nullptr);
+	Process::PID pid = ph.id();
+	PipeOutputStream ostr(inPipe);
+	ostr << std::string(10, 'x');
+	ostr.close();
+	while (Process::isRunning(pid))
+		Thread::sleep(100);
+#endif // defined(POCO_OS_FAMILY_UNIX)
+}
+
+
+void ProcessTest::testWaitAfterIsRunning()
+{
+#if !defined(_WIN32_WCE)
+	std::string name("TestApp");
+	std::string cmd;
+#if defined(_DEBUG) && (POCO_OS != POCO_OS_ANDROID)
+	name += "d";
+#endif
+
+#if defined(POCO_OS_FAMILY_UNIX)
+	cmd += name;
+#else
+	cmd = name;
+#endif
+
+	// Verify that calling wait() after isRunning() returns false works correctly.
+	// isRunning() reaps the process internally via waitpid; wait() must still succeed.
+	std::vector<std::string> args;
+	args.push_back("-count");
+	Pipe inPipe;
+	ProcessHandle ph = Process::launch(cmd, args, &inPipe, nullptr, nullptr);
+	PipeOutputStream ostr(inPipe);
+	ostr << std::string(42, 'x');
+	ostr.close();
 	while (Process::isRunning(ph))
 		Thread::sleep(100);
+	int rc = ph.wait();
+	assertTrue (rc == 42);
 #endif // !defined(_WIN32_WCE)
 }
 
 
+void ProcessTest::testConcurrentWaitAndIsRunning()
+{
+#if defined(POCO_OS_FAMILY_UNIX)
+	std::string name("TestApp");
+	std::string cmd;
+#if defined(_DEBUG) && (POCO_OS != POCO_OS_ANDROID)
+	name += "d";
+#endif
+
+	cmd += name;
+
+	// Test thread safety: one thread polls isRunning while another calls wait.
+	std::vector<std::string> args;
+	args.push_back("-count");
+	Pipe inPipe;
+	ProcessHandle ph = Process::launch(cmd, args, &inPipe, nullptr, nullptr);
+	PipeOutputStream ostr(inPipe);
+
+	std::atomic<int> waitResult{-1};
+	std::atomic<bool> waitDone{false};
+
+	Thread waiter;
+	waiter.startFunc([&ph, &waitResult, &waitDone]()
+	{
+		waitResult = ph.wait();
+		waitDone = true;
+	});
+
+	// Let the waiter thread start, then close the pipe to let the process exit
+	Thread::sleep(100);
+	ostr << std::string(7, 'x');
+	ostr.close();
+
+	// Poll isRunning concurrently with the waiter thread
+	while (Process::isRunning(ph))
+		Thread::sleep(50);
+
+	waiter.join();
+	assertTrue (waitDone);
+	assertTrue (waitResult == 7);
+#endif // defined(POCO_OS_FAMILY_UNIX)
+}
+
+
 void ProcessTest::testSignalExitCode()
 {
 #if defined(POCO_OS_FAMILY_UNIX)
 	std::string name("TestApp");
 	std::string cmd;
+#if defined(_DEBUG) && (POCO_OS != POCO_OS_ANDROID)
+	name += "d";
+#endif
 
-	cmd = "./";
 	cmd += name;
 
 	std::vector<std::string> args;
@@ -417,6 +534,9 @@ CppUnit::Test* ProcessTest::suite()
 	CppUnit_addTest(pSuite, ProcessTest, testIsRunning);
 	CppUnit_addTest(pSuite, ProcessTest, testLaunchCloseHandles);
 	CppUnit_addTest(pSuite, ProcessTest, testIsRunningAllowsForTermination);
+	CppUnit_addTest(pSuite, ProcessTest, testIsRunningByPidAllowsForTermination);
+	CppUnit_addTest(pSuite, ProcessTest, testWaitAfterIsRunning);
+	CppUnit_addTest(pSuite, ProcessTest, testConcurrentWaitAndIsRunning);
 	CppUnit_addTest(pSuite, ProcessTest, testSignalExitCode);
 
 	return pSuite;

Foundation/testsuite/src/ProcessTest.h

@@ -34,6 +34,9 @@ class ProcessTest: public CppUnit::TestCase
 	void testIsRunning();
 	void testLaunchCloseHandles();
 	void testIsRunningAllowsForTermination();
+	void testIsRunningByPidAllowsForTermination();
+	void testWaitAfterIsRunning();
+	void testConcurrentWaitAndIsRunning();
 	void testSignalExitCode();
 
 	void setUp();