diff --git a/Doc/library/test.rst b/Doc/library/test.rst index 54ad620d7dae9d..c33465d758d574 100644 --- a/Doc/library/test.rst +++ b/Doc/library/test.rst @@ -825,6 +825,21 @@ The :mod:`test.support` module defines the following functions: target of the "as" clause, if there is one. +.. function:: wait_process(pid, *, exitcode, timeout=None) + + Wait until process *pid* completes and check that the process exit code is + *exitcode*. + + Raise an :exc:`AssertionError` if the process exit code is not equal to + *exitcode*. + + If the process runs longer than *timeout* seconds (:data:`SHORT_TIMEOUT` by + default), kill the process and raise an :exc:`AssertionError`. The timeout + feature is not available on Windows. + + .. versionadded:: 3.9 + + .. function:: wait_threads_exit(timeout=60.0) Context manager to wait until all threads created in the ``with`` statement diff --git a/Lib/test/_test_multiprocessing.py b/Lib/test/_test_multiprocessing.py index 4a87b1761f9efe..d00e928c177905 100644 --- a/Lib/test/_test_multiprocessing.py +++ b/Lib/test/_test_multiprocessing.py @@ -5124,7 +5124,7 @@ def check_resource_tracker_death(self, signum, should_die): pid = _resource_tracker._pid if pid is not None: os.kill(pid, signal.SIGKILL) - os.waitpid(pid, 0) + support.wait_process(pid, exitcode=-signal.SIGKILL) with warnings.catch_warnings(): warnings.simplefilter("ignore") _resource_tracker.ensure_running() diff --git a/Lib/test/fork_wait.py b/Lib/test/fork_wait.py index f6bbffe1e7fdcd..8c177556a4e332 100644 --- a/Lib/test/fork_wait.py +++ b/Lib/test/fork_wait.py @@ -44,16 +44,7 @@ def f(self, id): pass def wait_impl(self, cpid): - for i in range(10): - # waitpid() shouldn't hang, but some of the buildbots seem to hang - # in the forking tests. This is an attempt to fix the problem. - spid, status = os.waitpid(cpid, os.WNOHANG) - if spid == cpid: - break - time.sleep(2 * SHORTSLEEP) - - self.assertEqual(spid, cpid) - self.assertEqual(status, 0, "cause = %d, exit = %d" % (status&0xff, status>>8)) + support.wait_process(cpid, exitcode=0) def test_wait(self): for i in range(NUM_THREADS): diff --git a/Lib/test/support/__init__.py b/Lib/test/support/__init__.py index 259c7069bfc0ac..5b9aebbda79719 100644 --- a/Lib/test/support/__init__.py +++ b/Lib/test/support/__init__.py @@ -3400,3 +3400,62 @@ def __exit__(self, *exc_info): del self.exc_value del self.exc_traceback del self.thread + + +def wait_process(pid, *, exitcode, timeout=None): + """ + Wait until process pid completes and check that the process exit code is + exitcode. + + Raise an AssertionError if the process exit code is not equal to exitcode. + + If the process runs longer than timeout seconds (SHORT_TIMEOUT by default), + kill the process (if signal.SIGKILL is available) and raise an + AssertionError. The timeout feature is not available on Windows. + """ + if os.name != "nt": + if timeout is None: + timeout = SHORT_TIMEOUT + t0 = time.monotonic() + deadline = t0 + timeout + sleep = 0.001 + max_sleep = 0.1 + while True: + pid2, status = os.waitpid(pid, os.WNOHANG) + if pid2 != 0: + break + # process is still running + + dt = time.monotonic() - t0 + if dt > SHORT_TIMEOUT: + try: + os.kill(pid, signal.SIGKILL) + os.waitpid(pid, 0) + except OSError: + # Ignore errors like ChildProcessError or PermissionError + pass + + raise AssertionError(f"process {pid} is still running " + f"after {dt:.1f} seconds") + + sleep = min(sleep * 2, max_sleep) + time.sleep(sleep) + + if os.WIFEXITED(status): + exitcode2 = os.WEXITSTATUS(status) + elif os.WIFSIGNALED(status): + exitcode2 = -os.WTERMSIG(status) + else: + raise ValueError(f"invalid wait status: {status!r}") + else: + # Windows implementation + pid2, status = os.waitpid(pid, 0) + exitcode2 = (status >> 8) + + if exitcode2 != exitcode: + raise AssertionError(f"process {pid} exited with code {exitcode2}, " + f"but exit code {exitcode} is expected") + + # sanity check: it should not fail in practice + if pid2 != pid: + raise AssertionError(f"pid {pid2} != pid {pid}") diff --git a/Lib/test/test_builtin.py b/Lib/test/test_builtin.py index 1e9012e7e5ed2e..86e5f1dc0246e0 100644 --- a/Lib/test/test_builtin.py +++ b/Lib/test/test_builtin.py @@ -25,6 +25,7 @@ from textwrap import dedent from types import AsyncGeneratorType, FunctionType from operator import neg +from test import support from test.support import ( EnvironmentVarGuard, TESTFN, check_warnings, swap_attr, unlink, maybe_get_event_loop_policy) @@ -1890,7 +1891,7 @@ def run_child(self, child, terminal_input): os.close(fd) # Wait until the child process completes - os.waitpid(pid, 0) + support.wait_process(pid, exitcode=0) return lines diff --git a/Lib/test/test_logging.py b/Lib/test/test_logging.py index e223522cc7eccc..2ad3c5c2085839 100644 --- a/Lib/test/test_logging.py +++ b/Lib/test/test_logging.py @@ -727,30 +727,19 @@ def lock_holder_thread_fn(): locks_held__ready_to_fork.wait() pid = os.fork() - if pid == 0: # Child. + if pid == 0: + # Child process try: test_logger.info(r'Child process did not deadlock. \o/') finally: os._exit(0) - else: # Parent. + else: + # Parent process test_logger.info(r'Parent process returned from fork. \o/') fork_happened__release_locks_and_end_thread.set() lock_holder_thread.join() - start_time = time.monotonic() - while True: - test_logger.debug('Waiting for child process.') - waited_pid, status = os.waitpid(pid, os.WNOHANG) - if waited_pid == pid: - break # child process exited. - if time.monotonic() - start_time > 7: - break # so long? implies child deadlock. - time.sleep(0.05) - test_logger.debug('Done waiting.') - if waited_pid != pid: - os.kill(pid, signal.SIGKILL) - waited_pid, status = os.waitpid(pid, 0) - self.fail("child process deadlocked.") - self.assertEqual(status, 0, msg="child process error") + + support.wait_process(pid, exitcode=0) class BadStream(object): diff --git a/Lib/test/test_mailbox.py b/Lib/test/test_mailbox.py index 36a265390e9855..fdda1d11d3307e 100644 --- a/Lib/test/test_mailbox.py +++ b/Lib/test/test_mailbox.py @@ -1092,7 +1092,7 @@ def test_lock_conflict(self): # Signal the child it can now release the lock and exit. p.send(b'p') # Wait for child to exit. Locking should now succeed. - exited_pid, status = os.waitpid(pid, 0) + support.wait_process(pid, exitcode=0) self._box.lock() self._box.unlock() diff --git a/Lib/test/test_os.py b/Lib/test/test_os.py index 9c965444f04b23..be85616ff88bf5 100644 --- a/Lib/test/test_os.py +++ b/Lib/test/test_os.py @@ -2792,8 +2792,7 @@ def test_waitpid(self): args = [sys.executable, '-c', 'pass'] # Add an implicit test for PyUnicode_FSConverter(). pid = os.spawnv(os.P_NOWAIT, FakePath(args[0]), args) - status = os.waitpid(pid, 0) - self.assertEqual(status, (pid, 0)) + support.wait_process(pid, exitcode=0) class SpawnTests(unittest.TestCase): @@ -2877,14 +2876,7 @@ def test_spawnvpe(self): def test_nowait(self): args = self.create_args() pid = os.spawnv(os.P_NOWAIT, args[0], args) - result = os.waitpid(pid, 0) - self.assertEqual(result[0], pid) - status = result[1] - if hasattr(os, 'WIFEXITED'): - self.assertTrue(os.WIFEXITED(status)) - self.assertEqual(os.WEXITSTATUS(status), self.exitcode) - else: - self.assertEqual(status, self.exitcode << 8) + support.wait_process(pid, exitcode=self.exitcode) @requires_os_func('spawnve') def test_spawnve_bytes(self): diff --git a/Lib/test/test_platform.py b/Lib/test/test_platform.py index 3084663a8fadd5..f167fb1e7b9bf1 100644 --- a/Lib/test/test_platform.py +++ b/Lib/test/test_platform.py @@ -236,9 +236,7 @@ def test_mac_ver_with_fork(self): else: # parent - cpid, sts = os.waitpid(pid, 0) - self.assertEqual(cpid, pid) - self.assertEqual(sts, 0) + support.wait_process(pid, exitcode=0) def test_libc_ver(self): # check that libc_ver(executable) doesn't raise an exception diff --git a/Lib/test/test_posix.py b/Lib/test/test_posix.py index fad26d88be2f3f..be121ae463dbb3 100644 --- a/Lib/test/test_posix.py +++ b/Lib/test/test_posix.py @@ -37,6 +37,7 @@ def _supports_sched(): requires_sched = unittest.skipUnless(_supports_sched(), 'requires POSIX scheduler API') + class PosixTester(unittest.TestCase): def setUp(self): @@ -180,7 +181,6 @@ def test_truncate(self): @unittest.skipUnless(getattr(os, 'execve', None) in os.supports_fd, "test needs execve() to support the fd parameter") @unittest.skipUnless(hasattr(os, 'fork'), "test needs os.fork()") - @unittest.skipUnless(hasattr(os, 'waitpid'), "test needs os.waitpid()") def test_fexecve(self): fp = os.open(sys.executable, os.O_RDONLY) try: @@ -189,7 +189,7 @@ def test_fexecve(self): os.chdir(os.path.split(sys.executable)[0]) posix.execve(fp, [sys.executable, '-c', 'pass'], os.environ) else: - self.assertEqual(os.waitpid(pid, 0), (pid, 0)) + support.wait_process(pid, exitcode=0) finally: os.close(fp) @@ -1539,7 +1539,7 @@ def test_returns_pid(self): """ args = self.python_args('-c', script) pid = self.spawn_func(args[0], args, os.environ) - self.assertEqual(os.waitpid(pid, 0), (pid, 0)) + support.wait_process(pid, exitcode=0) with open(pidfile) as f: self.assertEqual(f.read(), str(pid)) @@ -1569,7 +1569,7 @@ def test_specify_environment(self): args = self.python_args('-c', script) pid = self.spawn_func(args[0], args, {**os.environ, 'foo': 'bar'}) - self.assertEqual(os.waitpid(pid, 0), (pid, 0)) + support.wait_process(pid, exitcode=0) with open(envfile) as f: self.assertEqual(f.read(), 'bar') @@ -1580,7 +1580,7 @@ def test_none_file_actions(self): os.environ, file_actions=None ) - self.assertEqual(os.waitpid(pid, 0), (pid, 0)) + support.wait_process(pid, exitcode=0) def test_empty_file_actions(self): pid = self.spawn_func( @@ -1589,7 +1589,7 @@ def test_empty_file_actions(self): os.environ, file_actions=[] ) - self.assertEqual(os.waitpid(pid, 0), (pid, 0)) + support.wait_process(pid, exitcode=0) def test_resetids_explicit_default(self): pid = self.spawn_func( @@ -1598,7 +1598,7 @@ def test_resetids_explicit_default(self): os.environ, resetids=False ) - self.assertEqual(os.waitpid(pid, 0), (pid, 0)) + support.wait_process(pid, exitcode=0) def test_resetids(self): pid = self.spawn_func( @@ -1607,7 +1607,7 @@ def test_resetids(self): os.environ, resetids=True ) - self.assertEqual(os.waitpid(pid, 0), (pid, 0)) + support.wait_process(pid, exitcode=0) def test_resetids_wrong_type(self): with self.assertRaises(TypeError): @@ -1622,7 +1622,7 @@ def test_setpgroup(self): os.environ, setpgroup=os.getpgrp() ) - self.assertEqual(os.waitpid(pid, 0), (pid, 0)) + support.wait_process(pid, exitcode=0) def test_setpgroup_wrong_type(self): with self.assertRaises(TypeError): @@ -1643,7 +1643,7 @@ def test_setsigmask(self): os.environ, setsigmask=[signal.SIGUSR1] ) - self.assertEqual(os.waitpid(pid, 0), (pid, 0)) + support.wait_process(pid, exitcode=0) def test_setsigmask_wrong_type(self): with self.assertRaises(TypeError): @@ -1684,7 +1684,8 @@ def test_setsid(self): finally: os.close(wfd) - self.assertEqual(os.waitpid(pid, 0), (pid, 0)) + support.wait_process(pid, exitcode=0) + output = os.read(rfd, 100) child_sid = int(output) parent_sid = os.getsid(os.getpid()) @@ -1707,10 +1708,7 @@ def test_setsigdef(self): finally: signal.signal(signal.SIGUSR1, original_handler) - pid2, status = os.waitpid(pid, 0) - self.assertEqual(pid2, pid) - self.assertTrue(os.WIFSIGNALED(status), status) - self.assertEqual(os.WTERMSIG(status), signal.SIGUSR1) + support.wait_process(pid, exitcode=-signal.SIGUSR1) def test_setsigdef_wrong_type(self): with self.assertRaises(TypeError): @@ -1744,7 +1742,7 @@ def test_setscheduler_only_param(self): os.environ, scheduler=(None, os.sched_param(priority)) ) - self.assertEqual(os.waitpid(pid, 0), (pid, 0)) + support.wait_process(pid, exitcode=0) @requires_sched @unittest.skipIf(sys.platform.startswith(('freebsd', 'netbsd')), @@ -1764,7 +1762,7 @@ def test_setscheduler_with_policy(self): os.environ, scheduler=(policy, os.sched_param(priority)) ) - self.assertEqual(os.waitpid(pid, 0), (pid, 0)) + support.wait_process(pid, exitcode=0) def test_multiple_file_actions(self): file_actions = [ @@ -1776,7 +1774,7 @@ def test_multiple_file_actions(self): self.NOOP_PROGRAM, os.environ, file_actions=file_actions) - self.assertEqual(os.waitpid(pid, 0), (pid, 0)) + support.wait_process(pid, exitcode=0) def test_bad_file_actions(self): args = self.NOOP_PROGRAM @@ -1822,7 +1820,8 @@ def test_open_file(self): args = self.python_args('-c', script) pid = self.spawn_func(args[0], args, os.environ, file_actions=file_actions) - self.assertEqual(os.waitpid(pid, 0), (pid, 0)) + + support.wait_process(pid, exitcode=0) with open(outfile) as f: self.assertEqual(f.read(), 'hello') @@ -1840,7 +1839,8 @@ def test_close_file(self): args = self.python_args('-c', script) pid = self.spawn_func(args[0], args, os.environ, file_actions=[(os.POSIX_SPAWN_CLOSE, 0)]) - self.assertEqual(os.waitpid(pid, 0), (pid, 0)) + + support.wait_process(pid, exitcode=0) with open(closefile) as f: self.assertEqual(f.read(), 'is closed %d' % errno.EBADF) @@ -1858,7 +1858,7 @@ def test_dup2(self): args = self.python_args('-c', script) pid = self.spawn_func(args[0], args, os.environ, file_actions=file_actions) - self.assertEqual(os.waitpid(pid, 0), (pid, 0)) + support.wait_process(pid, exitcode=0) with open(dupfile) as f: self.assertEqual(f.read(), 'hello') @@ -1890,13 +1890,12 @@ def test_posix_spawnp(self): spawn_args = (program, '-I', '-S', '-c', 'pass') code = textwrap.dedent(""" import os + from test import support + args = %a pid = os.posix_spawnp(args[0], args, os.environ) - pid2, status = os.waitpid(pid, 0) - if pid2 != pid: - raise Exception(f"pid {pid2} != {pid}") - if status != 0: - raise Exception(f"status {status} != 0") + + support.wait_process(pid, exitcode=0) """ % (spawn_args,)) # Use a subprocess to test os.posix_spawnp() with a modified PATH diff --git a/Lib/test/test_random.py b/Lib/test/test_random.py index c147105376199a..548af706dbee22 100644 --- a/Lib/test/test_random.py +++ b/Lib/test/test_random.py @@ -1103,8 +1103,7 @@ def test_after_fork(self): child_val = eval(f.read()) self.assertNotEqual(val, child_val) - pid, status = os.waitpid(pid, 0) - self.assertEqual(status, 0) + support.wait_process(pid, exitcode=0) if __name__ == "__main__": diff --git a/Lib/test/test_socketserver.py b/Lib/test/test_socketserver.py index 85382a013136de..f818df0b22f6ad 100644 --- a/Lib/test/test_socketserver.py +++ b/Lib/test/test_socketserver.py @@ -65,9 +65,7 @@ def simple_subprocess(testcase): except: raise finally: - pid2, status = os.waitpid(pid, 0) - testcase.assertEqual(pid2, pid) - testcase.assertEqual(72 << 8, status) + test.support.wait_process(pid, exitcode=72) class SocketServerTest(unittest.TestCase): diff --git a/Lib/test/test_ssl.py b/Lib/test/test_ssl.py index 0093a49115d266..4184665b2b1581 100644 --- a/Lib/test/test_ssl.py +++ b/Lib/test/test_ssl.py @@ -408,8 +408,7 @@ def test_random_fork(self): else: os.close(wfd) self.addCleanup(os.close, rfd) - _, status = os.waitpid(pid, 0) - self.assertEqual(status, 0) + support.wait_process(pid, exitcode=0) child_random = os.read(rfd, 16) self.assertEqual(len(child_random), 16) diff --git a/Lib/test/test_subprocess.py b/Lib/test/test_subprocess.py index 868f2798394551..7cf31e1f0921dc 100644 --- a/Lib/test/test_subprocess.py +++ b/Lib/test/test_subprocess.py @@ -3114,12 +3114,10 @@ def test_stopped(self): proc = subprocess.Popen(args) # Wait until the real process completes to avoid zombie process - pid = proc.pid - pid, status = os.waitpid(pid, 0) - self.assertEqual(status, 0) + support.wait_process(proc.pid, exitcode=0) status = _testcapi.W_STOPCODE(3) - with mock.patch('subprocess.os.waitpid', return_value=(pid, status)): + with mock.patch('subprocess.os.waitpid', return_value=(proc.pid, status)): returncode = proc.wait() self.assertEqual(returncode, -3) @@ -3130,10 +3128,7 @@ def test_send_signal_race(self): proc = subprocess.Popen(ZERO_RETURN_CMD) # wait until the process completes without using the Popen APIs. - pid, status = os.waitpid(proc.pid, 0) - self.assertEqual(pid, proc.pid) - self.assertTrue(os.WIFEXITED(status), status) - self.assertEqual(os.WEXITSTATUS(status), 0) + support.wait_process(proc.pid, exitcode=0) # returncode is still None but the process completed. self.assertIsNone(proc.returncode) diff --git a/Lib/test/test_support.py b/Lib/test/test_support.py index 175f7c845fe8c6..99a4cad2bb8877 100644 --- a/Lib/test/test_support.py +++ b/Lib/test/test_support.py @@ -176,13 +176,10 @@ def test_temp_dir__forked_child(self): with support.temp_cwd() as temp_path: pid = os.fork() if pid != 0: - # parent process (child has pid == 0) + # parent process # wait for the child to terminate - (pid, status) = os.waitpid(pid, 0) - if status != 0: - raise AssertionError(f"Child process failed with exit " - f"status indication 0x{status:x}.") + support.wait_process(pid, exitcode=0) # Make sure that temp_path is still present. When the child # process leaves the 'temp_cwd'-context, the __exit__()- diff --git a/Lib/test/test_tempfile.py b/Lib/test/test_tempfile.py index 5fe9506b0b7ba1..524ab7c6d766ff 100644 --- a/Lib/test/test_tempfile.py +++ b/Lib/test/test_tempfile.py @@ -200,15 +200,7 @@ def test_process_awareness(self): child_value = os.read(read_fd, len(parent_value)).decode("ascii") finally: if pid: - # best effort to ensure the process can't bleed out - # via any bugs above - try: - os.kill(pid, signal.SIGKILL) - except OSError: - pass - - # Read the process exit status to avoid zombie process - os.waitpid(pid, 0) + support.wait_process(pid, exitcode=0) os.close(read_fd) os.close(write_fd) diff --git a/Lib/test/test_tracemalloc.py b/Lib/test/test_tracemalloc.py index 7283d9c31db4ee..635a9d39816058 100644 --- a/Lib/test/test_tracemalloc.py +++ b/Lib/test/test_tracemalloc.py @@ -314,10 +314,7 @@ def test_fork(self): finally: os._exit(exitcode) else: - pid2, status = os.waitpid(pid, 0) - self.assertTrue(os.WIFEXITED(status)) - exitcode = os.WEXITSTATUS(status) - self.assertEqual(exitcode, 0) + support.wait_process(pid, exitcode=0) class TestSnapshot(unittest.TestCase): diff --git a/Lib/test/test_uuid.py b/Lib/test/test_uuid.py index 27fc56d226c080..0b267f4a978613 100644 --- a/Lib/test/test_uuid.py +++ b/Lib/test/test_uuid.py @@ -657,7 +657,7 @@ def testIssue8621(self): os.close(fds[1]) self.addCleanup(os.close, fds[0]) parent_value = self.uuid.uuid4().hex - os.waitpid(pid, 0) + support.wait_process(pid, exitcode=0) child_value = os.read(fds[0], 100).decode('latin-1') self.assertNotEqual(parent_value, child_value) diff --git a/Misc/NEWS.d/next/Tests/2020-03-31-18-57-52.bpo-40094.m3fTJe.rst b/Misc/NEWS.d/next/Tests/2020-03-31-18-57-52.bpo-40094.m3fTJe.rst new file mode 100644 index 00000000000000..cae001bcb209e2 --- /dev/null +++ b/Misc/NEWS.d/next/Tests/2020-03-31-18-57-52.bpo-40094.m3fTJe.rst @@ -0,0 +1 @@ +Add :func:`test.support.wait_process` function.