Defer KI if trio is doing IO

docs/source/reference-lowlevel.rst

@@ -344,6 +344,8 @@ Spawning threads
 .. autofunction:: start_thread_soon
 
 
+.. _ki-handling:
+
 Safer KeyboardInterrupt handling
 ================================
 
@@ -355,10 +357,21 @@ correctness invariants. On the other, if the user accidentally writes
 an infinite loop, we do want to be able to break out of that. Our
 solution is to install a default signal handler which checks whether
 it's safe to raise :exc:`KeyboardInterrupt` at the place where the
-signal is received. If so, then we do; otherwise, we schedule a
-:exc:`KeyboardInterrupt` to be delivered to the main task at the next
-available opportunity (similar to how :exc:`~trio.Cancelled` is
-delivered).
+signal is received. If so, then we do. Otherwise, we cancel all tasks
+and add `KeyboardInterrupt` as the result of :func:`trio.run`.
+
+.. note:: This behavior means it's not a good idea to try to catch
+   `KeyboardInterrupt` within a Trio task. Most Trio
+   programs are I/O-bound, so most interrupts will be received while
+   no task is running (because Trio is waiting for I/O). There's no
+   task that should obviously receive the interrupt in such cases, so
+   Trio doesn't raise it within a task at all: every task gets cancelled,
+   then `KeyboardInterrupt` is raised once that's complete.
+
+   If you want to handle Ctrl+C by doing something other than "cancel
+   all tasks", then you should use :func:`~trio.open_signal_receiver` to
+   install a handler for `signal.SIGINT`. If you do that, then Ctrl+C will
+   go to your handler, and it can do whatever it wants.
 
 So that's great, but – how do we know whether we're in one of the
 sensitive parts of the program or not?

newsfragments/2649.removal.rst

@@ -0,0 +1,4 @@
+The abort function passed to :func:`~trio.lowlevel.wait_task_rescheduled`
+now directly takes as argument the cancellation exception that should be
+raised after a successful asynchronous cancellation. Previously, it took
+a callable that would raise the exception when called.

newsfragments/733.breaking.rst

@@ -0,0 +1,42 @@
+:ref:`Sometimes <ki-handling>`, a Trio program receives an interrupt
+signal (Ctrl+C) at a time when Python's default response (raising
+`KeyboardInterrupt` immediately) might corrupt Trio's internal
+state. Previously, Trio would handle this situation by raising the
+`KeyboardInterrupt` at the next :ref:`checkpoint <checkpoints>` executed
+by the main task (the one running the function you passed to :func:`trio.run`).
+This was responsible for a lot of internal complexity and sometimes led to
+surprising behavior.
+
+With this release, such a "deferred" `KeyboardInterrupt` is handled in a
+different way: Trio will first cancel all running tasks, then raise
+`KeyboardInterrupt` directly out of the call to :func:`trio.run`.
+The difference is relevant if you have code that tries to catch
+`KeyboardInterrupt` within Trio. This was never entirely robust, but it
+previously might have worked in many cases, whereas now it will never
+catch the interrupt.
+
+An example of code that mostly worked on previous releases, but won't
+work on this release::
+
+    async def main():
+        try:
+            await trio.sleep_forever()
+        except KeyboardInterrupt:
+            print("interrupted")
+    trio.run(main)
+
+The fix is to catch `KeyboardInterrupt` outside Trio::
+
+    async def main():
+        await trio.sleep_forever()
+    try:
+        trio.run(main)
+    except KeyboardInterrupt:
+        print("interrupted")
+
+If that doesn't work for you (because you want to respond to
+`KeyboardInterrupt` by doing something other than cancelling all
+tasks), then you can start a task that uses
+`trio.open_signal_receiver` to receive the interrupt signal ``SIGINT``
+directly and handle it however you wish. Such a task takes precedence
+over Trio's default interrupt handling.

src/trio/_channel.py

@@ -17,7 +17,7 @@
 import trio
 
 from ._abc import ReceiveChannel, ReceiveType, SendChannel, SendType, T
-from ._core import Abort, RaiseCancelT, Task, enable_ki_protection
+from ._core import Abort, Task, enable_ki_protection
 from ._util import (
     MultipleExceptionError,
     NoPublicConstructor,
@@ -227,7 +227,7 @@ async def send(self, value: SendType) -> None:
         self._state.send_tasks[task] = value
         task.custom_sleep_data = self
 
-        def abort_fn(_: RaiseCancelT) -> Abort:
+        def abort_fn(_: BaseException) -> Abort:
             self._tasks.remove(task)
             del self._state.send_tasks[task]
             return trio.lowlevel.Abort.SUCCEEDED
@@ -375,7 +375,7 @@ async def receive(self) -> ReceiveType:
         self._state.receive_tasks[task] = None
         task.custom_sleep_data = self
 
-        def abort_fn(_: RaiseCancelT) -> Abort:
+        def abort_fn(_: BaseException) -> Abort:
             self._tasks.remove(task)
             del self._state.receive_tasks[task]
             return trio.lowlevel.Abort.SUCCEEDED

src/trio/_core/_exceptions.py

@@ -1,7 +1,8 @@
 from __future__ import annotations
 
-from typing import TYPE_CHECKING
+from typing import TYPE_CHECKING, NoReturn
 
+from trio import _deprecate
 from trio._util import NoPublicConstructor, final
 
 if TYPE_CHECKING:
@@ -70,6 +71,19 @@ class Cancelled(BaseException, metaclass=NoPublicConstructor):
     def __str__(self) -> str:
         return "Cancelled"
 
+    def __call__(self) -> NoReturn:
+        # If a Cancelled exception is passed to an old abort_fn that
+        # expects a raise_cancel callback, someone will eventually try
+        # to call the exception instead of raising it. Provide a
+        # deprecation warning and raise it instead.
+        _deprecate.warn_deprecated(
+            "wait_task_rescheduled's abort_fn taking a callback argument",
+            "0.30.0",
+            issue=2649,
+            instead="an exception argument",
+        )
+        raise self
+
     def __reduce__(self) -> tuple[Callable[[], Cancelled], tuple[()]]:
         return (Cancelled._create, ())
 

src/trio/_core/_generated_io_kqueue.py

@@ -16,7 +16,7 @@
 
     from .. import _core
     from .._file_io import _HasFileNo
-    from ._traps import Abort, RaiseCancelT
+    from ._traps import Abort
 
 assert not TYPE_CHECKING or sys.platform == "darwin"
 
@@ -59,7 +59,7 @@ def monitor_kevent(
 
 @enable_ki_protection
 async def wait_kevent(
-    ident: int, filter: int, abort_func: Callable[[RaiseCancelT], Abort]
+    ident: int, filter: int, abort_func: Callable[[BaseException], Abort]
 ) -> Abort:
     """TODO: these are implemented, but are currently more of a sketch than
     anything real. See `#26

src/trio/_core/_io_epoll.py

@@ -16,7 +16,7 @@
 if TYPE_CHECKING:
     from typing_extensions import TypeAlias
 
-    from .._core import Abort, RaiseCancelT
+    from .._core import Abort
     from .._file_io import _HasFileNo
 
 
@@ -303,7 +303,7 @@ async def _epoll_wait(self, fd: int | _HasFileNo, attr_name: str) -> None:
         setattr(waiters, attr_name, _core.current_task())
         self._update_registrations(fd)
 
-        def abort(_: RaiseCancelT) -> Abort:
+        def abort(_: BaseException) -> Abort:
             setattr(waiters, attr_name, None)
             self._update_registrations(fd)
             return _core.Abort.SUCCEEDED

src/trio/_core/_io_kqueue.py

@@ -18,7 +18,7 @@
 
     from typing_extensions import TypeAlias
 
-    from .._core import Abort, RaiseCancelT, Task, UnboundedQueue
+    from .._core import Abort, Task, UnboundedQueue
     from .._file_io import _HasFileNo
 
 assert not TYPE_CHECKING or (sys.platform != "linux" and sys.platform != "win32")
@@ -147,7 +147,7 @@ async def wait_kevent(
         self,
         ident: int,
         filter: int,
-        abort_func: Callable[[RaiseCancelT], Abort],
+        abort_func: Callable[[BaseException], Abort],
     ) -> Abort:
         """TODO: these are implemented, but are currently more of a sketch than
         anything real. See `#26
@@ -160,8 +160,8 @@ async def wait_kevent(
             )
         self._registered[key] = _core.current_task()
 
-        def abort(raise_cancel: RaiseCancelT) -> Abort:
-            r = abort_func(raise_cancel)
+        def abort(cancel_exc: BaseException) -> Abort:
+            r = abort_func(cancel_exc)
             if r is _core.Abort.SUCCEEDED:  # TODO: test this branch
                 del self._registered[key]
             return r
@@ -180,7 +180,7 @@ async def _wait_common(
         event = select.kevent(fd, filter, flags)
         self._kqueue.control([event], 0)
 
-        def abort(_: RaiseCancelT) -> Abort:
+        def abort(_: BaseException) -> Abort:
             event = select.kevent(fd, filter, select.KQ_EV_DELETE)
             try:
                 self._kqueue.control([event], 0)

src/trio/_core/_io_windows.py

@@ -45,7 +45,7 @@
     from typing_extensions import Buffer, TypeAlias
 
     from .._file_io import _HasFileNo
-    from ._traps import Abort, RaiseCancelT
+    from ._traps import Abort
     from ._unbounded_queue import UnboundedQueue
 
 EventResult: TypeAlias = int
@@ -752,7 +752,7 @@ async def _afd_poll(self, sock: _HasFileNo | int, mode: str) -> None:
         # we let it escape.
         self._refresh_afd(base_handle)
 
-        def abort_fn(_: RaiseCancelT) -> Abort:
+        def abort_fn(_: BaseException) -> Abort:
             setattr(waiters, mode, None)
             self._refresh_afd(base_handle)
             return _core.Abort.SUCCEEDED
@@ -864,11 +864,11 @@ async def wait_overlapped(
             )
         task = _core.current_task()
         self._overlapped_waiters[lpOverlapped] = task
-        raise_cancel = None
+        cancel_exc = None
 
-        def abort(raise_cancel_: RaiseCancelT) -> Abort:
-            nonlocal raise_cancel
-            raise_cancel = raise_cancel_
+        def abort(cancel_exc_: BaseException) -> Abort:
+            nonlocal cancel_exc
+            cancel_exc = cancel_exc_
             try:
                 _check(kernel32.CancelIoEx(handle, lpOverlapped))
             except OSError as exc:
@@ -914,8 +914,8 @@ def abort(raise_cancel_: RaiseCancelT) -> Abort:
             # it will produce the right sorts of exceptions
             code = ntdll.RtlNtStatusToDosError(lpOverlappedTyped.Internal)
             if code == ErrorCodes.ERROR_OPERATION_ABORTED:
-                if raise_cancel is not None:
-                    raise_cancel()
+                if cancel_exc is not None:
+                    raise cancel_exc
                 else:
                     # We didn't request this cancellation, so assume
                     # it happened due to the underlying handle being

src/trio/_core/_parking_lot.py

@@ -183,7 +183,7 @@ async def park(self) -> None:
         self._parked[task] = None
         task.custom_sleep_data = self
 
-        def abort_fn(_: _core.RaiseCancelT) -> _core.Abort:
+        def abort_fn(_: BaseException) -> _core.Abort:
             del task.custom_sleep_data._parked[task]
             return _core.Abort.SUCCEEDED