Bug 1433905 - [mozprocess] poll() always returns None for stopped process until wait() is called.
If the process quit externally (shutdown by itself or via kill),
the poll method still returns None even the process doesn't exist
anymore.
To fix that the poll method should at least try to join the reader
thread once before checking if it is still alive. Otherwise it will
continue to run, and never stop.
Also the attribute existence check for "returncode" on the process
instance has to be removed because this property always exists.
Instead a check for the "returncode" property of the ProcessHandler
class is necessary.
MozReview-Commit-ID: AxPTUMnwpaF
--- a/testing/mozbase/mozprocess/mozprocess/processhandler.py
+++ b/testing/mozbase/mozprocess/mozprocess/processhandler.py
@@ -812,26 +812,30 @@ falling back to not using job objects fo
"""Check if child process has terminated
Returns the current returncode value:
- None if the process hasn't terminated yet
- A negative number if the process was killed by signal N (Unix only)
- '0' if the process ended without failures
"""
+ if not hasattr(self, 'proc'):
+ raise RuntimeError("Calling poll() on a non started process is not"
+ " allowed.")
+
# Ensure that we first check for the reader status. Otherwise
# we might mark the process as finished while output is still getting
# processed.
- if not hasattr(self, 'proc'):
- raise RuntimeError("Calling poll() on a non started process is not"
- " allowed.")
- elif self.reader.is_alive():
+ if self.reader.thread and self.reader.thread is not threading.current_thread():
+ self.reader.join(timeout=1)
+
+ if self.reader.is_alive():
return None
- elif hasattr(self.proc, "returncode"):
- return self.proc.returncode
+ elif hasattr(self, "returncode"):
+ return self.returncode
else:
return self.proc.poll()
def processOutput(self, timeout=None, outputTimeout=None):
"""
Handle process output until the process terminates or times out.
If timeout is not None, the process will be allowed to continue for
@@ -861,22 +865,22 @@ falling back to not using job objects fo
does not kill the process.
Returns the process exit code value:
- None if the process hasn't terminated yet
- A negative number if the process was killed by signal N (Unix only)
- '0' if the process ended without failures
"""
+ # Thread.join() blocks the main thread until the reader thread is finished
+ # wake up once a second in case a keyboard interrupt is sent
if self.reader.thread and self.reader.thread is not threading.current_thread():
- # Thread.join() blocks the main thread until the reader thread is finished
- # wake up once a second in case a keyboard interrupt is sent
count = 0
while self.reader.is_alive():
- self.reader.thread.join(timeout=1)
+ self.reader.join(timeout=1)
count += 1
if timeout is not None and count > timeout:
return None
self.returncode = self.proc.wait()
return self.returncode
# TODO Remove this method when consumers have been fixed
@@ -1061,16 +1065,21 @@ class ProcessReader(object):
if not timed_out:
self.finished_callback()
def is_alive(self):
if self.thread:
return self.thread.is_alive()
return False
+ def join(self, timeout=None):
+ if self.thread:
+ self.thread.join(timeout=timeout)
+
+
# default output handlers
# these should be callables that take the output line
class StoreOutput(object):
"""accumulate stdout"""
def __init__(self):
--- a/testing/mozbase/mozprocess/tests/test_poll.py
+++ b/testing/mozbase/mozprocess/tests/test_poll.py
@@ -104,25 +104,25 @@ class ProcTestPoll(proctest.ProcTest):
def test_poll_after_external_kill(self):
"""Process is killed externally, and poll() is called."""
p = processhandler.ProcessHandler([self.python, self.proclaunch,
"process_normal_finish.ini"],
cwd=here)
p.run()
os.kill(p.pid, signal.SIGTERM)
- returncode = p.wait()
+ returncode = p.poll()
# We killed the process, so the returncode should be non-zero
if mozinfo.isWin:
self.assertEqual(returncode, signal.SIGTERM,
'Positive returncode expected, got "%s"' % returncode)
else:
self.assertEqual(returncode, -signal.SIGTERM,
'%s expected, got "%s"' % (-signal.SIGTERM, returncode))
- self.assertEqual(returncode, p.poll())
+ self.assertEqual(returncode, p.wait())
self.determine_status(p)
if __name__ == '__main__':
mozunit.main()
--- a/testing/mozbase/mozprocess/tests/test_wait.py
+++ b/testing/mozbase/mozprocess/tests/test_wait.py
@@ -1,20 +1,22 @@
#!/usr/bin/env python
from __future__ import absolute_import
import os
-import proctest
+import signal
+
import mozinfo
-
import mozunit
from mozprocess import processhandler
+import proctest
+
here = os.path.dirname(os.path.abspath(__file__))
class ProcTestWait(proctest.ProcTest):
""" Class to test process waits and timeouts """
def test_normal_finish(self):
"""Process is started, runs to completion while we wait for it"""
@@ -96,11 +98,32 @@ class ProcTestWait(proctest.ProcTest):
self.assertGreater(returncode2, 0,
'Positive returncode expected, got "%s"' % returncode2)
else:
self.assertLess(returncode2, 0,
'Negative returncode expected, got "%s"' % returncode2)
self.assertEqual(returncode1, returncode2,
'Expected both returncodes of wait() to be equal')
+ def test_wait_after_external_kill(self):
+ """Process is killed externally, and poll() is called."""
+ p = processhandler.ProcessHandler([self.python, self.proclaunch,
+ "process_normal_finish.ini"],
+ cwd=here)
+ p.run()
+ os.kill(p.pid, signal.SIGTERM)
+ returncode = p.wait()
+
+ # We killed the process, so the returncode should be non-zero
+ if mozinfo.isWin:
+ self.assertEqual(returncode, signal.SIGTERM,
+ 'Positive returncode expected, got "%s"' % returncode)
+ else:
+ self.assertEqual(returncode, -signal.SIGTERM,
+ '%s expected, got "%s"' % (-signal.SIGTERM, returncode))
+
+ self.assertEqual(returncode, p.poll())
+
+ self.determine_status(p)
+
if __name__ == '__main__':
mozunit.main()