From 179f7b61136a9bbedd08ec1ea210322b6a9436ec Mon Sep 17 00:00:00 2001 From: Brian Thorne Date: Tue, 25 Aug 2026 01:42:33 +1200 Subject: [PATCH 1/2] fix(bench): exit before finalizers in the Python benchmark runner (#434) MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit The 2026-07-15 python-nightly failed with exit 139. Every scenario in `--scenario workers` completed and emitted its JSON first; the process then died with SIGSEGV during interpreter finalization. This is the crash class #228 already characterised: pyo3-async-runtimes holds its tokio runtime in process-global state, that runtime's outstanding tasks hold references to Python objects, and finalization tears those objects down while the runtime may still touch them (PyO3/pyo3#1415). The mitigation #228 settled on — exit before finalizers run — lives in `tests/_subprocess_exit.py` and is used by both chaos subprocess helpers. `scripts/benchmark_runtime.py` is the only other `asyncio.run()` entry point in the package and never got it, which is why the same crash resurfaced in the nightly benchmark rather than the chaos helpers. `os._exit` does not flush stdio. Both existing helpers pass `flush=True` on every print so they are unaffected; this script passes it on none of its ten prints, and the nightly tees stdout into the artifact `check-bench-regression.py` reads. Verified by experiment: without the explicit flushes the piped run captures zero lines while still exiting 0, so the naive mitigation would have silently emptied the benchmark artifact. The shared helper gets the same flush so the next caller does not have to know this. The nightly benchmark step also sets PYTHONFAULTHANDLER=1 — this crash was diagnosable only from the shell's "Segmentation fault" line, with no Python frame. I was not able to reproduce the SIGSEGV locally (6 runs of the failing command pinned to 2 CPUs, plus targeted attempts at the shutdown-timeout and sequential-client paths), so this is a structural mitigation of a known upstream hazard rather than a verified-by-reproduction fix. --- .github/workflows/nightly-chaos.yml | 4 ++++ awa-python/scripts/benchmark_runtime.py | 14 ++++++++++++++ awa-python/tests/_subprocess_exit.py | 15 +++++++++++++++ 3 files changed, 33 insertions(+) diff --git a/.github/workflows/nightly-chaos.yml b/.github/workflows/nightly-chaos.yml index fea82073..f9c35261 100644 --- a/.github/workflows/nightly-chaos.yml +++ b/.github/workflows/nightly-chaos.yml @@ -340,6 +340,10 @@ jobs: " || true - name: Run Python worker benchmarks (sweep, jitter, rescue) timeout-minutes: 15 + env: + # A native crash here prints no Python frame on its own; #434 was + # diagnosable only from the shell's "Segmentation fault" line. + PYTHONFAULTHANDLER: "1" run: | set -o pipefail PYTHONPATH=scripts .venv/bin/python scripts/benchmark_runtime.py --scenario workers \ diff --git a/awa-python/scripts/benchmark_runtime.py b/awa-python/scripts/benchmark_runtime.py index d545edf0..74527001 100644 --- a/awa-python/scripts/benchmark_runtime.py +++ b/awa-python/scripts/benchmark_runtime.py @@ -3,6 +3,7 @@ import argparse import asyncio import os +import sys from dataclasses import dataclass from datetime import datetime, timedelta, timezone from statistics import quantiles @@ -1077,6 +1078,19 @@ def parse_args() -> argparse.Namespace: def main() -> None: asyncio.run(async_main(parse_args())) + # Exit before the interpreter finalizer phase: the pyo3-async-runtimes + # tokio runtime is process-global and its outstanding tasks hold Python + # references, so finalization can SIGSEGV after correct output (#434, + # #228, PyO3/pyo3#1415). `tests/_subprocess_exit.py` does the same for + # the chaos helpers. + # + # The flushes are load-bearing: `os._exit` skips stdio, this script does + # not pass `flush=True` per print, and the nightly tees stdout into the + # artifact the regression checker reads. + sys.stdout.flush() + sys.stderr.flush() + os._exit(0) + if __name__ == "__main__": main() diff --git a/awa-python/tests/_subprocess_exit.py b/awa-python/tests/_subprocess_exit.py index 452405e8..ba47d628 100644 --- a/awa-python/tests/_subprocess_exit.py +++ b/awa-python/tests/_subprocess_exit.py @@ -1,12 +1,26 @@ import asyncio import os import signal +import sys from collections.abc import Awaitable, Callable from types import FrameType from typing import NoReturn +def _flush_std_streams() -> None: + """`os._exit` skips stdio flushing, so callers that do not pass + `flush=True` on every print would lose buffered output when stdout is a + pipe. Both current callers do flush per print; this keeps the next one + from having to know that.""" + for stream in (sys.stdout, sys.stderr): + try: + stream.flush() + except Exception: + pass + + def _exit_from_signal(signum: int, _frame: FrameType | None) -> NoReturn: + _flush_std_streams() os._exit(128 + signum) @@ -18,4 +32,5 @@ def install_exit_without_finalizers_on_signals() -> None: def run_async_main_without_finalizers(main: Callable[[], Awaitable[None]]) -> NoReturn: install_exit_without_finalizers_on_signals() asyncio.run(main()) + _flush_std_streams() os._exit(0) From c9d49e1884f40e95e29ff03000675229a768ef28 Mon Sep 17 00:00:00 2001 From: Brian Thorne Date: Tue, 25 Aug 2026 03:07:46 +1200 Subject: [PATCH 2/2] fix(bench): tolerate a closed stream when flushing before _exit MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Review feedback on #472, and the first item is not cosmetic. The inline flushes were unguarded. When stdout is a closed pipe, flush() raises BrokenPipeError, the exception propagates out of main(), os._exit(0) is never reached, and the interpreter runs normal finalization — the exact crash path this change exists to avoid. Measured with the output piped into a reader that exits immediately: unguarded gives BrokenPipeError and exit 120 (Python's "failed to flush stdout on exit"), guarded gives exit 0. So a broken pipe silently disarmed the fix. Both flushes now tolerate OSError and ValueError, mirroring _flush_std_streams in tests/_subprocess_exit.py. Duplicated rather than imported because that module is outside this script's import path. Also corrects that helper's docstring: it claimed both callers flush per print, which stopped being true when benchmark_runtime.py started relying on a final flush for buffered output. It now states the actual contract. Verified the artifact is still intact through tee (complete output, exit 0) and test_subprocess_exit.py still passes. --- awa-python/scripts/benchmark_runtime.py | 26 +++++++++++++++++++------ awa-python/tests/_subprocess_exit.py | 11 +++++++---- 2 files changed, 27 insertions(+), 10 deletions(-) diff --git a/awa-python/scripts/benchmark_runtime.py b/awa-python/scripts/benchmark_runtime.py index 74527001..55bec18a 100644 --- a/awa-python/scripts/benchmark_runtime.py +++ b/awa-python/scripts/benchmark_runtime.py @@ -1075,6 +1075,25 @@ def parse_args() -> argparse.Namespace: return parser.parse_args() +def _flush_std_streams() -> None: + """Flush stdout and stderr, tolerating an already-closed stream. + + `os._exit` skips stdio, this script uses buffered output rather than + `flush=True` per print, and the nightly tees stdout into the artifact the + regression checker reads — so without this the artifact comes out empty. + + A raising flush would propagate out of `main` and hand control back to + normal interpreter finalization, which is the crash path the caller is + avoiding. Mirrors `_flush_std_streams` in `tests/_subprocess_exit.py`; + duplicated because that module lives outside this script's import path. + """ + for stream in (sys.stdout, sys.stderr): + try: + stream.flush() + except (OSError, ValueError): + pass + + def main() -> None: asyncio.run(async_main(parse_args())) @@ -1083,12 +1102,7 @@ def main() -> None: # references, so finalization can SIGSEGV after correct output (#434, # #228, PyO3/pyo3#1415). `tests/_subprocess_exit.py` does the same for # the chaos helpers. - # - # The flushes are load-bearing: `os._exit` skips stdio, this script does - # not pass `flush=True` per print, and the nightly tees stdout into the - # artifact the regression checker reads. - sys.stdout.flush() - sys.stderr.flush() + _flush_std_streams() os._exit(0) diff --git a/awa-python/tests/_subprocess_exit.py b/awa-python/tests/_subprocess_exit.py index ba47d628..3acedefb 100644 --- a/awa-python/tests/_subprocess_exit.py +++ b/awa-python/tests/_subprocess_exit.py @@ -8,10 +8,13 @@ def _flush_std_streams() -> None: - """`os._exit` skips stdio flushing, so callers that do not pass - `flush=True` on every print would lose buffered output when stdout is a - pipe. Both current callers do flush per print; this keeps the next one - from having to know that.""" + """Flush stdout and stderr before a forced exit, tolerating a closed stream. + + `os._exit` skips stdio, so callers may rely on this for their final + flush rather than passing `flush=True` on every print. A raising flush + is swallowed: letting it propagate would hand control back to normal + interpreter finalization, which is what the caller is avoiding. + """ for stream in (sys.stdout, sys.stderr): try: stream.flush()