enh(ProcessRunner): does not detect launch errors #4482

Foundation/include/Poco/ProcessRunner.h

@@ -121,6 +121,8 @@ class Foundation_API ProcessRunner: public Poco::Runnable
 	int runCount() const;
 		/// Returns the number of times the process has been executed.
 
+	const std::string& error() const;
+		/// Returns the error message.
 
 private:
 	static const Poco::ProcessHandle::PID INVALID_PID = -1;
@@ -141,10 +143,23 @@ class Foundation_API ProcessRunner: public Poco::Runnable
 		/// Process initialization completion is indicated by new pid in
 		/// the pid file. If pid file is not specified, there is no waiting.
 
-	void checkTimeout(const Poco::Stopwatch& sw, const std::string& msg);
+	void checkError();
 		/// If timeout is exceeded, throws TimeoutException with `msg`
 		/// message.
 
+	void checkTimeout(const std::string& msg);
+		/// If timeout is exceeded, throws TimeoutException with `msg`
+		/// message.
+
+	void checkStatus(const std::string& msg, bool tOut = true);
+		/// If there were andy errors during process start/stop,
+		/// throws RuntimeException with the error message;
+		/// otherwise, if tOut is true and timeout is exceeded, throws
+		/// TimeoutException with `msg` message.
+
+	void setError(const std::string& msg);
+		/// Sets the error message.
+
 	Poco::Thread _t;
 	std::string _cmd;
 	Args _args;
@@ -156,6 +171,8 @@ class Foundation_API ProcessRunner: public Poco::Runnable
 	std::atomic<bool> _started;
 	std::atomic<int> _rc;
 	std::atomic<int> _runCount;
+	Stopwatch _sw;
+	std::string _error;
 };
 
 
@@ -193,6 +210,18 @@ inline int ProcessRunner::runCount() const
 }
 
 
+inline void ProcessRunner::setError(const std::string& msg)
+{
+	_error = Poco::format("ProcessRunner(%s): %s", cmdLine(), msg);
+}
+
+
+inline const std::string& ProcessRunner::error() const
+{
+	return _error;
+}
+
+
 } // namespace Poco
 
 

Foundation/src/ProcessRunner.cpp

@@ -20,6 +20,7 @@
 #include "Poco/File.h"
 #include "Poco/Path.h"
 #include "Poco/String.h"
+#include "Poco/Error.h"
 #include <fstream>
 
 
@@ -104,15 +105,46 @@ std::string ProcessRunner::cmdLine() const
 
 void ProcessRunner::run()
 {
+	int errHandle = 0;
+	int errPID = 0;
+	int errRC = 0;
+
+	_error.clear();
+	_pid = INVALID_PID;
+	_pPH = nullptr;
+
 	ProcessHandle* pPH = nullptr;
 	try
 	{
 		_pPH = pPH = new ProcessHandle(Process::launch(_cmd, _args, _options));
+		errHandle = Error::last();
+
 		_pid = pPH->id();
+		errPID = Error::last();
+
 		_rc = pPH->wait();
+		errRC = Error::last();
+
+		if (errHandle || errPID || errRC || _rc != 0)
+		{
+			Poco::format(_error, "ProcessRunner::run() error; "
+				"handle=%d (%d:%s); pid=%d (%d:%s); return=%d (%d:%s)",
+				(pPH ? pPH->id() : 0), errHandle, Error::getMessage(errHandle),
+				_pid.load(), errPID, Error::getMessage(errPID),
+				_rc.load(), errRC, Error::getMessage(errRC));
+		}
+	}
+	catch (Poco::Exception& ex)
+	{
+		setError(ex.displayText());
+	}
+	catch (std::exception& ex)
+	{
+		setError(ex.what());
 	}
 	catch (...)
 	{
+		setError("Unknown exception"s);
 	}
 
 	_pid = INVALID_PID;
@@ -127,17 +159,17 @@ void ProcessRunner::stop()
 	if (_started)
 	{
 		PID pid;
-		Stopwatch sw; sw.start();
+		_sw.restart();
 		if (_pPH.exchange(nullptr) && ((pid = _pid.exchange(INVALID_PID))) != INVALID_PID)
 		{
 			while (Process::isRunning(pid))
 			{
 				if (pid > 0)
 				{
 					Process::requestTermination(pid);
-					checkTimeout(sw, "Waiting for process termination");
+					checkStatus("Waiting for process termination");
 				}
-				else throw Poco::IllegalStateException("Invalid PID, can;t terminate process");
+				else throw Poco::IllegalStateException("Invalid PID, can't terminate process");
 			}
 			_t.join();
 		}
@@ -150,19 +182,25 @@ void ProcessRunner::stop()
 				_pidFile.clear();
 				std::string msg;
 				Poco::format(msg, "Waiting for PID file (pidFile: '%s')", _pidFile);
-				sw.restart();
-				while (pidFile.exists())
-					checkTimeout(sw, msg);
+				_sw.restart();
+				while (pidFile.exists()) checkStatus(msg);
 			}
 		}
 	}
 	_started.store(false);
 }
 
 
-void ProcessRunner::checkTimeout(const Stopwatch& sw, const std::string& msg)
+void ProcessRunner::checkError()
+{
+	if (!_error.empty())
+		throw Poco::RuntimeException(_error);
+}
+
+
+void ProcessRunner::checkTimeout(const std::string& msg)
 {
-	if (sw.elapsedSeconds() > _timeout)
+	if (_sw.elapsedSeconds() > _timeout)
 	{
 		throw Poco::TimeoutException(
 			Poco::format("ProcessRunner::checkTimeout(): %s", msg));
@@ -171,6 +209,13 @@ void ProcessRunner::checkTimeout(const Stopwatch& sw, const std::string& msg)
 }
 
 
+void ProcessRunner::checkStatus(const std::string& msg, bool tOut)
+{
+	checkError();
+	if (tOut) checkTimeout(msg);
+}
+
+
 void ProcessRunner::start()
 {
 	if (!_started.exchange(true))
@@ -181,21 +226,21 @@ void ProcessRunner::start()
 
 		std::string msg;
 		Poco::format(msg, "Waiting for process to start (pidFile: '%s')", _pidFile);
-		Stopwatch sw; sw.start();
+		_sw.restart();
 
 		// wait for the process to be either running or completed by monitoring run counts.
-		while (!running() && prevRunCnt >= runCount()) checkTimeout(sw, msg);
+		while (!running() && prevRunCnt >= runCount()) checkStatus(msg);
 
 		// we could wait for the process handle != INVALID_PID,
 		// but if pidFile name was given, we should wait for
 		// the process to write it
 		if (!_pidFile.empty())
 		{
-			sw.restart();
+			_sw.restart();
 			// wait until process is fully initialized
 			File pidFile(_pidFile);
 			while (!pidFile.exists())
-				checkTimeout(sw, "waiting for PID file");
+				checkStatus(Poco::format("waiting for PID file '%s' creation.", _pidFile));
 
 			// verify that the file content is actually the process PID
 			FileInputStream fis(_pidFile);
@@ -205,12 +250,13 @@ void ProcessRunner::start()
 			while (fPID != pid())
 			{
 				fis.clear(); fis.seekg(0); fis >> fPID;
-				checkTimeout(sw, Poco::format("waiting for new PID (%s)", _pidFile));
+				checkStatus(Poco::format("waiting for new PID (%s)", _pidFile));
 			}
 		}
 	}
 	else
 		throw Poco::InvalidAccessException("start() called on started ProcessRunner");
+	checkStatus("", false);
 }
 
 

Foundation/testsuite/src/ProcessRunnerTest.cpp

@@ -111,11 +111,11 @@ void ProcessRunnerTest::testProcessRunner()
 		assertTrue (pr.cmdLine() == cmdLine(cmd, args));
 		assertFalse (pr.running());
 		pr.start();
-		
+
 		Stopwatch sw; sw.start();
 		while (!pr.running())
 			checkTimeout(sw, "Waiting for process to start", 1000, __LINE__);
-		
+
 		assertTrue (pr.running());
 		try
 		{
@@ -264,6 +264,42 @@ void ProcessRunnerTest::testProcessRunner()
 		}
 		assertTrue (!File(pidFile).exists());
 	}
+
+	// non-existent executable with no PID file created
+	{
+		std::string cmd = "nonexistent_123-xyz";
+		std::vector<std::string> args;
+		char c = Path::separator();
+		std::string pidFile = Poco::format("run%c%s.pid", c, name);
+		{
+			std::unique_ptr<ProcessRunner> pr;
+			try
+			{
+				pr.reset(new ProcessRunner(cmd, args));
+				fail("ProcessRunner should throw an exception.", __LINE__, __FILE__);
+			} catch(const Poco::RuntimeException& e) {}
+		}
+		assertTrue (!File(pidFile).exists());
+	}
+
+	// non-existent executable with PID file created
+	{
+		std::string cmd = "nonexistent_123-xyz";
+		std::vector<std::string> args;
+		char c = Path::separator();
+		std::string pidFile = Poco::format("run%c%s.pid", c, name);
+		args.push_back(std::string("-p=").append(pidFile));
+		{
+			std::unique_ptr<ProcessRunner> pr;
+			try
+			{
+				pr.reset(new ProcessRunner(cmd, args));
+				fail("ProcessRunner should throw an exception.", __LINE__, __FILE__);
+			} catch(const Poco::RuntimeException& e) {}
+		}
+		assertTrue (!File(pidFile).exists());
+	}
+
 #if defined(POCO_OS_FAMILY_UNIX)
 	// start process launching multiple threads
 	{