Skip to content

Commit 50fa1b4

Browse files
committed
fix: allow async cancellation
1 parent 8feaf40 commit 50fa1b4

13 files changed

Lines changed: 693 additions & 291 deletions

marimo/_pyodide/pyodide_session.py

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -483,7 +483,7 @@ def _launch_pyodide_kernel(
483483
)
484484

485485
if is_edit_mode:
486-
signal.signal(signal.SIGINT, handlers.construct_interrupt_handler(ctx))
486+
signal.signal(signal.SIGINT, handlers.construct_interrupt_handler())
487487

488488
async def listen_completion() -> None:
489489
while True:

marimo/_runtime/context/kernel_context.py

Lines changed: 9 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -24,6 +24,7 @@
2424

2525
from marimo._ast.app import InternalApp
2626
from marimo._messaging.types import KernelStreams
27+
from marimo._runtime.runner.scheduler import Scheduler
2728
from marimo._runtime.runtime import Kernel
2829
from marimo._runtime.state import State
2930
from marimo._runtime.virtual_file import VirtualFileStorageType
@@ -41,6 +42,14 @@ class KernelRuntimeContext(RuntimeContext):
4142
_app: InternalApp | None = None
4243
_id_provider: IDProvider | None = None
4344
_execution_context: ExecutionContext | None = None
45+
# Set while a Scheduler's `async with` is open. Lookup goes through
46+
# the currently-installed context — not one captured at install
47+
# time — so embedded-app child contexts route SIGINT correctly.
48+
_active_scheduler: Scheduler | None = None
49+
50+
@property
51+
def active_scheduler(self) -> Scheduler | None:
52+
return self._active_scheduler
4453

4554
@property
4655
def graph(self) -> DirectedGraph:

marimo/_runtime/executor/evaluator.py

Lines changed: 2 additions & 65 deletions
Original file line numberDiff line numberDiff line change
@@ -3,24 +3,18 @@
33

44
from __future__ import annotations
55

6-
import asyncio
7-
import contextlib
8-
import functools
9-
import signal
10-
import threading
116
from dataclasses import replace
12-
from typing import TYPE_CHECKING, Any
7+
from typing import TYPE_CHECKING
138

149
from marimo import _loggers
1510
from marimo._entrypoints.registry import EntryPointRegistry
16-
from marimo._runtime.control_flow import MarimoInterrupt
1711
from marimo._runtime.executor.executor import DefaultExecutor, Executor
1812
from marimo._runtime.executor.lifecycles import ExecutionLifecycle, Skip
1913
from marimo._runtime.runner.result import RunResult
2014
from marimo._types.globals import MutableGlobals
2115

2216
if TYPE_CHECKING:
23-
from collections.abc import Callable, Iterator
17+
from collections.abc import Callable
2418

2519
from marimo._ast.cell import CellImpl
2620

@@ -92,18 +86,6 @@ def evaluate_sync(
9286

9387
return self._teardown_chain(cell, glbls, completed, result)
9488

95-
async def evaluate_interruptible(
96-
self, cell: CellImpl, glbls: MutableGlobals
97-
) -> RunResult:
98-
"""Await `evaluate` with SIGINT capture for coroutine cells."""
99-
if not cell.is_coroutine():
100-
return await self.evaluate(cell, glbls)
101-
future = asyncio.ensure_future(self.evaluate(cell, glbls))
102-
if threading.current_thread() is threading.main_thread():
103-
with _cancel_on_sigint(future):
104-
return await future
105-
return await future
106-
10789
def _setup_chain(
10890
self, cell: CellImpl, glbls: MutableGlobals
10991
) -> tuple[list[ExecutionLifecycle], Skip | None, BaseException | None]:
@@ -196,48 +178,3 @@ def resolve_executor() -> Executor:
196178
e,
197179
)
198180
return DefaultExecutor()
199-
200-
201-
# Adapted from
202-
# https://github.com/ipython/ipykernel/blob/eddd3e666a82ebec287168b0da7cfa03639a3772/ipykernel/ipkernel.py#L312
203-
@contextlib.contextmanager
204-
def _cancel_on_sigint(future: asyncio.Future[Any]) -> Iterator[None]:
205-
"""Cancel `future` if a SIGINT arrives during evaluation."""
206-
sigint_future: asyncio.Future[int] = asyncio.Future()
207-
208-
def cancel_unless_done(f: asyncio.Future[Any], _: Any) -> None:
209-
if f.cancelled() or f.done():
210-
return
211-
f.cancel()
212-
213-
sigint_future.add_done_callback(
214-
functools.partial(cancel_unless_done, future)
215-
)
216-
future.add_done_callback(
217-
functools.partial(cancel_unless_done, sigint_future)
218-
)
219-
220-
# Capture the previously-installed SIGINT handler *before* we install
221-
# ours so `handle_sigint` can invoke it for its side effects
222-
# (kernel broadcast, duckdb interrupt). For async cells the actual
223-
# halt comes from cancelling the future, not from a raised
224-
# `MarimoInterrupt` — so we swallow that here.
225-
prior_sigint = signal.getsignal(signal.SIGINT)
226-
227-
def handle_sigint(signum: int, frame: Any) -> None:
228-
if sigint_future.cancelled() or sigint_future.done():
229-
return
230-
sigint_future.set_result(1)
231-
if callable(prior_sigint):
232-
try:
233-
prior_sigint(signum, frame)
234-
except MarimoInterrupt:
235-
# The kernel's handler raises MarimoInterrupt for sync
236-
# halt; we cancel the future instead.
237-
pass
238-
239-
save_sigint = signal.signal(signal.SIGINT, handle_sigint)
240-
try:
241-
yield
242-
finally:
243-
signal.signal(signal.SIGINT, save_sigint)

marimo/_runtime/handlers.py

Lines changed: 42 additions & 23 deletions
Original file line numberDiff line numberDiff line change
@@ -10,6 +10,7 @@
1010
from marimo._messaging.notification_utils import broadcast_notification
1111
from marimo._runtime.context import get_context
1212
from marimo._runtime.context.kernel_context import KernelRuntimeContext
13+
from marimo._runtime.context.types import safe_get_context
1314
from marimo._runtime.control_flow import MarimoInterrupt
1415

1516
LOGGER = _loggers.marimo_logger()
@@ -20,35 +21,53 @@
2021
from marimo._runtime.runtime import Kernel
2122

2223

23-
def construct_interrupt_handler(
24-
context: KernelRuntimeContext,
25-
) -> Callable[[int, Any], None]:
24+
def construct_interrupt_handler() -> Callable[[int, Any], None]:
2625
def interrupt_handler(signum: int, frame: Any) -> None:
2726
"""Tries to interrupt the kernel."""
2827
del signum
2928
del frame
3029

30+
# Resolve the *currently installed* context, not one captured at
31+
# install time — embedded apps swap in their own child context.
32+
ctx = safe_get_context()
33+
if not isinstance(ctx, KernelRuntimeContext):
34+
return
35+
36+
# `execution_context` is a per-task ContextVar — unreadable from
37+
# this thread while user tasks are suspended in `select()`. The
38+
# scheduler publication is the authoritative "is a run in flight"
39+
# signal; `execution_context` is opportunistic (only used for
40+
# the duckdb hook below).
41+
sched = ctx.active_scheduler
42+
exec_ctx = ctx.execution_context
43+
if sched is None and exec_ctx is None:
44+
return
45+
3146
LOGGER.info("Interrupt request received")
32-
# TODO(akshayka): if kernel is in `run` but not executing,
33-
# it won't be interrupted, which isn't right ... but the
34-
# probability of that happening is low.
35-
if context.execution_context is not None:
36-
broadcast_notification(InterruptedNotification())
37-
# DuckDB connections are sometimes left in an inconsistent
38-
# state when interrupted by a SIGINT. Manually interrupting
39-
# duckdb through its own API seems to be safer.
40-
if context.execution_context.duckdb_connection is not None:
41-
try:
42-
context.execution_context.duckdb_connection.interrupt()
43-
except Exception as e:
44-
# Coarse try/except; let's not kill the kernel if something
45-
# goes wrong.
46-
LOGGER.warning(
47-
"Failed to interrupt running duckdb connection. This "
48-
"may be a bug in duckdb or marimo. %s",
49-
e,
50-
)
51-
raise MarimoInterrupt
47+
broadcast_notification(InterruptedNotification())
48+
49+
# DuckDB connections are sometimes left in an inconsistent state
50+
# when interrupted by a SIGINT; route through duckdb's own API.
51+
if exec_ctx is not None and exec_ctx.duckdb_connection is not None:
52+
try:
53+
exec_ctx.duckdb_connection.interrupt()
54+
except Exception as e:
55+
LOGGER.warning(
56+
"Failed to interrupt running duckdb connection. This "
57+
"may be a bug in duckdb or marimo. %s",
58+
e,
59+
)
60+
61+
if sched is not None and sched.has_active_tasks():
62+
# Async cell in flight: cancel via the loop. Raising from a
63+
# signal handler escapes into asyncio internals and surfaces
64+
# as an internal-error empty RunResult.
65+
sched.cancel_all()
66+
return
67+
68+
if sched is not None:
69+
sched.cancel_all()
70+
raise MarimoInterrupt
5271

5372
return interrupt_handler
5473

0 commit comments

Comments
 (0)