-
-
Notifications
You must be signed in to change notification settings - Fork 30.5k
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
Conversation
This switches the main pyrepl event loop to always be non-blocking so that it can listen to incoming interruptions from other threads. This also resolves invalid display of exceptions from other threads (pythongh-123178).
@@ -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 comment
The reason will be displayed to describe this comment to others. Learn more.
asyncio REPL is now again properly recognizing KeyboardInterrupts. Display | |
asyncio REPL is now again properly recognizing :exc:`KeyboardInterrupt`. Display |
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.
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 comment
The 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 comment
The reason will be displayed to describe this comment to others. Learn more.
FTR, to put the s, you would do :exc:`KeyboardInterrupt`\s
|
||
handler = ExceptHookHandler() | ||
reader.threading_hook = handler | ||
threading.excepthook = handler.exception |
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.
This except hook essentially queues tracebacks for nicer display whenever the REPL is actually ready to do it.
@@ -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 comment
The 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.
return ( | ||
not self.event_queue.empty() | ||
or self.more_in_buffer() | ||
or bool(self.pollob.poll(timeout)) | ||
) |
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.
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.
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 comment
The 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.
@@ -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 comment
The 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 comment
The reason will be displayed to describe this comment to others. Learn more.
lol
|
||
|
||
def install_threading_hook(reader: Reader) -> None: | ||
import threading |
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.
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.
Looks great @ambv. Thanks for making the PR easy to review with explanatory comments. So excited for the new REPL @ambv and @pablogsal. 🎉
Thanks @ambv for the PR 🌮🎉.. I'm working now to backport this PR to: 3.13. |
…123795) This switches the main pyrepl event loop to always be non-blocking so that it can listen to incoming interruptions from other threads. This also resolves invalid display of exceptions from other threads (pythongh-123178). This also fixes freezes with pasting and an active input hook. (cherry picked from commit 033510e) Co-authored-by: Łukasz Langa <[email protected]>
GH-123799 is a backport of this pull request to the 3.13 branch. |
… (#123799) This switches the main pyrepl event loop to always be non-blocking so that it can listen to incoming interruptions from other threads. This also resolves invalid display of exceptions from other threads (gh-123178). This also fixes freezes with pasting and an active input hook. (cherry picked from commit 033510e) Co-authored-by: Łukasz Langa <[email protected]>
This switches the main pyrepl event loop to always be non-blocking so that it can listen to incoming interruptions from other threads.
This also resolves invalid display of exceptions from other threads (gh-123178).
This also fixes freezes with pasting and an active input hook.