-
-
Notifications
You must be signed in to change notification settings - Fork 31.2k
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
gh-120221: Support KeyboardInterrupt in asyncio REPL #123795
Changes from all commits
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
Original file line number | Diff line number | Diff line change |
---|---|---|
@@ -0,0 +1,74 @@ | ||
from __future__ import annotations | ||
|
||
from dataclasses import dataclass, field | ||
import traceback | ||
|
||
|
||
TYPE_CHECKING = False | ||
if TYPE_CHECKING: | ||
from threading import Thread | ||
from types import TracebackType | ||
from typing import Protocol | ||
|
||
class ExceptHookArgs(Protocol): | ||
@property | ||
def exc_type(self) -> type[BaseException]: ... | ||
@property | ||
def exc_value(self) -> BaseException | None: ... | ||
@property | ||
def exc_traceback(self) -> TracebackType | None: ... | ||
@property | ||
def thread(self) -> Thread | None: ... | ||
|
||
class ShowExceptions(Protocol): | ||
def __call__(self) -> int: ... | ||
def add(self, s: str) -> None: ... | ||
|
||
from .reader import Reader | ||
|
||
|
||
def install_threading_hook(reader: Reader) -> None: | ||
import threading | ||
|
||
@dataclass | ||
class ExceptHookHandler: | ||
lock: threading.Lock = field(default_factory=threading.Lock) | ||
messages: list[str] = field(default_factory=list) | ||
|
||
def show(self) -> int: | ||
count = 0 | ||
with self.lock: | ||
if not self.messages: | ||
return 0 | ||
reader.restore() | ||
for tb in self.messages: | ||
count += 1 | ||
if tb: | ||
print(tb) | ||
self.messages.clear() | ||
reader.scheduled_commands.append("ctrl-c") | ||
reader.prepare() | ||
return count | ||
|
||
def add(self, s: str) -> None: | ||
with self.lock: | ||
self.messages.append(s) | ||
|
||
def exception(self, args: ExceptHookArgs) -> None: | ||
lines = traceback.format_exception( | ||
args.exc_type, | ||
args.exc_value, | ||
args.exc_traceback, | ||
colorize=reader.can_colorize, | ||
) # type: ignore[call-overload] | ||
pre = f"\nException in {args.thread.name}:\n" if args.thread else "\n" | ||
tb = pre + "".join(lines) | ||
self.add(tb) | ||
|
||
def __call__(self) -> int: | ||
return self.show() | ||
|
||
|
||
handler = ExceptHookHandler() | ||
reader.threading_hook = handler | ||
threading.excepthook = handler.exception | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. This except hook essentially queues tracebacks for nicer display whenever the REPL is actually ready to do it. |
Original file line number | Diff line number | Diff line change |
---|---|---|
|
@@ -36,8 +36,7 @@ | |
|
||
# types | ||
Command = commands.Command | ||
if False: | ||
from .types import Callback, SimpleContextManager, KeySpec, CommandName | ||
from .types import Callback, SimpleContextManager, KeySpec, CommandName | ||
|
||
|
||
def disp_str(buffer: str) -> tuple[str, list[int]]: | ||
|
@@ -247,6 +246,7 @@ class Reader: | |
lxy: tuple[int, int] = field(init=False) | ||
scheduled_commands: list[str] = field(default_factory=list) | ||
can_colorize: bool = False | ||
threading_hook: Callback | None = None | ||
|
||
## cached metadata to speed up screen refreshes | ||
@dataclass | ||
|
@@ -722,6 +722,24 @@ def do_cmd(self, cmd: tuple[str, list[str]]) -> None: | |
self.console.finish() | ||
self.finish() | ||
|
||
def run_hooks(self) -> None: | ||
threading_hook = self.threading_hook | ||
if threading_hook is None and 'threading' in sys.modules: | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. This automatic installation is such that when people create threads in the REPL directly or indirectly (via libraries), they get correctly formatted tracebacks. It just so happens that asyncio REPL uses threads so it triggers this condition, too. |
||
from ._threading_handler import install_threading_hook | ||
install_threading_hook(self) | ||
if threading_hook is not None: | ||
try: | ||
threading_hook() | ||
except Exception: | ||
pass | ||
|
||
input_hook = self.console.input_hook | ||
if input_hook: | ||
try: | ||
input_hook() | ||
except Exception: | ||
pass | ||
|
||
def handle1(self, block: bool = True) -> bool: | ||
"""Handle a single event. Wait as long as it takes if block | ||
is true (the default), otherwise return False if no event is | ||
|
@@ -732,16 +750,13 @@ def handle1(self, block: bool = True) -> bool: | |
self.dirty = True | ||
|
||
while True: | ||
input_hook = self.console.input_hook | ||
if input_hook: | ||
input_hook() | ||
# We use the same timeout as in readline.c: 100ms | ||
while not self.console.wait(100): | ||
input_hook() | ||
event = self.console.get_event(block=False) | ||
else: | ||
event = self.console.get_event(block) | ||
if not event: # can only happen if we're not blocking | ||
# We use the same timeout as in readline.c: 100ms | ||
self.run_hooks() | ||
self.console.wait(100) | ||
event = self.console.get_event(block=False) | ||
if not event: | ||
if block: | ||
continue | ||
return False | ||
|
||
translate = True | ||
|
@@ -763,8 +778,7 @@ def handle1(self, block: bool = True) -> bool: | |
if cmd is None: | ||
if block: | ||
continue | ||
else: | ||
return False | ||
return False | ||
|
||
self.do_cmd(cmd) | ||
return True | ||
|
Original file line number | Diff line number | Diff line change |
---|---|---|
|
@@ -199,8 +199,14 @@ def _my_getstr(cap: str, optional: bool = False) -> bytes | None: | |
self.event_queue = EventQueue(self.input_fd, self.encoding) | ||
self.cursor_visible = 1 | ||
|
||
def more_in_buffer(self) -> bool: | ||
return bool( | ||
self.input_buffer | ||
and self.input_buffer_pos < len(self.input_buffer) | ||
) | ||
|
||
def __read(self, n: int) -> bytes: | ||
if not self.input_buffer or self.input_buffer_pos >= len(self.input_buffer): | ||
if not self.more_in_buffer(): | ||
self.input_buffer = os.read(self.input_fd, 10000) | ||
|
||
ret = self.input_buffer[self.input_buffer_pos : self.input_buffer_pos + n] | ||
|
@@ -393,6 +399,7 @@ def get_event(self, block: bool = True) -> Event | None: | |
""" | ||
if not block and not self.wait(timeout=0): | ||
return None | ||
|
||
while self.event_queue.empty(): | ||
while True: | ||
try: | ||
|
@@ -413,7 +420,11 @@ def wait(self, timeout: float | None = None) -> bool: | |
""" | ||
Wait for events on the console. | ||
""" | ||
return bool(self.pollob.poll(timeout)) | ||
return ( | ||
not self.event_queue.empty() | ||
or self.more_in_buffer() | ||
or bool(self.pollob.poll(timeout)) | ||
) | ||
Comment on lines
+423
to
+427
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. The previous behavior caused pyrepl to lose reads in non-blocking mode (i.e. with an input hook) if more than 1 character was read previously to the input_buffer (only 1 character is turned into an event at a time). This made tests flaky. Now they pass every time. |
||
|
||
def set_cursor_vis(self, visible): | ||
""" | ||
|
Original file line number | Diff line number | Diff line change |
---|---|---|
|
@@ -479,7 +479,7 @@ def wait(self, timeout: float | None) -> bool: | |
while True: | ||
if msvcrt.kbhit(): # type: ignore[attr-defined] | ||
return True | ||
if timeout and time.time() - start_time > timeout: | ||
if timeout and time.time() - start_time > timeout / 1000: | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. haha There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. lol |
||
return False | ||
time.sleep(0.01) | ||
|
||
|
Original file line number | Diff line number | Diff line change |
---|---|---|
|
@@ -127,6 +127,15 @@ def run(self): | |
|
||
loop.call_soon_threadsafe(loop.stop) | ||
|
||
def interrupt(self) -> None: | ||
if not CAN_USE_PYREPL: | ||
return | ||
|
||
from _pyrepl.simple_interact import _get_reader | ||
r = _get_reader() | ||
if r.threading_hook is not None: | ||
r.threading_hook.add("") # type: ignore | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. This is how we add a KeyboardInterrupt to the other thread. We don't want a traceback, and the "KeyboardInterrupt" message will already appear. |
||
|
||
|
||
if __name__ == '__main__': | ||
sys.audit("cpython.run_stdin") | ||
|
@@ -184,6 +193,7 @@ def run(self): | |
keyboard_interrupted = True | ||
if repl_future and not repl_future.done(): | ||
repl_future.cancel() | ||
repl_thread.interrupt() | ||
continue | ||
else: | ||
break | ||
|
Original file line number | Diff line number | Diff line change | ||||
---|---|---|---|---|---|---|
@@ -0,0 +1,2 @@ | ||||||
asyncio REPL is now again properly recognizing KeyboardInterrupts. Display | ||||||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more.
Suggested change
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. That's not the same word. Let's do stuff like that in a subsequent PR please, so we don't waste time on CI now. There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Agreed. The upcoming release is more important now. There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. FTR, to put the s, you would do |
||||||
of exceptions raised in secondary threads is fixed. |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I put this import here so that pyrepl will continue to work in thread-free environments like iOS and the browser.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
FYI: iOS and Android do support threads, but not subprocesses. The only platform that doesn't support threads is WASM.