From 57c050e57ec4f41981bfed480a5685075660662c Mon Sep 17 00:00:00 2001 From: Victor Stinner Date: Sat, 2 Sep 2023 18:51:09 +0200 Subject: [PATCH] Revert "[3.11] gh-94026: Buffer regrtest worker stdout in temporary file (GH-94253) (GH-94408)" This reverts commit 0122ab235b5acb52dd99fd05d8802a00f438b828. --- Lib/test/libregrtest/runtest_mp.py | 54 ++++++++++++++---------------- 1 file changed, 26 insertions(+), 28 deletions(-) diff --git a/Lib/test/libregrtest/runtest_mp.py b/Lib/test/libregrtest/runtest_mp.py index 13649ce10b3a41..8f8c8dd918807b 100644 --- a/Lib/test/libregrtest/runtest_mp.py +++ b/Lib/test/libregrtest/runtest_mp.py @@ -5,11 +5,10 @@ import signal import subprocess import sys -import tempfile import threading import time import traceback -from typing import NamedTuple, NoReturn, Literal, Any, TextIO +from typing import NamedTuple, NoReturn, Literal, Any from test import support from test.support import os_helper @@ -52,7 +51,7 @@ def parse_worker_args(worker_args) -> tuple[Namespace, str]: return (ns, test_name) -def run_test_in_subprocess(testname: str, ns: Namespace, stdout_fh: TextIO) -> subprocess.Popen: +def run_test_in_subprocess(testname: str, ns: Namespace) -> subprocess.Popen: ns_dict = vars(ns) worker_args = (ns_dict, testname) worker_args = json.dumps(worker_args) @@ -68,17 +67,18 @@ def run_test_in_subprocess(testname: str, ns: Namespace, stdout_fh: TextIO) -> s # 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 = 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, - ) + kw = {} if USE_PROCESS_GROUP: kw['start_new_session'] = True - return subprocess.Popen(cmd, **kw) + 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) 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, stdout_fh: TextIO) -> int: + def _run_process(self, test_name: str) -> tuple[int, str, str]: self.start_time = time.monotonic() self.current_test_name = test_name try: - popen = run_test_in_subprocess(test_name, self.ns, stdout_fh) + popen = run_test_in_subprocess(test_name, self.ns) self._killed = False self._popen = popen @@ -226,10 +226,10 @@ def _run_process(self, test_name: str, stdout_fh: TextIO) -> int: raise ExitThread try: - # gh-94026: stdout+stderr are written to tempfile - retcode = popen.wait(timeout=self.timeout) + # bpo-45410: stderr is written into stdout + stdout, _ = popen.communicate(timeout=self.timeout) + retcode = popen.returncode assert retcode is not None - return retcode except subprocess.TimeoutExpired: if self._stopped: # kill() has been called: communicate() fails on reading @@ -244,12 +244,17 @@ def _run_process(self, test_name: str, stdout_fh: TextIO) -> int: # 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 @@ -259,17 +264,7 @@ def _run_process(self, test_name: str, stdout_fh: TextIO) -> int: self.current_test_name = None def _runtest(self, test_name: str) -> MultiprocessResult: - # 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() + retcode, stdout = self._run_process(test_name) if retcode is None: return self.mp_result_error(Timeout(test_name), stdout) @@ -316,6 +311,9 @@ 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: