Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

gh-94026: Buffer regrtest worker stdout in temporary file (GH-94253) #94253

Merged
merged 3 commits into from
Jun 29, 2022
Merged
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
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