Skip to content

Commit

Permalink
Get rid of signal_raise
Browse files Browse the repository at this point in the history
  • Loading branch information
A5rocks authored Nov 1, 2024
1 parent 9d6e134 commit 9a23dec
Show file tree
Hide file tree
Showing 7 changed files with 22 additions and 95 deletions.
7 changes: 3 additions & 4 deletions src/trio/_core/_tests/test_guest_mode.py
Original file line number Diff line number Diff line change
Expand Up @@ -28,7 +28,6 @@
import trio.testing
from trio.abc import Instrument

from ..._util import signal_raise
from .tutil import gc_collect_harder, restore_unraisablehook

if TYPE_CHECKING:
Expand Down Expand Up @@ -577,10 +576,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:
Expand All @@ -593,7 +592,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:
Expand Down
3 changes: 1 addition & 2 deletions src/trio/_core/_tests/test_ki.py
Original file line number Diff line number Diff line change
Expand Up @@ -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:
Expand All @@ -41,7 +40,7 @@


def ki_self() -> None:
signal_raise(signal.SIGINT)
signal.raise_signal(signal.SIGINT)


def test_ki_self() -> None:
Expand Down
6 changes: 3 additions & 3 deletions src/trio/_signals.py
Original file line number Diff line number Diff line change
Expand Up @@ -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
Expand Down Expand Up @@ -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()
Expand All @@ -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()

Expand Down
3 changes: 1 addition & 2 deletions src/trio/_tests/test_repl.py
Original file line number Diff line number Diff line change
Expand Up @@ -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')",
Expand Down
29 changes: 14 additions & 15 deletions src/trio/_tests/test_signals.py
Original file line number Diff line number Diff line change
Expand Up @@ -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
Expand All @@ -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
Expand Down Expand Up @@ -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__()
Expand Down Expand Up @@ -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()
Expand All @@ -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}
Expand All @@ -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
Expand All @@ -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
Expand Down
15 changes: 0 additions & 15 deletions src/trio/_tests/test_util.py
Original file line number Diff line number Diff line change
Expand Up @@ -21,27 +21,12 @@
fixup_module_metadata,
generic_function,
is_main_thread,
signal_raise,
)
from ..testing import wait_all_tasks_blocked

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")
Expand Down
54 changes: 0 additions & 54 deletions src/trio/_util.py
Original file line number Diff line number Diff line change
Expand Up @@ -35,60 +35,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
Expand Down

0 comments on commit 9a23dec

Please sign in to comment.