Skip to content

Commit

Permalink
pythongh-94026: Buffer regrtest worker stdout in temporary file (pyth…
Browse files Browse the repository at this point in the history
…onGH-94253)

Co-authored-by: Victor Stinner <[email protected]>
  • Loading branch information
tiran and vstinner authored Jun 29, 2022
1 parent 5150cbc commit 199ba23
Showing 1 changed file with 38 additions and 39 deletions.
77 changes: 38 additions & 39 deletions Lib/test/libregrtest/runtest_mp.py
Original file line number Diff line number Diff line change
Expand Up @@ -9,7 +9,7 @@
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
Expand Down Expand Up @@ -53,7 +53,7 @@ def parse_worker_args(worker_args) -> tuple[Namespace, str]:
return (ns, test_name)


def run_test_in_subprocess(testname: str, ns: Namespace, tmp_dir: str) -> subprocess.Popen:
def run_test_in_subprocess(testname: str, ns: Namespace, tmp_dir: str, stdout_fh: TextIO) -> subprocess.Popen:
ns_dict = vars(ns)
worker_args = (ns_dict, testname)
worker_args = json.dumps(worker_args)
Expand All @@ -75,18 +75,18 @@ def run_test_in_subprocess(testname: str, ns: Namespace, tmp_dir: str) -> subpro
# 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 = {'env': env}
kw = dict(
env=env,
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:
Expand Down Expand Up @@ -212,12 +212,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, tmp_dir: str) -> tuple[int, str, str]:
def _run_process(self, test_name: str, tmp_dir: 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, tmp_dir)
popen = run_test_in_subprocess(test_name, self.ns, tmp_dir, stdout_fh)

self._killed = False
self._popen = popen
Expand All @@ -234,10 +234,10 @@ def _run_process(self, test_name: str, tmp_dir: 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
Expand All @@ -252,17 +252,12 @@ def _run_process(self, test_name: str, tmp_dir: 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
Expand All @@ -272,23 +267,30 @@ def _run_process(self, test_name: str, tmp_dir: str) -> tuple[int, str, str]:
self.current_test_name = None

def _runtest(self, test_name: str) -> MultiprocessResult:
# Don't check for leaked temporary files and directories if Python is
# run on WASI. WASI don't pass environment variables like TMPDIR to
# worker processes.
if not support.is_wasi:
# 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.
tmp_dir = tempfile.mkdtemp(prefix="test_python_")
tmp_dir = os.path.abspath(tmp_dir)
try:
retcode, stdout = self._run_process(test_name, tmp_dir)
finally:
tmp_files = os.listdir(tmp_dir)
os_helper.rmtree(tmp_dir)
else:
retcode, stdout = self._run_process(test_name, None)
tmp_files = ()
if not support.is_wasi:
# Don't check for leaked temporary files and directories if Python is
# run on WASI. WASI don't pass environment variables like TMPDIR to
# worker processes.
tmp_dir = tempfile.mkdtemp(prefix="test_python_")
tmp_dir = os.path.abspath(tmp_dir)
try:
retcode = self._run_process(test_name, tmp_dir, stdout_fh)
finally:
tmp_files = os.listdir(tmp_dir)
os_helper.rmtree(tmp_dir)
else:
retcode = self._run_process(test_name, None, stdout_fh)
tmp_files = ()
stdout_fh.seek(0)
stdout = stdout_fh.read().strip()

if retcode is None:
return self.mp_result_error(Timeout(test_name), stdout)
Expand Down Expand Up @@ -343,9 +345,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:
Expand Down

0 comments on commit 199ba23

Please sign in to comment.