enh(Process): Changes to detect defunct process more reliable.

matejk · matejk · commit 483d0a7b8df9 · 2026-03-15T18:01:58.000+01:00
diff --git a/Foundation/include/Poco/Process_UNIX.h b/Foundation/include/Poco/Process_UNIX.h
@@ -45,8 +45,10 @@ class Foundation_API ProcessHandleImpl: public RefCountedObject
 	int wait() const;
 	int wait(int options) const;
 	int tryWait() const;
+	bool isRunning() const;
 
 private:
+	static int statusToExitCode(int status);
 	const std::atomic<pid_t> _pid;
 	mutable Event _event;
 	mutable std::optional<std::atomic<int>> _status;
diff --git a/Foundation/src/Process_UNIX.cpp b/Foundation/src/Process_UNIX.cpp
@@ -57,12 +57,21 @@ pid_t ProcessHandleImpl::id() const
 }
 
 
+int ProcessHandleImpl::statusToExitCode(int status)
+{
+	if (WIFEXITED(status))
+		return WEXITSTATUS(status);
+	else
+		return -WTERMSIG(status);
+}
+
+
 int ProcessHandleImpl::wait() const
 {
 	if (wait(0) != _pid)
 		throw SystemException("Cannot wait for process", NumberFormatter::format(_pid));
 
-	return WEXITSTATUS(_status.value());
+	return statusToExitCode(_status.value());
 }
 
 
@@ -73,7 +82,7 @@ int ProcessHandleImpl::wait(int options) const
 	int rc;
 	do
 	{
-		rc = waitpid(_pid, &status, options);
+		rc = ::waitpid(_pid, &status, options);
 	}
 	while (rc < 0 && errno == EINTR);
 	if (rc == _pid)
@@ -83,10 +92,9 @@ int ProcessHandleImpl::wait(int options) const
 	}
 	else if (rc < 0 && errno == ECHILD)
 	{
-		// Looks like another thread was lucky and it should update the status for us shortly
-		_event.wait();
-
-		if (_status.has_value())
+		// 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())
 		{
 			rc = _pid;
 		}
@@ -98,21 +106,20 @@ int ProcessHandleImpl::wait(int options) const
 
 int ProcessHandleImpl::tryWait() const
 {
-	int status;
-	int rc;
-	do
-	{
-		rc = waitpid(_pid, &status, WNOHANG);
-	}
-	while (rc < 0 && errno == EINTR);
+	int rc = wait(WNOHANG);
 	if (rc == 0)
 		return -1;
 	if (rc != _pid)
 		throw SystemException("Cannot wait for process", NumberFormatter::format(_pid));
-	if (WIFEXITED(status)) // normal termination
-		return WEXITSTATUS(status);
-	else // termination by a signal
-		return 256 + WTERMSIG(status);
+	return statusToExitCode(_status.value());
+}
+
+
+bool ProcessHandleImpl::isRunning() const
+{
+	if (_status.has_value())
+		return false;
+	return wait(WNOHANG) == 0;
 }
 
 
@@ -121,14 +128,14 @@ int ProcessHandleImpl::tryWait() const
 //
 ProcessImpl::PIDImpl ProcessImpl::idImpl()
 {
-	return getpid();
+	return ::getpid();
 }
 
 
 void ProcessImpl::timesImpl(long& userTime, long& kernelTime)
 {
 	struct rusage usage;
-	getrusage(RUSAGE_SELF, &usage);
+	::getrusage(RUSAGE_SELF, &usage);
 	userTime   = usage.ru_utime.tv_sec;
 	kernelTime = usage.ru_stime.tv_sec;
 }
@@ -137,7 +144,7 @@ void ProcessImpl::timesImpl(long& userTime, long& kernelTime)
 void ProcessImpl::timesMicrosecondsImpl(Poco::Int64& userTime, Poco::Int64& kernelTime)
 {
 	struct rusage usage;
-	getrusage(RUSAGE_SELF, &usage);
+	::getrusage(RUSAGE_SELF, &usage);
 	userTime   = static_cast<Poco::Int64>(usage.ru_utime.tv_sec)*1000000 + usage.ru_utime.tv_usec;
 	kernelTime = static_cast<Poco::Int64>(usage.ru_stime.tv_sec)*1000000 + usage.ru_stime.tv_usec;
 }
@@ -186,17 +193,17 @@ ProcessHandleImpl* ProcessImpl::launchImpl(const std::string& command, const Arg
 			envPtr = &envPtrs[0];
 		}
 
-		int pid = spawn(command.c_str(), 3, fdmap, &inherit, argv, envPtr);
+		int pid = ::spawn(command.c_str(), 3, fdmap, &inherit, argv, envPtr);
 		delete [] argv;
 		if (pid == -1)
 			throw SystemException("cannot spawn", command);
 
 		if (inPipe)  inPipe->close(Pipe::CLOSE_READ);
-		if (options & PROCESS_CLOSE_STDIN) close(STDIN_FILENO);
+		if (options & PROCESS_CLOSE_STDIN) ::close(STDIN_FILENO);
 		if (outPipe) outPipe->close(Pipe::CLOSE_WRITE);
-		if (options & PROCESS_CLOSE_STDOUT) close(STDOUT_FILENO);
+		if (options & PROCESS_CLOSE_STDOUT) ::close(STDOUT_FILENO);
 		if (errPipe) errPipe->close(Pipe::CLOSE_WRITE);
-		if (options & PROCESS_CLOSE_STDERR) close(STDERR_FILENO);
+		if (options & PROCESS_CLOSE_STDERR) ::close(STDERR_FILENO);
 		return new ProcessHandleImpl(pid);
 	}
 	else
@@ -234,7 +241,7 @@ ProcessHandleImpl* ProcessImpl::launchByForkExecImpl(const std::string& command,
 
 		const char* pInitialDirectory = initialDirectory.empty() ? nullptr : initialDirectory.c_str();
 
-		int pid = fork();
+		int pid = ::fork();
 		if (pid < 0)
 		{
 			throw SystemException("Cannot fork process for", command);
@@ -243,7 +250,7 @@ ProcessHandleImpl* ProcessImpl::launchByForkExecImpl(const std::string& command,
 		{
 			if (pInitialDirectory)
 			{
-				if (chdir(pInitialDirectory) != 0)
+				if (::chdir(pInitialDirectory) != 0)
 				{
 					break;
 				}
@@ -253,33 +260,33 @@ ProcessHandleImpl* ProcessImpl::launchByForkExecImpl(const std::string& command,
 			char* p = &envChars[0];
 			while (*p)
 			{
-				putenv(p);
+				::putenv(p);
 				while (*p) ++p;
 				++p;
 			}
 
 			// setup redirection
 			if (inPipe)
 			{
-				dup2(inPipe->readHandle(), STDIN_FILENO);
+				::dup2(inPipe->readHandle(), STDIN_FILENO);
 				inPipe->close(Pipe::CLOSE_BOTH);
 			}
-			if (options & PROCESS_CLOSE_STDIN) close(STDIN_FILENO);
+			if (options & PROCESS_CLOSE_STDIN) ::close(STDIN_FILENO);
 
 			// outPipe and errPipe may be the same, so we dup first and close later
-			if (outPipe) dup2(outPipe->writeHandle(), STDOUT_FILENO);
-			if (errPipe) dup2(errPipe->writeHandle(), STDERR_FILENO);
+			if (outPipe) ::dup2(outPipe->writeHandle(), STDOUT_FILENO);
+			if (errPipe) ::dup2(errPipe->writeHandle(), STDERR_FILENO);
 			if (outPipe) outPipe->close(Pipe::CLOSE_BOTH);
-			if (options & PROCESS_CLOSE_STDOUT) close(STDOUT_FILENO);
+			if (options & PROCESS_CLOSE_STDOUT) ::close(STDOUT_FILENO);
 			if (errPipe) errPipe->close(Pipe::CLOSE_BOTH);
-			if (options & PROCESS_CLOSE_STDERR) close(STDERR_FILENO);
+			if (options & PROCESS_CLOSE_STDERR) ::close(STDERR_FILENO);
 			// close all open file descriptors other than stdin, stdout, stderr
-			long fdMax = sysconf(_SC_OPEN_MAX);
+			long fdMax = ::sysconf(_SC_OPEN_MAX);
 			// on some systems, sysconf(_SC_OPEN_MAX) returns a ridiculously high number
 			if (fdMax > CLOSE_FD_MAX) fdMax = CLOSE_FD_MAX;
 			for (long j = 3; j < fdMax; ++j)
 			{
-				close(j);
+				::close(j);
 			}
 
 			// Create a new process group so the entire tree can be signaled.
@@ -289,7 +296,7 @@ ProcessHandleImpl* ProcessImpl::launchByForkExecImpl(const std::string& command,
 					_exit(PROCESS_EXIT_SETPGID_FAILED);
 			}
 
-			execvp(argv[0], &argv[0]);
+			::execvp(argv[0], &argv[0]);
 			break;
 		}
 
@@ -322,7 +329,7 @@ void ProcessImpl::killImpl(ProcessHandleImpl& handle)
 
 void ProcessImpl::killImpl(PIDImpl pid)
 {
-	if (kill(pid, SIGKILL) != 0)
+	if (::kill(pid, SIGKILL) != 0)
 	{
 		switch (errno)
 		{
@@ -339,13 +346,13 @@ void ProcessImpl::killImpl(PIDImpl pid)
 
 bool ProcessImpl::isRunningImpl(const ProcessHandleImpl& handle)
 {
-	return isRunningImpl(handle.id());
+	return handle.isRunning();
 }
 
 
 bool ProcessImpl::isRunningImpl(PIDImpl pid)
 {
-	if (kill(pid, 0) == 0)
+	if (::kill(pid, 0) == 0)
 	{
 		return true;
 	}
@@ -358,7 +365,7 @@ bool ProcessImpl::isRunningImpl(PIDImpl pid)
 
 void ProcessImpl::requestTerminationImpl(PIDImpl pid)
 {
-	if (kill(pid, SIGINT) != 0)
+	if (::kill(pid, SIGINT) != 0)
 	{
 		switch (errno)
 		{
diff --git a/Foundation/testsuite/src/ProcessTest.cpp b/Foundation/testsuite/src/ProcessTest.cpp
@@ -19,6 +19,7 @@
 #include "Poco/Environment.h"
 #include "Poco/ProcessOptions.h"
 #include "Poco/Thread.h"
+#include <csignal>
 #include <iostream>
 
 

Original file line number	Diff line number	Diff line change
`@@ -57,12 +57,21 @@ pid_t ProcessHandleImpl::id() const`
`57`	`57`	`}`
`58`	`58`
`59`	`59`
	`60`	`+int ProcessHandleImpl::statusToExitCode(int status)`
	`61`	`+{`
	`62`	`+ if (WIFEXITED(status))`
	`63`	`+ return WEXITSTATUS(status);`
	`64`	`+ else`
	`65`	`+ return -WTERMSIG(status);`
	`66`	`+}`
	`67`	`+`
	`68`	`+`
`60`	`69`	`int ProcessHandleImpl::wait() const`
`61`	`70`	`{`
`62`	`71`	`if (wait(0) != _pid)`
`63`	`72`	`throw SystemException("Cannot wait for process", NumberFormatter::format(_pid));`
`64`	`73`
`65`		`- return WEXITSTATUS(_status.value());`
	`74`	`+ return statusToExitCode(_status.value());`
`66`	`75`	`}`
`67`	`76`
`68`	`77`
`@@ -73,7 +82,7 @@ int ProcessHandleImpl::wait(int options) const`
`73`	`82`	`int rc;`
`74`	`83`	`do`
`75`	`84`	`{`
`76`		`- rc = waitpid(_pid, &status, options);`
	`85`	`+ rc = ::waitpid(_pid, &status, options);`
`77`	`86`	`}`
`78`	`87`	`while (rc < 0 && errno == EINTR);`
`79`	`88`	`if (rc == _pid)`
`@@ -83,10 +92,9 @@ int ProcessHandleImpl::wait(int options) const`
`83`	`92`	`}`
`84`	`93`	`else if (rc < 0 && errno == ECHILD)`
`85`	`94`	`{`
`86`		`- // Looks like another thread was lucky and it should update the status for us shortly`
`87`		`- _event.wait();`
`88`		`-`
`89`		`- if (_status.has_value())`
	`95`	`+ // Another thread reaped the process; wait for it to update the status.`
	`96`	`+ // Use a timeout to prevent hanging indefinitely if something goes wrong.`
	`97`	`+ if (_event.tryWait(5000) && _status.has_value())`
`90`	`98`	`{`
`91`	`99`	`rc = _pid;`
`92`	`100`	`}`
`@@ -98,21 +106,20 @@ int ProcessHandleImpl::wait(int options) const`
`98`	`106`
`99`	`107`	`int ProcessHandleImpl::tryWait() const`
`100`	`108`	`{`
`101`		`- int status;`
`102`		`- int rc;`
`103`		`- do`
`104`		`- {`
`105`		`- rc = waitpid(_pid, &status, WNOHANG);`
`106`		`- }`
`107`		`- while (rc < 0 && errno == EINTR);`
	`109`	`+ int rc = wait(WNOHANG);`
`108`	`110`	`if (rc == 0)`
`109`	`111`	`return -1;`
`110`	`112`	`if (rc != _pid)`
`111`	`113`	`throw SystemException("Cannot wait for process", NumberFormatter::format(_pid));`
`112`		`- if (WIFEXITED(status)) // normal termination`
`113`		`- return WEXITSTATUS(status);`
`114`		`- else // termination by a signal`
`115`		`- return 256 + WTERMSIG(status);`
	`114`	`+ return statusToExitCode(_status.value());`
	`115`	`+}`
	`116`	`+`
	`117`	`+`
	`118`	`+bool ProcessHandleImpl::isRunning() const`
	`119`	`+{`
	`120`	`+ if (_status.has_value())`
	`121`	`+ return false;`
	`122`	`+ return wait(WNOHANG) == 0;`
`116`	`123`	`}`
`117`	`124`
`118`	`125`
`@@ -121,14 +128,14 @@ int ProcessHandleImpl::tryWait() const`
`121`	`128`	`//`
`122`	`129`	`ProcessImpl::PIDImpl ProcessImpl::idImpl()`
`123`	`130`	`{`
`124`		`- return getpid();`
	`131`	`+ return ::getpid();`
`125`	`132`	`}`
`126`	`133`
`127`	`134`
`128`	`135`	`void ProcessImpl::timesImpl(long& userTime, long& kernelTime)`
`129`	`136`	`{`
`130`	`137`	`struct rusage usage;`
`131`		`- getrusage(RUSAGE_SELF, &usage);`
	`138`	`+ ::getrusage(RUSAGE_SELF, &usage);`
`132`	`139`	`userTime = usage.ru_utime.tv_sec;`
`133`	`140`	`kernelTime = usage.ru_stime.tv_sec;`
`134`	`141`	`}`
`@@ -137,7 +144,7 @@ void ProcessImpl::timesImpl(long& userTime, long& kernelTime)`
`137`	`144`	`void ProcessImpl::timesMicrosecondsImpl(Poco::Int64& userTime, Poco::Int64& kernelTime)`
`138`	`145`	`{`
`139`	`146`	`struct rusage usage;`
`140`		`- getrusage(RUSAGE_SELF, &usage);`
	`147`	`+ ::getrusage(RUSAGE_SELF, &usage);`
`141`	`148`	`userTime = static_cast<Poco::Int64>(usage.ru_utime.tv_sec)*1000000 + usage.ru_utime.tv_usec;`
`142`	`149`	`kernelTime = static_cast<Poco::Int64>(usage.ru_stime.tv_sec)*1000000 + usage.ru_stime.tv_usec;`
`143`	`150`	`}`
`@@ -186,17 +193,17 @@ ProcessHandleImpl* ProcessImpl::launchImpl(const std::string& command, const Arg`
`186`	`193`	`envPtr = &envPtrs[0];`
`187`	`194`	`}`
`188`	`195`
`189`		`- int pid = spawn(command.c_str(), 3, fdmap, &inherit, argv, envPtr);`
	`196`	`+ int pid = ::spawn(command.c_str(), 3, fdmap, &inherit, argv, envPtr);`
`190`	`197`	`delete [] argv;`
`191`	`198`	`if (pid == -1)`
`192`	`199`	`throw SystemException("cannot spawn", command);`
`193`	`200`
`194`	`201`	`if (inPipe) inPipe->close(Pipe::CLOSE_READ);`
`195`		`- if (options & PROCESS_CLOSE_STDIN) close(STDIN_FILENO);`
	`202`	`+ if (options & PROCESS_CLOSE_STDIN) ::close(STDIN_FILENO);`
`196`	`203`	`if (outPipe) outPipe->close(Pipe::CLOSE_WRITE);`
`197`		`- if (options & PROCESS_CLOSE_STDOUT) close(STDOUT_FILENO);`
	`204`	`+ if (options & PROCESS_CLOSE_STDOUT) ::close(STDOUT_FILENO);`
`198`	`205`	`if (errPipe) errPipe->close(Pipe::CLOSE_WRITE);`
`199`		`- if (options & PROCESS_CLOSE_STDERR) close(STDERR_FILENO);`
	`206`	`+ if (options & PROCESS_CLOSE_STDERR) ::close(STDERR_FILENO);`
`200`	`207`	`return new ProcessHandleImpl(pid);`
`201`	`208`	`}`
`202`	`209`	`else`
`@@ -234,7 +241,7 @@ ProcessHandleImpl* ProcessImpl::launchByForkExecImpl(const std::string& command,`
`234`	`241`
`235`	`242`	`const char* pInitialDirectory = initialDirectory.empty() ? nullptr : initialDirectory.c_str();`
`236`	`243`
`237`		`- int pid = fork();`
	`244`	`+ int pid = ::fork();`
`238`	`245`	`if (pid < 0)`
`239`	`246`	`{`
`240`	`247`	`throw SystemException("Cannot fork process for", command);`
`@@ -243,7 +250,7 @@ ProcessHandleImpl* ProcessImpl::launchByForkExecImpl(const std::string& command,`
`243`	`250`	`{`
`244`	`251`	`if (pInitialDirectory)`
`245`	`252`	`{`
`246`		`- if (chdir(pInitialDirectory) != 0)`
	`253`	`+ if (::chdir(pInitialDirectory) != 0)`
`247`	`254`	`{`
`248`	`255`	`break;`
`249`	`256`	`}`
`@@ -253,33 +260,33 @@ ProcessHandleImpl* ProcessImpl::launchByForkExecImpl(const std::string& command,`
`253`	`260`	`char* p = &envChars[0];`
`254`	`261`	`while (*p)`
`255`	`262`	`{`
`256`		`- putenv(p);`
	`263`	`+ ::putenv(p);`
`257`	`264`	`while (*p) ++p;`
`258`	`265`	`++p;`
`259`	`266`	`}`
`260`	`267`
`261`	`268`	`// setup redirection`
`262`	`269`	`if (inPipe)`
`263`	`270`	`{`
`264`		`- dup2(inPipe->readHandle(), STDIN_FILENO);`
	`271`	`+ ::dup2(inPipe->readHandle(), STDIN_FILENO);`
`265`	`272`	`inPipe->close(Pipe::CLOSE_BOTH);`
`266`	`273`	`}`
`267`		`- if (options & PROCESS_CLOSE_STDIN) close(STDIN_FILENO);`
	`274`	`+ if (options & PROCESS_CLOSE_STDIN) ::close(STDIN_FILENO);`
`268`	`275`
`269`	`276`	`// outPipe and errPipe may be the same, so we dup first and close later`
`270`		`- if (outPipe) dup2(outPipe->writeHandle(), STDOUT_FILENO);`
`271`		`- if (errPipe) dup2(errPipe->writeHandle(), STDERR_FILENO);`
	`277`	`+ if (outPipe) ::dup2(outPipe->writeHandle(), STDOUT_FILENO);`
	`278`	`+ if (errPipe) ::dup2(errPipe->writeHandle(), STDERR_FILENO);`
`272`	`279`	`if (outPipe) outPipe->close(Pipe::CLOSE_BOTH);`
`273`		`- if (options & PROCESS_CLOSE_STDOUT) close(STDOUT_FILENO);`
	`280`	`+ if (options & PROCESS_CLOSE_STDOUT) ::close(STDOUT_FILENO);`
`274`	`281`	`if (errPipe) errPipe->close(Pipe::CLOSE_BOTH);`
`275`		`- if (options & PROCESS_CLOSE_STDERR) close(STDERR_FILENO);`
	`282`	`+ if (options & PROCESS_CLOSE_STDERR) ::close(STDERR_FILENO);`
`276`	`283`	`// close all open file descriptors other than stdin, stdout, stderr`
`277`		`- long fdMax = sysconf(_SC_OPEN_MAX);`
	`284`	`+ long fdMax = ::sysconf(_SC_OPEN_MAX);`
`278`	`285`	`// on some systems, sysconf(_SC_OPEN_MAX) returns a ridiculously high number`
`279`	`286`	`if (fdMax > CLOSE_FD_MAX) fdMax = CLOSE_FD_MAX;`
`280`	`287`	`for (long j = 3; j < fdMax; ++j)`
`281`	`288`	`{`
`282`		`- close(j);`
	`289`	`+ ::close(j);`
`283`	`290`	`}`
`284`	`291`
`285`	`292`	`// Create a new process group so the entire tree can be signaled.`
`@@ -289,7 +296,7 @@ ProcessHandleImpl* ProcessImpl::launchByForkExecImpl(const std::string& command,`
`289`	`296`	`_exit(PROCESS_EXIT_SETPGID_FAILED);`
`290`	`297`	`}`
`291`	`298`
`292`		`- execvp(argv[0], &argv[0]);`
	`299`	`+ ::execvp(argv[0], &argv[0]);`
`293`	`300`	`break;`
`294`	`301`	`}`
`295`	`302`
`@@ -322,7 +329,7 @@ void ProcessImpl::killImpl(ProcessHandleImpl& handle)`
`322`	`329`
`323`	`330`	`void ProcessImpl::killImpl(PIDImpl pid)`
`324`	`331`	`{`
`325`		`- if (kill(pid, SIGKILL) != 0)`
	`332`	`+ if (::kill(pid, SIGKILL) != 0)`
`326`	`333`	`{`
`327`	`334`	`switch (errno)`
`328`	`335`	`{`
`@@ -339,13 +346,13 @@ void ProcessImpl::killImpl(PIDImpl pid)`
`339`	`346`
`340`	`347`	`bool ProcessImpl::isRunningImpl(const ProcessHandleImpl& handle)`
`341`	`348`	`{`
`342`		`- return isRunningImpl(handle.id());`
	`349`	`+ return handle.isRunning();`
`343`	`350`	`}`
`344`	`351`
`345`	`352`
`346`	`353`	`bool ProcessImpl::isRunningImpl(PIDImpl pid)`
`347`	`354`	`{`
`348`		`- if (kill(pid, 0) == 0)`
	`355`	`+ if (::kill(pid, 0) == 0)`
`349`	`356`	`{`
`350`	`357`	`return true;`
`351`	`358`	`}`
`@@ -358,7 +365,7 @@ bool ProcessImpl::isRunningImpl(PIDImpl pid)`
`358`	`365`
`359`	`366`	`void ProcessImpl::requestTerminationImpl(PIDImpl pid)`
`360`	`367`	`{`
`361`		`- if (kill(pid, SIGINT) != 0)`
	`368`	`+ if (::kill(pid, SIGINT) != 0)`
`362`	`369`	`{`
`363`	`370`	`switch (errno)`
`364`	`371`	`{`