From 5c14ddb0ce7f22e6b38f3dbcbb655fae06759002 Mon Sep 17 00:00:00 2001 From: EXPLOSION Date: Sat, 9 Nov 2024 01:43:11 -0500 Subject: [PATCH] Get rid of `signal_raise` (#3126) * Get rid of `signal_raise` * [pre-commit.ci] auto fixes from pre-commit.com hooks for more information, see https://pre-commit.ci --------- Co-authored-by: pre-commit-ci[bot] <66853113+pre-commit-ci[bot]@users.noreply.github.com> --- src/trio/_core/_tests/test_guest_mode.py | 7 ++- src/trio/_core/_tests/test_ki.py | 3 +- src/trio/_signals.py | 6 +-- src/trio/_tests/test_repl.py | 3 +- src/trio/_tests/test_signals.py | 29 ++++++------ src/trio/_tests/test_util.py | 16 ------- src/trio/_util.py | 56 ------------------------ 7 files changed, 22 insertions(+), 98 deletions(-) diff --git a/src/trio/_core/_tests/test_guest_mode.py b/src/trio/_core/_tests/test_guest_mode.py index 678874fec7..1aae101b0f 100644 --- a/src/trio/_core/_tests/test_guest_mode.py +++ b/src/trio/_core/_tests/test_guest_mode.py @@ -28,7 +28,6 @@ import trio.testing from trio.abc import Clock, Instrument -from ..._util import signal_raise from .tutil import gc_collect_harder, restore_unraisablehook if TYPE_CHECKING: @@ -605,10 +604,10 @@ def test_guest_mode_ki() -> None: # Check SIGINT in Trio func and in host func async def trio_main(in_host: InHost) -> None: with pytest.raises(KeyboardInterrupt): - signal_raise(signal.SIGINT) + signal.raise_signal(signal.SIGINT) # Host SIGINT should get injected into Trio - in_host(partial(signal_raise, signal.SIGINT)) + in_host(partial(signal.raise_signal, signal.SIGINT)) await trio.sleep(10) with pytest.raises(KeyboardInterrupt) as excinfo: @@ -621,7 +620,7 @@ async def trio_main(in_host: InHost) -> None: final_exc = KeyError("whoa") async def trio_main_raising(in_host: InHost) -> NoReturn: - in_host(partial(signal_raise, signal.SIGINT)) + in_host(partial(signal.raise_signal, signal.SIGINT)) raise final_exc with pytest.raises(KeyboardInterrupt) as excinfo: diff --git a/src/trio/_core/_tests/test_ki.py b/src/trio/_core/_tests/test_ki.py index 823c7aab09..67c83e8358 100644 --- a/src/trio/_core/_tests/test_ki.py +++ b/src/trio/_core/_tests/test_ki.py @@ -25,7 +25,6 @@ from ..._abc import Instrument from ..._core import _ki from ..._timeouts import sleep -from ..._util import signal_raise from ...testing import wait_all_tasks_blocked if TYPE_CHECKING: @@ -41,7 +40,7 @@ def ki_self() -> None: - signal_raise(signal.SIGINT) + signal.raise_signal(signal.SIGINT) def test_ki_self() -> None: diff --git a/src/trio/_signals.py b/src/trio/_signals.py index 63ee176c1b..729c48ad4e 100644 --- a/src/trio/_signals.py +++ b/src/trio/_signals.py @@ -7,7 +7,7 @@ import trio -from ._util import ConflictDetector, is_main_thread, signal_raise +from ._util import ConflictDetector, is_main_thread if TYPE_CHECKING: from collections.abc import AsyncIterator, Callable, Generator, Iterable @@ -78,7 +78,7 @@ def __init__(self) -> None: def _add(self, signum: int) -> None: if self._closed: - signal_raise(signum) + signal.raise_signal(signum) else: self._pending[signum] = None self._lot.unpark() @@ -95,7 +95,7 @@ def deliver_next() -> None: if self._pending: signum, _ = self._pending.popitem(last=False) try: - signal_raise(signum) + signal.raise_signal(signum) finally: deliver_next() diff --git a/src/trio/_tests/test_repl.py b/src/trio/_tests/test_repl.py index eba166d7d5..be9338ce4c 100644 --- a/src/trio/_tests/test_repl.py +++ b/src/trio/_tests/test_repl.py @@ -103,12 +103,11 @@ async def test_KI_interrupts( console = trio._repl.TrioInteractiveConsole(repl_locals=build_locals()) raw_input = build_raw_input( [ - "from trio._util import signal_raise", "import signal, trio, trio.lowlevel", "async def f():", " trio.lowlevel.spawn_system_task(" " trio.to_thread.run_sync," - " signal_raise,signal.SIGINT," + " signal.raise_signal, signal.SIGINT," " )", # just awaiting this kills the test runner?! " await trio.sleep_forever()", " print('should not see this')", diff --git a/src/trio/_tests/test_signals.py b/src/trio/_tests/test_signals.py index 453b1b68f2..d149b86575 100644 --- a/src/trio/_tests/test_signals.py +++ b/src/trio/_tests/test_signals.py @@ -10,7 +10,6 @@ from .. import _core from .._signals import _signal_handler, get_pending_signal_count, open_signal_receiver -from .._util import signal_raise if TYPE_CHECKING: from types import FrameType @@ -21,16 +20,16 @@ async def test_open_signal_receiver() -> None: with open_signal_receiver(signal.SIGILL) as receiver: # Raise it a few times, to exercise signal coalescing, both at the # call_soon level and at the SignalQueue level - signal_raise(signal.SIGILL) - signal_raise(signal.SIGILL) + signal.raise_signal(signal.SIGILL) + signal.raise_signal(signal.SIGILL) await _core.wait_all_tasks_blocked() - signal_raise(signal.SIGILL) + signal.raise_signal(signal.SIGILL) await _core.wait_all_tasks_blocked() async for signum in receiver: # pragma: no branch assert signum == signal.SIGILL break assert get_pending_signal_count(receiver) == 0 - signal_raise(signal.SIGILL) + signal.raise_signal(signal.SIGILL) async for signum in receiver: # pragma: no branch assert signum == signal.SIGILL break @@ -101,8 +100,8 @@ async def test_open_signal_receiver_no_starvation() -> None: print(signal.getsignal(signal.SIGILL)) previous = None for _ in range(10): - signal_raise(signal.SIGILL) - signal_raise(signal.SIGFPE) + signal.raise_signal(signal.SIGILL) + signal.raise_signal(signal.SIGFPE) await wait_run_sync_soon_idempotent_queue_barrier() if previous is None: previous = await receiver.__anext__() @@ -134,8 +133,8 @@ def direct_handler(signo: int, frame: FrameType | None) -> None: # before we exit the with block: with _signal_handler({signal.SIGILL, signal.SIGFPE}, direct_handler): with open_signal_receiver(signal.SIGILL, signal.SIGFPE) as receiver: - signal_raise(signal.SIGILL) - signal_raise(signal.SIGFPE) + signal.raise_signal(signal.SIGILL) + signal.raise_signal(signal.SIGFPE) await wait_run_sync_soon_idempotent_queue_barrier() assert delivered_directly == {signal.SIGILL, signal.SIGFPE} delivered_directly.clear() @@ -145,8 +144,8 @@ def direct_handler(signo: int, frame: FrameType | None) -> None: # we exit the with block: with _signal_handler({signal.SIGILL, signal.SIGFPE}, direct_handler): with open_signal_receiver(signal.SIGILL, signal.SIGFPE) as receiver: - signal_raise(signal.SIGILL) - signal_raise(signal.SIGFPE) + signal.raise_signal(signal.SIGILL) + signal.raise_signal(signal.SIGFPE) await wait_run_sync_soon_idempotent_queue_barrier() assert get_pending_signal_count(receiver) == 2 assert delivered_directly == {signal.SIGILL, signal.SIGFPE} @@ -157,14 +156,14 @@ def direct_handler(signo: int, frame: FrameType | None) -> None: print(3) with _signal_handler({signal.SIGILL}, signal.SIG_IGN): with open_signal_receiver(signal.SIGILL) as receiver: - signal_raise(signal.SIGILL) + signal.raise_signal(signal.SIGILL) await wait_run_sync_soon_idempotent_queue_barrier() # test passes if the process reaches this point without dying print(4) with _signal_handler({signal.SIGILL}, signal.SIG_IGN): with open_signal_receiver(signal.SIGILL) as receiver: - signal_raise(signal.SIGILL) + signal.raise_signal(signal.SIGILL) await wait_run_sync_soon_idempotent_queue_barrier() assert get_pending_signal_count(receiver) == 1 # test passes if the process reaches this point without dying @@ -177,8 +176,8 @@ def raise_handler(signum: int, frame: FrameType | None) -> NoReturn: with _signal_handler({signal.SIGILL, signal.SIGFPE}, raise_handler): with pytest.raises(RuntimeError) as excinfo: with open_signal_receiver(signal.SIGILL, signal.SIGFPE) as receiver: - signal_raise(signal.SIGILL) - signal_raise(signal.SIGFPE) + signal.raise_signal(signal.SIGILL) + signal.raise_signal(signal.SIGFPE) await wait_run_sync_soon_idempotent_queue_barrier() assert get_pending_signal_count(receiver) == 2 exc = excinfo.value diff --git a/src/trio/_tests/test_util.py b/src/trio/_tests/test_util.py index 23d16fe84c..5036d76e52 100644 --- a/src/trio/_tests/test_util.py +++ b/src/trio/_tests/test_util.py @@ -1,6 +1,5 @@ from __future__ import annotations -import signal import sys import types from typing import TYPE_CHECKING, TypeVar @@ -25,7 +24,6 @@ fixup_module_metadata, generic_function, is_main_thread, - signal_raise, ) from ..testing import wait_all_tasks_blocked @@ -35,20 +33,6 @@ T = TypeVar("T") -def test_signal_raise() -> None: - record = [] - - def handler(signum: int, _: object) -> None: - record.append(signum) - - old = signal.signal(signal.SIGFPE, handler) - try: - signal_raise(signal.SIGFPE) - finally: - signal.signal(signal.SIGFPE, old) - assert record == [signal.SIGFPE] - - async def test_ConflictDetector() -> None: ul1 = ConflictDetector("ul1") ul2 = ConflictDetector("ul2") diff --git a/src/trio/_util.py b/src/trio/_util.py index b354fbf5d0..994a4655b2 100644 --- a/src/trio/_util.py +++ b/src/trio/_util.py @@ -3,9 +3,7 @@ import collections.abc import inspect -import os import signal -import threading from abc import ABCMeta from collections.abc import Awaitable, Callable, Sequence from functools import update_wrapper @@ -36,60 +34,6 @@ PosArgsT = TypeVarTuple("PosArgsT") -if TYPE_CHECKING: - # Don't type check the implementation below, pthread_kill does not exist on Windows. - def signal_raise(signum: int) -> None: ... - - -# Equivalent to the C function raise(), which Python doesn't wrap -elif os.name == "nt": - # On Windows, os.kill exists but is really weird. - # - # If you give it CTRL_C_EVENT or CTRL_BREAK_EVENT, it tries to deliver - # those using GenerateConsoleCtrlEvent. But I found that when I tried - # to run my test normally, it would freeze waiting... unless I added - # print statements, in which case the test suddenly worked. So I guess - # these signals are only delivered if/when you access the console? I - # don't really know what was going on there. From reading the - # GenerateConsoleCtrlEvent docs I don't know how it worked at all. - # - # I later spent a bunch of time trying to make GenerateConsoleCtrlEvent - # work for creating synthetic control-C events, and... failed - # utterly. There are lots of details in the code and comments - # removed/added at this commit: - # https://github.com/python-trio/trio/commit/95843654173e3e826c34d70a90b369ba6edf2c23 - # - # OTOH, if you pass os.kill any *other* signal number... then CPython - # just calls TerminateProcess (wtf). - # - # So, anyway, os.kill is not so useful for testing purposes. Instead, - # we use raise(): - # - # https://msdn.microsoft.com/en-us/library/dwwzkt4c.aspx - # - # Have to import cffi inside the 'if os.name' block because we don't - # depend on cffi on non-Windows platforms. (It would be easy to switch - # this to ctypes though if we ever remove the cffi dependency.) - # - # Some more information: - # https://bugs.python.org/issue26350 - # - # Anyway, we use this for two things: - # - redelivering unhandled signals - # - generating synthetic signals for tests - # and for both of those purposes, 'raise' works fine. - import cffi - - _ffi = cffi.FFI() - _ffi.cdef("int raise(int);") - _lib = _ffi.dlopen("api-ms-win-crt-runtime-l1-1-0.dll") - signal_raise = getattr(_lib, "raise") -else: - - def signal_raise(signum: int) -> None: - signal.pthread_kill(threading.get_ident(), signum) - - # See: #461 as to why this is needed. # The gist is that threading.main_thread() has the capability to lie to us # if somebody else edits the threading ident cache to replace the main