Allow for process termination when polling with isRunning

On *NIX, one needs to call `waitpid()` in order for process to exit the
zombie state. If one uses `Process::isRunning()` to emulate non-blocking
wait for child process termination, process will stay zombie and function
will always return true.

This commit changes `Process::isRunning()` to call `waitpid()` with
`WNOHANG` instead of using `kill()` when checking for child process (i.e.
the one we have ProcessHandle for), which allows for process termination.
Additional trickery with mutex and event is needed to prevent exceptions
when `Process::isRunning()` and/or `Process::wait()` is called concurrently
on the same handle from different threads.

Fixes #1097.

Foundation/include/Poco/Process_UNIX.h

@@ -21,6 +21,9 @@
 
 
 #include "Poco/Foundation.h"
+#include "Poco/Event.h"
+#include "Poco/Mutex.h"
+#include "Poco/Optional.h"
 #include "Poco/RefCountedObject.h"
 #include <unistd.h>
 #include <vector>
@@ -41,9 +44,13 @@ class Foundation_API ProcessHandleImpl: public RefCountedObject
 
 	pid_t id() const;
 	int wait() const;
+	int wait(int options) const;
 
 private:
-	pid_t _pid;
+	const pid_t _pid;
+	mutable FastMutex _mutex;
+	mutable Event _event;
+	mutable Optional<int> _status;
 };
 
 

Foundation/src/Process_UNIX.cpp

@@ -18,6 +18,7 @@
 #include "Poco/Exception.h"
 #include "Poco/NumberFormatter.h"
 #include "Poco/Pipe.h"
+#include "Poco/Thread.h"
 #include <limits>
 #include <errno.h>
 #include <signal.h>
@@ -42,7 +43,10 @@ namespace Poco {
 // ProcessHandleImpl
 //
 ProcessHandleImpl::ProcessHandleImpl(pid_t pid):
-	_pid(pid)
+	_pid(pid),
+	_mutex(),
+	_event(Event::EVENT_MANUALRESET),
+	_status()
 {
 }
 
@@ -60,24 +64,60 @@ pid_t ProcessHandleImpl::id() const
 
 int ProcessHandleImpl::wait() const
 {
-	int status;
-	int rc;
-	do
-	{
-		rc = waitpid(_pid, &status, 0);
-	}
-	while (rc < 0 && errno == EINTR);
-	if (rc != _pid)
+	if (wait(0) != _pid)
 		throw SystemException("Cannot wait for process", NumberFormatter::format(_pid));
+
+	const int status = _status.value();
 	if (WIFEXITED(status))
 		return WEXITSTATUS(status);
 	if (WIFSIGNALED(status))
 		return -WTERMSIG(status);
+
 	// This line should never be reached.
 	return std::numeric_limits<int>::max();
 }
 
 
+int ProcessHandleImpl::wait(int options) const
+{
+	{
+		FastMutex::ScopedLock lock(_mutex);
+		if (_status.isSpecified())
+		{
+			return _pid;
+		}
+	}
+
+	int status;
+	int rc;
+	do
+	{
+		rc = waitpid(_pid, &status, options);
+	}
+	while (rc < 0 && errno == EINTR);
+
+	if (rc == _pid)
+	{
+		FastMutex::ScopedLock lock(_mutex);
+		_status = status;
+		_event.set();
+	}
+	else if (rc < 0 && errno == ECHILD)
+	{
+		// Looks like another thread was lucky and it should update the status for us shortly
+		_event.wait();
+
+		FastMutex::ScopedLock lock(_mutex);
+		if (_status.isSpecified())
+		{
+			rc = _pid;
+		}
+	}
+
+	return rc;
+}
+
+
 //
 // ProcessImpl
 //
@@ -251,7 +291,7 @@ void ProcessImpl::killImpl(PIDImpl pid)
 
 bool ProcessImpl::isRunningImpl(const ProcessHandleImpl& handle)
 {
-	return isRunningImpl(handle.id());
+	return handle.wait(WNOHANG) == 0;
 }
 
 

Foundation/testsuite/src/ProcessTest.cpp

@@ -16,6 +16,7 @@
 #include "Poco/Process.h"
 #include "Poco/Pipe.h"
 #include "Poco/PipeStream.h"
+#include "Poco/Thread.h"
 #include <csignal>
 
 
@@ -176,6 +177,27 @@ void ProcessTest::testIsRunning()
 }
 
 
+void ProcessTest::testIsRunningAllowsForTermination()
+{
+#if !defined(_WIN32_WCE)
+	std::string name("TestApp");
+	std::string cmd;
+
+#if defined(POCO_OS_FAMILY_UNIX)
+	cmd = "./";
+	cmd += name;
+#else
+	cmd = name;
+#endif
+
+	std::vector<std::string> args;
+	ProcessHandle ph = Process::launch(cmd, args, 0, 0, 0);
+	while (Process::isRunning(ph))
+		Poco::Thread::sleep(100);
+#endif // !defined(_WIN32_WCE)
+}
+
+
 void ProcessTest::testSignalExitCode()
 {
 #if defined(POCO_OS_FAMILY_UNIX)
@@ -213,6 +235,7 @@ CppUnit::Test* ProcessTest::suite()
 	CppUnit_addTest(pSuite, ProcessTest, testLaunchRedirectOut);
 	CppUnit_addTest(pSuite, ProcessTest, testLaunchEnv);
 	CppUnit_addTest(pSuite, ProcessTest, testIsRunning);
+	CppUnit_addTest(pSuite, ProcessTest, testIsRunningAllowsForTermination);
 	CppUnit_addTest(pSuite, ProcessTest, testSignalExitCode);
 
 	return pSuite;

Foundation/testsuite/src/ProcessTest.h

@@ -31,6 +31,7 @@ class ProcessTest: public CppUnit::TestCase
 	void testLaunchRedirectOut();
 	void testLaunchEnv();
 	void testIsRunning();
+	void testIsRunningAllowsForTermination();
 	void testSignalExitCode();
 
 	void setUp();