From 5dea1bb9547e67c11972dc5600dc3a88bc79ff5f Mon Sep 17 00:00:00 2001 From: Christian Heimes Date: Wed, 29 Jun 2022 10:05:16 +0200 Subject: [PATCH] [3.11] gh-94026: Buffer regrtest worker stdout in temporary file (GH-94253) Co-authored-by: Victor Stinner . (cherry picked from commit 199ba233248ab279f445e0809c2077976f0711bc) Co-authored-by: Christian Heimes --- Lib/test/libregrtest/runtest_mp.py | 54 ++++++++++++++++-------------- 1 file changed, 28 insertions(+), 26 deletions(-) diff --git a/Lib/test/libregrtest/runtest_mp.py b/Lib/test/libregrtest/runtest_mp.py index 8f8c8dd918807b..13649ce10b3a41 100644 --- a/Lib/test/libregrtest/runtest_mp.py +++ b/Lib/test/libregrtest/runtest_mp.py @@ -5,10 +5,11 @@ import signal import subprocess import sys +import tempfile import threading import time import traceback -from typing import NamedTuple, NoReturn, Literal, Any +from typing import NamedTuple, NoReturn, Literal, Any, TextIO from test import support from test.support import os_helper @@ -51,7 +52,7 @@ def parse_worker_args(worker_args) -> tuple[Namespace, str]: return (ns, test_name) -def run_test_in_subprocess(testname: str, ns: Namespace) -> subprocess.Popen: +def run_test_in_subprocess(testname: str, ns: Namespace, stdout_fh: TextIO) -> subprocess.Popen: ns_dict = vars(ns) worker_args = (ns_dict, testname) worker_args = json.dumps(worker_args) @@ -67,18 +68,17 @@ def run_test_in_subprocess(testname: str, ns: Namespace) -> subprocess.Popen: # Running the child from the same working directory as regrtest's original # invocation ensures that TEMPDIR for the child is the same when # sysconfig.is_python_build() is true. See issue 15300. - kw = {} + kw = dict( + stdout=stdout_fh, + # bpo-45410: Write stderr into stdout to keep messages order + stderr=stdout_fh, + text=True, + close_fds=(os.name != 'nt'), + cwd=os_helper.SAVEDCWD, + ) if USE_PROCESS_GROUP: kw['start_new_session'] = True - return subprocess.Popen(cmd, - stdout=subprocess.PIPE, - # bpo-45410: Write stderr into stdout to keep - # messages order - stderr=subprocess.STDOUT, - universal_newlines=True, - close_fds=(os.name != 'nt'), - cwd=os_helper.SAVEDCWD, - **kw) + return subprocess.Popen(cmd, **kw) def run_tests_worker(ns: Namespace, test_name: str) -> NoReturn: @@ -204,12 +204,12 @@ def mp_result_error( test_result.duration_sec = time.monotonic() - self.start_time return MultiprocessResult(test_result, stdout, err_msg) - def _run_process(self, test_name: str) -> tuple[int, str, str]: + def _run_process(self, test_name: str, stdout_fh: TextIO) -> int: self.start_time = time.monotonic() self.current_test_name = test_name try: - popen = run_test_in_subprocess(test_name, self.ns) + popen = run_test_in_subprocess(test_name, self.ns, stdout_fh) self._killed = False self._popen = popen @@ -226,10 +226,10 @@ def _run_process(self, test_name: str) -> tuple[int, str, str]: raise ExitThread try: - # bpo-45410: stderr is written into stdout - stdout, _ = popen.communicate(timeout=self.timeout) - retcode = popen.returncode + # gh-94026: stdout+stderr are written to tempfile + retcode = popen.wait(timeout=self.timeout) assert retcode is not None + return retcode except subprocess.TimeoutExpired: if self._stopped: # kill() has been called: communicate() fails on reading @@ -244,17 +244,12 @@ def _run_process(self, test_name: str) -> tuple[int, str, str]: # bpo-38207: Don't attempt to call communicate() again: on it # can hang until all child processes using stdout # pipes completes. - stdout = '' except OSError: if self._stopped: # kill() has been called: communicate() fails # on reading closed stdout raise ExitThread raise - else: - stdout = stdout.strip() - - return (retcode, stdout) except: self._kill() raise @@ -264,7 +259,17 @@ def _run_process(self, test_name: str) -> tuple[int, str, str]: self.current_test_name = None def _runtest(self, test_name: str) -> MultiprocessResult: - retcode, stdout = self._run_process(test_name) + # gh-94026: Write stdout+stderr to a tempfile as workaround for + # non-blocking pipes on Emscripten with NodeJS. + with tempfile.TemporaryFile( + 'w+', encoding=sys.stdout.encoding + ) as stdout_fh: + # gh-93353: Check for leaked temporary files in the parent process, + # since the deletion of temporary files can happen late during + # Python finalization: too late for libregrtest. + retcode = self._run_process(test_name, stdout_fh) + stdout_fh.seek(0) + stdout = stdout_fh.read().strip() if retcode is None: return self.mp_result_error(Timeout(test_name), stdout) @@ -311,9 +316,6 @@ def run(self) -> None: def _wait_completed(self) -> None: popen = self._popen - # stdout must be closed to ensure that communicate() does not hang - popen.stdout.close() - try: popen.wait(JOIN_TIMEOUT) except (subprocess.TimeoutExpired, OSError) as exc: