From 4706eb8fb1244ea232ac9d6d0bf405f3d9199d79 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?=D0=95=D0=B2=D0=B3=D0=B5=D0=BD=D0=B8=D0=B9=20=D0=91=D0=BB?= =?UTF-8?q?=D0=B8=D0=BD=D0=BE=D0=B2?= Date: Tue, 3 Nov 2026 04:50:46 +0300 Subject: [PATCH 1/3] Fix interrupted native library loading; release 0.0.8 --- Cargo.lock | 2 +- Cargo.toml | 2 +- README.md | 4 +- benchset/_native.py | 65 ++++- pyproject.toml | 2 +- tests/test_loader_failures.py | 12 +- tests/test_loader_signals.py | 489 ++++++++++++++++++++++++++++++++++ 7 files changed, 560 insertions(+), 16 deletions(-) create mode 100644 tests/test_loader_signals.py diff --git a/Cargo.lock b/Cargo.lock index 46f7181..781708c 100644 --- a/Cargo.lock +++ b/Cargo.lock @@ -4,4 +4,4 @@ version = 3 [[package]] name = "benchset" -version = "0.0.7" +version = "0.0.8" diff --git a/Cargo.toml b/Cargo.toml index fc9be10..f0decc3 100644 --- a/Cargo.toml +++ b/Cargo.toml @@ -1,7 +1,7 @@ [package] name = "benchset" # Generated from [project].version in pyproject.toml by scripts/version.py. -version = "0.0.7" +version = "0.0.8" edition = "2021" rust-version = "1.74" build = "native/build.rs" diff --git a/README.md b/README.md index b722360..8394702 100644 --- a/README.md +++ b/README.md @@ -442,7 +442,7 @@ pinned release toolchain described under [Publishing](#publishing). ```toml [dependencies] -benchset = "0.0.7" +benchset = "0.0.8" # For changes from a local source checkout, use: # benchset = { path = "../benchset" } ``` @@ -608,7 +608,7 @@ the same repository. Rust is not required by the consumer. For a Swift manifest, add: ```swift -.package(url: "https://github.com/mutating/benchset.git", .exact("0.0.7")) +.package(url: "https://github.com/mutating/benchset.git", .exact("0.0.8")) ``` Add `.product(name: "Benchset", package: "benchset")` to your target's diff --git a/benchset/_native.py b/benchset/_native.py index 20fb0fe..d04d9d0 100644 --- a/benchset/_native.py +++ b/benchset/_native.py @@ -2,11 +2,14 @@ import _ctypes import ctypes +import signal import sys from _ctypes import CFuncPtr from functools import lru_cache from os.path import abspath from pathlib import Path +from threading import current_thread, main_thread +from types import FrameType from typing import TYPE_CHECKING, Any, Callable, List, NoReturn, Optional, Tuple, cast from benchset._metadata import DATA, canonical_name @@ -37,7 +40,7 @@ def _load_library(self, name: str, mode: int, handle: int, winmode: Optional[int @lru_cache(maxsize=1) -def _load_library(path: str) -> _CDLL: +def _open_library(path: str) -> _CDLL: """Open before constructing ctypes types, rolling back handles that cannot be adopted.""" if _AsDouble is None: return cast(_CDLL, cast(Any, ctypes.cdll)._dlltype(path)) @@ -53,13 +56,65 @@ def _load_library(path: str) -> _CDLL: factory = cast(Any, ctypes.cdll)._dlltype return cast(_CDLL, (_Library if factory is _CDLL else factory)(path, handle=handle)) except BaseException: - if sys.platform == 'win32': - _ctypes.FreeLibrary(handle) - else: - _ctypes.dlclose(handle) + _close_library(handle) raise +def _close_library(handle: int) -> None: + if sys.platform == 'win32': + _ctypes.FreeLibrary(handle) + else: + _ctypes.dlclose(handle) + + +def _shared_library(path: str) -> _CDLL: + """Discard surplus opens from concurrent/reentrant misses for the fixed native path.""" + library = _open_library(path) + shared = _open_library(path) + if library is not shared and _AsDouble is not None: + _close_library(library._handle) + return shared + + +def _load_library(path: str) -> _CDLL: + """Defer Python SIGINT delivery until the opened image has a cached owner. + + A try/finally cannot protect a C call's return before Python assigns its + handle, or a return before lru_cache publishes its value. Keep the original + policy and originating frames, delivering signals after either publication + or rollback. Python dispatches signal handlers only on the main thread. + """ + handler = signal.getsignal(signal.SIGINT) + if current_thread() is not main_thread() or not callable(handler): + return _shared_library(path) + pending: List[Tuple[int, Optional[FrameType]]] = [] + + def defer(signum: int, frame: Optional[FrameType]) -> None: + pending.append((signum, frame)) + + try: + signal.signal(signal.SIGINT, defer) + except ValueError: # Subinterpreters cannot install or receive Python signal handlers. + return _shared_library(path) + try: + return _shared_library(path) + finally: + try: + signal.signal(signal.SIGINT, handler) + while pending: + arguments = pending.pop(0) + try: + callback = signal.getsignal(signal.SIGINT) + if callable(callback): + callback(*arguments) + elif callback != signal.SIG_IGN: + signal.raise_signal(signal.SIGINT) + finally: + del arguments + finally: + pending.clear() # Frames can refer back to this queue, even with cyclic GC disabled. + + class _Functions: """Own independent address-bound functions without per-call ctypes cycles.""" diff --git a/pyproject.toml b/pyproject.toml index 7b3fe35..e56c7ff 100644 --- a/pyproject.toml +++ b/pyproject.toml @@ -5,7 +5,7 @@ backend-path = ["."] [project] name = "benchset" -version = "0.0.7" +version = "0.0.8" authors = [ { name="Evgeniy Blinov", email="zheni-b@yandex.ru" }, ] diff --git a/tests/test_loader_failures.py b/tests/test_loader_failures.py index e0bfc79..8c81119 100644 --- a/tests/test_loader_failures.py +++ b/tests/test_loader_failures.py @@ -22,7 +22,7 @@ def _clear_standard_loader() -> None: - loader = getattr(_native, '_load_library', None) + loader = getattr(_native, '_open_library', None) if loader is not None: loader.cache_clear() @@ -90,7 +90,7 @@ def test_standard_loader_preflights_and_adopts_the_open_handle(monkeypatch: pyte secure Windows absolute-path defaults. This is a new private bridge check; HEAD had no cached loader helper. """ - loader = getattr(_native, '_load_library', None) + loader = getattr(_native, '_open_library', None) if loader is None: pytest.skip('HEAD has no standard loader helper') loader.cache_clear() @@ -176,7 +176,7 @@ def initialize(library: Any, name: str, *_args: Any, **kwargs: Any) -> None: requested = 'subdir/../adoption-probe.dll' expected_path = os.path.abspath(requested) if platform == 'win32' else requested # noqa: PTH100 — Match CDLL. expected_mode = 0x1100 if platform == 'win32' else ctypes.DEFAULT_MODE - library = _native._load_library.__wrapped__(requested) + library = _native._open_library.__wrapped__(requested) assert calls == [('open', expected_path, expected_mode, 701), ('wrapper', expected_path, 701)] assert library._handle == 701 assert library._name == expected_path @@ -224,7 +224,7 @@ def test_standard_loader_keeps_platform_modes_and_normalizes_windows_path( ctypes' legacy alias reproduces modern Windows: the raw platform opener must still work without changing ownership, paths or search flags. """ - loader = getattr(_native, '_load_library', None) + loader = getattr(_native, '_open_library', None) if loader is None: pytest.skip('HEAD has no standard loader helper') calls: List[Tuple[str, str, object]] = [] @@ -268,7 +268,7 @@ def test_failed_wrapper_adoption_closes_only_its_open_handle_and_retries( # noq own handle, preserve the original error and remain uncached. Recovery then adopts and caches its handle without closing the successful image. """ - loader = getattr(_native, '_load_library', None) + loader = getattr(_native, '_open_library', None) if loader is None: pytest.skip('HEAD has no pre-opened wrapper adoption helper') loader.cache_clear() @@ -341,7 +341,7 @@ def test_failed_raw_open_never_constructs_or_closes_a_wrapper( _dlopen returned a real handle, while a recovered successful image remains open. Both close entry points are intercepted on every host. """ - loader = getattr(_native, '_load_library', None) + loader = getattr(_native, '_open_library', None) if loader is None: pytest.skip('HEAD has no pre-opened wrapper adoption helper') loader.cache_clear() diff --git a/tests/test_loader_signals.py b/tests/test_loader_signals.py new file mode 100644 index 0000000..4a7d024 --- /dev/null +++ b/tests/test_loader_signals.py @@ -0,0 +1,489 @@ +"""SIGINT must never abandon an opened native image between ownership transfers.""" + +import _ctypes +import ctypes +import os +import shutil +import signal +import subprocess +import sys +import weakref +from pathlib import Path +from threading import Barrier, Lock, Thread, current_thread, main_thread +from types import FrameType, SimpleNamespace +from typing import Any, List, Optional, cast + +import pytest + +from benchset import _native + +_ROOT = Path(__file__).resolve().parents[1] + + +@pytest.mark.parametrize('boundary', ['open_return', 'opened', 'adopt_return', 'cache_return']) +@pytest.mark.parametrize('policy', ['default', 'raising', 'returning']) +def test_sigint_during_loading_keeps_exactly_one_owned_handle( # noqa: PLR0915 — Trace all ownership boundaries with one handle ledger. + monkeypatch: pytest.MonkeyPatch, boundary: str, policy: str, +) -> None: + """Deliver real SIGINT before assignment, adoption and cache publication. + + A fake OS handle ledger avoids unloading the test runner's Rust library. + Interrupting any boundary must leave one usable cached owner, preserve + the original signal policy/exception, and never reopen or close that owner. + The returning handler also checks that delivery sees completed adoption. + """ + cached: Any = getattr(_native, '_open_library', _native._load_library) + cached.cache_clear() + raw_code = cached.__wrapped__.__code__ + opened: List[int] = [] + closed: List[int] = [] + deliveries: List[object] = [] + result = SimpleNamespace(_handle=701) + adopted = False + fired = False + error = KeyboardInterrupt('original handler exception') + cause = ValueError('original cause') + error.__cause__ = cause + previous = signal.getsignal(signal.SIGINT) + previous_trace = sys.gettrace() + + def handler(signum: int, _frame: Optional[FrameType]) -> None: + deliveries.append((signum, adopted, signal.getsignal(signal.SIGINT) is handler)) + if policy == 'raising': + raise error + + selected = signal.default_int_handler if policy == 'default' else handler + + def open_image(_path: str, _mode: int) -> int: + opened.append(701) + return 701 + + def construct(_path: str, *, handle: int) -> object: + nonlocal adopted + assert handle == 701 + adopted = True + return result + + def trace(frame: FrameType, event: str, _value: object) -> Any: + nonlocal fired + hit = ( + (boundary == 'open_return' and event == 'return' and frame.f_code is open_image.__code__) + or (boundary == 'opened' and event == 'line' and frame.f_code is raw_code + and 'handle' in frame.f_locals) + or (boundary == 'adopt_return' and event == 'return' and frame.f_code is construct.__code__) + or (boundary == 'cache_return' and event == 'return' and frame.f_code is raw_code) + ) + if hit and not fired: + fired = True + signal.raise_signal(signal.SIGINT) + return trace + + monkeypatch.setattr(ctypes, '_dlopen', open_image, raising=False) + monkeypatch.setattr(ctypes.cdll, '_dlltype', construct) + monkeypatch.setattr(_ctypes, 'dlclose', closed.append, raising=False) + monkeypatch.setattr(_ctypes, 'FreeLibrary', closed.append, raising=False) + signal.signal(signal.SIGINT, selected) + try: + sys.settrace(trace) + try: + if policy == 'returning': + assert _native._load_library('signal-probe') is result + else: + with pytest.raises(KeyboardInterrupt) as caught: + _native._load_library('signal-probe') + if policy == 'raising': + assert caught.value is error + assert caught.value.__cause__ is cause + finally: + sys.settrace(previous_trace) + assert fired, 'the intended ownership boundary was not exercised' + assert signal.getsignal(signal.SIGINT) is selected + assert cached.cache_info().currsize == 1, 'SIGINT abandoned an opened image outside the cache' + assert _native._load_library('signal-probe') is result + assert opened == [701], 'retry reopened an image whose first reference was abandoned' + assert closed == [], 'the cached image must remain usable' + if policy != 'default': + assert deliveries == [(signal.SIGINT, True, True)] + finally: + signal.signal(signal.SIGINT, previous) + cached.cache_clear() + + +def test_concurrent_loading_shares_one_owned_reference(monkeypatch: pytest.MonkeyPatch) -> None: + """Concurrent cold misses must release the redundant open without serializing dlopen. + + A barrier inside the opener models an audit hook waiting for another + loading thread. Holding a loader lock across this hook would deadlock. + Worker threads must never attempt to install Python signal handlers. + """ + cached: Any = getattr(_native, '_open_library', _native._load_library) + cached.cache_clear() + barrier = Barrier(2) + ledger_lock = Lock() + opened: List[int] = [] + closed: List[int] = [] + results: List[Any] = [] + errors: List[BaseException] = [] + installations: List[Thread] = [] + install = signal.signal + + def install_on_main(signum: int, handler: Any) -> Any: + installations.append(current_thread()) + return install(signum, handler) + + def open_image(_path: str, _mode: int) -> int: + with ledger_lock: + handle = 701 + len(opened) + opened.append(handle) + barrier.wait(timeout=5) + return handle + + def construct(_path: str, *, handle: int) -> object: + return SimpleNamespace(_handle=handle) + + def load() -> None: + try: + results.append(_native._load_library('concurrent-probe')) + except BaseException as error: # noqa: BLE001 — Propagate worker interruptions through the main assertion. + errors.append(error) + + monkeypatch.setattr(ctypes, '_dlopen', open_image, raising=False) + monkeypatch.setattr(ctypes.cdll, '_dlltype', construct) + monkeypatch.setattr(_ctypes, 'dlclose', closed.append, raising=False) + monkeypatch.setattr(_ctypes, 'FreeLibrary', closed.append, raising=False) + monkeypatch.setattr(signal, 'signal', install_on_main) + worker = Thread(target=load, daemon=True) + try: + worker.start() + load() + worker.join(timeout=5) + assert not worker.is_alive() + assert errors == [] + assert installations + assert all(thread is main_thread() for thread in installations) + assert len(results) == 2 + assert results[0] is results[1], 'concurrent callers retained separate raw references' + assert len(opened) == 2 + assert closed == [handle for handle in opened if handle != results[0]._handle] + finally: + cached.cache_clear() + + +@pytest.mark.parametrize('raising', [False, True]) +def test_deferred_signals_keep_frames_policies_and_release_temporaries( # noqa: PLR0915 — Verify delivery and frame release without an intervening GC. + monkeypatch: pytest.MonkeyPatch, raising: bool, +) -> None: + """Deferred delivery keeps each originating frame and honors handler replacements. + + With cyclic GC disabled, frame-local payloads must disappear immediately + after delivery, including unprocessed signals when a handler raises; + storing deferred frames in their own frame would leak. + """ + import gc # noqa: PLC0415 — Keep GC manipulation local to its regression. + + cached: Any = getattr(_native, '_open_library', _native._load_library) + cached.cache_clear() + delivered: List[object] = [] + payloads: List[Any] = [] + adopted = False + + class Payload: + pass + + def second(signum: int, frame: Optional[FrameType]) -> None: + assert frame is not None + delivered.append(('second', signum, frame.f_code is open_image.__code__, adopted)) + + def first(signum: int, frame: Optional[FrameType]) -> None: + assert frame is not None + delivered.append(('first', signum, frame.f_code is open_image.__code__, adopted)) + if raising: + raise KeyboardInterrupt + signal.signal(signal.SIGINT, second) + + def open_image(_path: str, _mode: int) -> int: + payload = Payload() + payloads.append(weakref.ref(payload)) + signal.raise_signal(signal.SIGINT) + signal.raise_signal(signal.SIGINT) + return 701 + + def construct(_path: str, *, handle: int) -> object: + nonlocal adopted + adopted = True + return SimpleNamespace(_handle=handle) + + monkeypatch.setattr(ctypes, '_dlopen', open_image, raising=False) + monkeypatch.setattr(ctypes.cdll, '_dlltype', construct) + previous = signal.signal(signal.SIGINT, first) + collecting = gc.isenabled() + gc.disable() + try: + if raising: + try: + _native._load_library('frames-probe') + except KeyboardInterrupt as error: + error.__traceback__ = None # Release the host-owned traceback before checking loader retention. + else: + pytest.fail('the handler exception was swallowed') + else: + _native._load_library('frames-probe') + expected = [('first', signal.SIGINT, True, True)] + if not raising: + expected.append(('second', signal.SIGINT, True, True)) + assert delivered == expected + assert signal.getsignal(signal.SIGINT) is (first if raising else second) + assert all(reference() is None for reference in payloads), 'deferred frames formed a reference cycle' + finally: + signal.signal(signal.SIGINT, previous) + cached.cache_clear() + if collecting: + gc.enable() + + +def test_shared_loading_leaves_non_cpython_lifetime_to_its_owner(monkeypatch: pytest.MonkeyPatch) -> None: + """PyPy's CDLL owns its rawffi handle; deduplication must not manually close it a second time.""" + private, shared = SimpleNamespace(_handle=701), SimpleNamespace(_handle=702) + owners = iter((private, shared)) + closed: List[int] = [] + monkeypatch.setattr(_native, '_AsDouble', None) + monkeypatch.setattr(_native, '_open_library', lambda _path: next(owners)) + monkeypatch.setattr(_ctypes, 'dlclose', closed.append, raising=False) + monkeypatch.setattr(_ctypes, 'FreeLibrary', closed.append, raising=False) + assert _native._shared_library('pypy-probe') is shared + assert closed == [] + + +@pytest.mark.parametrize('stage', ['open', 'adopt']) +@pytest.mark.parametrize('policy', ['default', 'custom', 'ignore', 'native']) +def test_loading_failure_restores_the_original_signal_policy( + monkeypatch: pytest.MonkeyPatch, stage: str, policy: str, +) -> None: + """Rollback preserves the exact exception and leaves callable, ignored and native policies intact.""" + cached: Any = getattr(_native, '_open_library', _native._load_library) + cached.cache_clear() + error = OSError('original loading failure') + closed: List[int] = [] + previous = signal.getsignal(signal.SIGINT) + selected: Any = {'default': signal.default_int_handler, 'custom': lambda *_args: None, + 'ignore': signal.SIG_IGN, 'native': None}[policy] + + def open_image(_path: str, _mode: int) -> int: + if stage == 'open': + raise error + return 701 + + def construct(_path: str, *, handle: int) -> object: + assert handle == 701 + raise error + + monkeypatch.setattr(ctypes, '_dlopen', open_image, raising=False) + monkeypatch.setattr(ctypes.cdll, '_dlltype', construct) + monkeypatch.setattr(_ctypes, 'dlclose', closed.append, raising=False) + monkeypatch.setattr(_ctypes, 'FreeLibrary', closed.append, raising=False) + if policy == 'native': + monkeypatch.setattr(signal, 'getsignal', lambda _signum: None) + else: + signal.signal(signal.SIGINT, selected) + try: + with pytest.raises(OSError, match='original loading failure') as caught: + _native._load_library('failure-probe') + assert caught.value is error + assert signal.getsignal(signal.SIGINT) is selected + assert closed == ([701] if stage == 'adopt' else []) + assert cached.cache_info().currsize == 0 + finally: + signal.signal(signal.SIGINT, previous) + cached.cache_clear() + + +def test_subinterpreter_loading_does_not_require_signal_registration(monkeypatch: pytest.MonkeyPatch) -> None: + """A subinterpreter's apparent main thread cannot install handlers but must still load normally.""" + result = object() + calls: List[str] = [] + + def unsupported(_signum: int, _handler: object) -> None: + raise ValueError('signal only works in main thread of the main interpreter') + + def shared(path: str) -> object: + calls.append(path) + return result + + monkeypatch.setattr(signal, 'signal', unsupported) + monkeypatch.setattr(_native, '_shared_library', shared) + assert _native._load_library('subinterpreter-probe') is result + assert calls == ['subinterpreter-probe'] + + +@pytest.mark.parametrize('replacement', [signal.SIG_IGN, signal.SIG_DFL]) +def test_deferred_delivery_honors_a_noncallable_replacement_policy( + monkeypatch: pytest.MonkeyPatch, replacement: int, +) -> None: + """A returning handler may ignore later signals or restore OS delivery, including deferred ones. + + Intercept OS redelivery so testing SIG_DFL cannot terminate the test runner. + """ + cached: Any = getattr(_native, '_open_library', _native._load_library) + cached.cache_clear() + redelivered: List[int] = [] + raise_signal = signal.raise_signal + + def handler(_signum: int, _frame: Optional[FrameType]) -> None: + signal.signal(signal.SIGINT, replacement) + + def open_image(_path: str, _mode: int) -> int: + raise_signal(signal.SIGINT) + raise_signal(signal.SIGINT) + monkeypatch.setattr(signal, 'raise_signal', redelivered.append) + return 701 + + def construct(_path: str, *, handle: int) -> object: + return SimpleNamespace(_handle=handle) + + monkeypatch.setattr(ctypes, '_dlopen', open_image, raising=False) + monkeypatch.setattr(ctypes.cdll, '_dlltype', construct) + previous = signal.signal(signal.SIGINT, handler) + try: + _native._load_library('replacement-probe') + assert signal.getsignal(signal.SIGINT) is replacement + assert redelivered == ([] if replacement == signal.SIG_IGN else [signal.SIGINT]) + finally: + signal.signal(signal.SIGINT, previous) + cached.cache_clear() + + +_REAL_LOADING = r''' +import _ctypes, ctypes, gc, os, signal, sys +from pathlib import Path +from benchset import _native +path, log, boundary = sys.argv[1:] +cached = getattr(_native, '_open_library', _native._load_library) +cached.cache_clear() +raw_code = cached.__wrapped__.__code__ +original_open = ctypes._dlopen +fired = False +def open_image(path, mode): + return original_open(path, mode) +ctypes._dlopen = open_image +def trace(frame, event, value): + global fired + hit = ((boundary == 'open_return' and event == 'return' and frame.f_code is open_image.__code__) + or (boundary == 'opened' and event == 'line' and frame.f_code is raw_code + and 'handle' in frame.f_locals) + or (boundary == 'cache_return' and event == 'return' and frame.f_code is raw_code)) + if hit and not fired: + fired = True + signal.raise_signal(signal.SIGINT) + return trace +os.environ['BENCHSET_LOADER_LOG'] = log +os.environ['BENCHSET_LOADER_INTERRUPT'] = '1' if boundary == 'constructor' else '0' +signal.signal(signal.SIGINT, signal.default_int_handler) +sys.settrace(trace) +try: + _native._load_library(path) +except KeyboardInterrupt: + pass +else: + raise AssertionError('SIGINT was swallowed') +finally: + sys.settrace(None) +os.environ['BENCHSET_LOADER_INTERRUPT'] = '0' +library = _native._load_library(path) +function = library.loader_probe +function.restype = ctypes.c_int +assert function() == 7, 'the surviving cache owner is unusable' +handle = library._handle +cached.cache_clear() +del function, library +gc.collect() +assert Path(log).read_text() == 'L', 'a live owner was unloaded' +_ctypes.dlclose(handle) # Release the one intentional process-lived cache reference. +assert Path(log).read_text() == 'LU', 'an interrupted load leaked an additional OS reference' +''' + + +@pytest.fixture(scope='module') +def unload_probe(tmp_path_factory: pytest.TempPathFactory) -> Path: + """Build a separate image whose constructor/destructor expose actual loader reference counts.""" + if os.name == 'nt': + pytest.skip('POSIX constructor/destructor probe; Windows ownership uses the intercepted OS ledger') + compiler = shutil.which('cc') or shutil.which('clang') or shutil.which('gcc') + if compiler is None: + pytest.skip('No C compiler for the real unloading probe') + directory = tmp_path_factory.mktemp('loader-signals') + source = directory / 'probe.c' + source.write_text('''#include +#include +#include +static void event(char value) { + FILE *log = fopen(getenv("BENCHSET_LOADER_LOG"), "a"); + if (!log) abort(); + fputc(value, log); fclose(log); +} +__attribute__((constructor)) static void loaded(void) { + event('L'); + if (getenv("BENCHSET_LOADER_INTERRUPT")[0] == '1') raise(SIGINT); +} +__attribute__((destructor)) static void unloaded(void) { event('U'); } +int loader_probe(void) { return 7; } +''') + image = directory / 'probe.so' + result = subprocess.run([compiler, '-shared', '-fPIC', str(source), '-o', str(image)], + capture_output=True, text=True, check=False) + assert result.returncode == 0, result.stderr + return image + + +@pytest.mark.parametrize('boundary', ['constructor', 'open_return', 'opened', 'cache_return']) +def test_real_sigint_does_not_leak_an_os_loader_reference( + unload_probe: Path, tmp_path: Path, boundary: str, +) -> None: + """After interruption and retry, exactly one dlclose must unload the independent image. + + Include SIGINT raised inside the actual C constructor, before dlopen can + return its handle. A subprocess safely isolates real signals and unloading. + """ + result = subprocess.run([sys.executable, '-c', _REAL_LOADING, str(unload_probe), + str(tmp_path / 'events'), boundary], cwd=_ROOT, + capture_output=True, text=True, timeout=15, check=False) + assert result.returncode == 0, result.stdout + result.stderr + + +def test_reentrant_loading_releases_the_surplus_reference(monkeypatch: pytest.MonkeyPatch) -> None: + """A loader audit hook may reenter loading; only the shared cache owner may survive. + + Both misses open the same image. A subsequent SIGINT must not strand the + outer, uncached OS reference, and the nested owner must not be closed. + """ + cached: Any = getattr(_native, '_open_library', _native._load_library) + cached.cache_clear() + opened: List[int] = [] + closed: List[int] = [] + nested: List[object] = [] + + def open_image(path: str, _mode: int) -> int: + handle = 701 + len(opened) + opened.append(handle) + if len(opened) == 1: + nested.append(_native._load_library(path)) + signal.raise_signal(signal.SIGINT) + return handle + + def construct(_path: str, *, handle: int) -> object: + return SimpleNamespace(_handle=handle) + + monkeypatch.setattr(ctypes, '_dlopen', open_image, raising=False) + monkeypatch.setattr(ctypes.cdll, '_dlltype', construct) + monkeypatch.setattr(_ctypes, 'dlclose', closed.append, raising=False) + monkeypatch.setattr(_ctypes, 'FreeLibrary', closed.append, raising=False) + previous = signal.signal(signal.SIGINT, signal.default_int_handler) + try: + with pytest.raises(KeyboardInterrupt): + _native._load_library('reentrant-probe') + assert opened == [701, 702] + assert closed == [701], 'only the redundant outer reference should be released' + assert _native._load_library('reentrant-probe') is nested[0] + assert cast(Any, nested[0])._handle == 702 + finally: + signal.signal(signal.SIGINT, previous) + cached.cache_clear() From 3d2175523acf8d6387137ff24047a7bebfbbb0c3 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?=D0=95=D0=B2=D0=B3=D0=B5=D0=BD=D0=B8=D0=B9=20=D0=91=D0=BB?= =?UTF-8?q?=D0=B8=D0=BD=D0=BE=D0=B2?= Date: Tue, 3 Nov 2026 04:58:09 +0300 Subject: [PATCH 2/3] Exercise reentrant native loading with canonical paths on every platform --- tests/test_loader_signals.py | 14 +++++++++++--- 1 file changed, 11 insertions(+), 3 deletions(-) diff --git a/tests/test_loader_signals.py b/tests/test_loader_signals.py index 4a7d024..cd227ce 100644 --- a/tests/test_loader_signals.py +++ b/tests/test_loader_signals.py @@ -449,19 +449,27 @@ def test_real_sigint_does_not_leak_an_os_loader_reference( assert result.returncode == 0, result.stdout + result.stderr -def test_reentrant_loading_releases_the_surplus_reference(monkeypatch: pytest.MonkeyPatch) -> None: +@pytest.mark.parametrize('platform', ['darwin', 'linux', 'win32']) +def test_reentrant_loading_releases_the_surplus_reference( + monkeypatch: pytest.MonkeyPatch, platform: str, +) -> None: """A loader audit hook may reenter loading; only the shared cache owner may survive. Both misses open the same image. A subsequent SIGINT must not strand the outer, uncached OS reference, and the nested owner must not be closed. + Use the absolute path supplied by Native on every platform: Windows + normalizes relative names before entering the opener's audit hook. """ cached: Any = getattr(_native, '_open_library', _native._load_library) cached.cache_clear() opened: List[int] = [] closed: List[int] = [] nested: List[object] = [] + requested = str(Path('reentrant-probe').resolve()) + monkeypatch.setattr(_native, 'sys', SimpleNamespace(platform=platform)) def open_image(path: str, _mode: int) -> int: + assert path == requested handle = 701 + len(opened) opened.append(handle) if len(opened) == 1: @@ -479,10 +487,10 @@ def construct(_path: str, *, handle: int) -> object: previous = signal.signal(signal.SIGINT, signal.default_int_handler) try: with pytest.raises(KeyboardInterrupt): - _native._load_library('reentrant-probe') + _native._load_library(requested) assert opened == [701, 702] assert closed == [701], 'only the redundant outer reference should be released' - assert _native._load_library('reentrant-probe') is nested[0] + assert _native._load_library(requested) is nested[0] assert cast(Any, nested[0])._handle == 702 finally: signal.signal(signal.SIGINT, previous) From ac424b658aebb492f828184ed6edb627e7aa0072 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?=D0=95=D0=B2=D0=B3=D0=B5=D0=BD=D0=B8=D0=B9=20=D0=91=D0=BB?= =?UTF-8?q?=D0=B8=D0=BD=D0=BE=D0=B2?= Date: Tue, 3 Nov 2026 12:42:41 +0300 Subject: [PATCH 3/3] Make signal fixtures deterministic across legacy Windows and starved CI workers --- tests/test_cli_loading_signals.py | 10 +++++++++- tests/test_loader_signals.py | 2 +- tests/test_suite_options.py | 28 +++++++++++++++++++++------- 3 files changed, 31 insertions(+), 9 deletions(-) diff --git a/tests/test_cli_loading_signals.py b/tests/test_cli_loading_signals.py index 09712e6..d7f28ec 100644 --- a/tests/test_cli_loading_signals.py +++ b/tests/test_cli_loading_signals.py @@ -28,6 +28,8 @@ def test_cli_sigint_during_native_loading_cancels_reports_and_releases_before_re makes a worker-side loader wait for observed cancellation without sleeps. Two calls on the same runner check restoration, joined workers and exact native handle release independently of Thread's own completion bookkeeping. + Acknowledge cancellation only after the runner's flag is committed, so a + free-threaded loader cannot resume between notification and cancellation. """ runner = BenchmarkRunner() cancelled = Event() @@ -80,9 +82,15 @@ def free(native: Native, handle: int, *arguments: Any) -> None: released.append(identity) original_free(native, handle, *arguments) + def acknowledge() -> None: + if not cancelled.is_set(): + committed = runner._active is not None and runner._active.cancelled + cancelled.set() # Always release the worker, including a broken fixture's assertion path. + assert committed, 'the loader was acknowledged before cancellation was committed' + def cancel() -> None: - cancelled.set() original_cancel() + acknowledge() class Worker(Thread): def __init__(self, *arguments: Any, **options: Any) -> None: diff --git a/tests/test_loader_signals.py b/tests/test_loader_signals.py index cd227ce..630ff62 100644 --- a/tests/test_loader_signals.py +++ b/tests/test_loader_signals.py @@ -465,7 +465,7 @@ def test_reentrant_loading_releases_the_surplus_reference( opened: List[int] = [] closed: List[int] = [] nested: List[object] = [] - requested = str(Path('reentrant-probe').resolve()) + requested = os.path.abspath('reentrant-probe') # noqa: PTH100 — Match ctypes on legacy Windows too. monkeypatch.setattr(_native, 'sys', SimpleNamespace(platform=platform)) def open_image(path: str, _mode: int) -> int: diff --git a/tests/test_suite_options.py b/tests/test_suite_options.py index 5b001ec..96dfae6 100644 --- a/tests/test_suite_options.py +++ b/tests/test_suite_options.py @@ -180,9 +180,12 @@ def test_second_child_conversion_exceptions_escape_after_cleanup_and_allow_reuse @pytest.mark.parametrize('window', ['warmup', 'measurement']) -@pytest.mark.parametrize('policy', ['default', 'handled', 'custom', 'ignored']) -def test_sigint_during_second_conversion_interrupts_caller_immediately( - monkeypatch: pytest.MonkeyPatch, window: str, policy: str, +@pytest.mark.parametrize(('policy', 'second_no_work'), [ + ('default', False), ('handled', False), ('custom', False), ('ignored', False), + ('handled', True), ('custom', True), ('ignored', True), +]) +def test_sigint_during_second_conversion_interrupts_caller_immediately( # noqa: PLR0915 — Keep signal identity, events and native cleanup in one lifetime. + monkeypatch: pytest.MonkeyPatch, window: str, policy: str, second_no_work: bool, ) -> None: """Legacy child conversions run under the caller's ordinary SIGINT policy. @@ -190,6 +193,8 @@ def test_sigint_during_second_conversion_interrupts_caller_immediately( its next statement or another conversion. The native callback catches the original KeyboardInterrupt until workers and handles are cleaned up, then returns that same exception without attaching its private transport error. + A nanosecond second window forces NoWork without changing conversion or + signal semantics, as can happen when a busy CI runner starts its workers. """ events: List[str] = [] raised: List[KeyboardInterrupt] = [] @@ -214,7 +219,7 @@ def custom_handler(_signum: int, _frame: object) -> None: runner = BenchmarkRunner() monkeypatch.setattr(api, 'BenchmarkRunner', lambda: runner) warmup = Seconds('warmup', (0.0, 0.0), events) - measurement = Seconds('measurement', (0.1, 0.1), events) + measurement = Seconds('measurement', (0.1, 1e-9 if second_no_work else 0.1), events) handler = custom_handler if policy == 'custom' else ( signal.SIG_IGN if policy == 'ignored' else signal.default_int_handler ) @@ -234,7 +239,16 @@ def custom_handler(_signum: int, _frame: object) -> None: except KeyboardInterrupt: pytest.fail('the caller handled SIGINT and the suite must continue') assert len(suite.results) == 2 - assert all(result.success for result in suite.results) + assert suite.results[0].success, suite + second = suite.results[1] + if second_no_work: + assert second.status == 'failed', second + if not second.success: + assert second.status == 'failed', second + assert second.reason == 'measurement window was too short to complete any work', second + assert second.iterations == 0 or second.elapsed_seconds == 0, second + assert second.score is None + assert second.pi_equivalent is None assert len(raised) == (1 if policy == 'handled' else 0) assert signal.getsignal(signal.SIGINT) is handler expected = ['warmup:1', 'measurement:1', 'start:single_core', 'completed:single_core', 'warmup:2'] @@ -246,9 +260,9 @@ def custom_handler(_signum: int, _frame: object) -> None: expected.append('after-signal') if window == 'warmup': expected.append('measurement:2') - expected.extend(['start:multi_core', 'completed:multi_core']) + expected.extend(['start:multi_core', second.status + ':multi_core']) assert events == expected - assert runner.run('single_core', warmup_seconds=0, measurement_seconds=0.1).success + assert_runner_reusable(runner) finally: signal.signal(signal.SIGINT, previous)