Skip to content
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

Error exiting the new REPL with Ctrl+Z on Windows #119896

Closed
eryksun opened this issue Jun 1, 2024 · 8 comments
Closed

Error exiting the new REPL with Ctrl+Z on Windows #119896

eryksun opened this issue Jun 1, 2024 · 8 comments
Assignees
Labels
3.13 bugs and security fixes 3.14 new features, bugs and security fixes OS-windows stdlib Python modules in the Lib dir topic-repl Related to the interactive shell triaged The issue has been accepted as valid by a triager. type-bug An unexpected behavior, bug, or error

Comments

@eryksun
Copy link
Contributor

eryksun commented Jun 1, 2024

Bug report

Bug description:

With the new REPL implementation on Windows, I'm getting an error from code that mistakenly tries to suspend the current process via os.kill(os.getpid(), signal.SIGSTOP) when exiting via Ctrl+Z, Enter. This raises the following AttributeError:

    os.kill(os.getpid(), signal.SIGSTOP)
                         ^^^^^^^^^^^^^^
AttributeError: module 'signal' has no attribute 'SIGSTOP'

This is due to Ctrl+Z getting mapped to the "suspend" command. This key sequence has always been supported for exiting the REPL on Windows. It's also supported when reading from io._WindowsConsoleIO console-input files. Anything after Ctrl+Z at the start of a line gets ignored, yielding an empty read.

Note that the kernel on Windows has no support for POSIX signals. There's just a basic emulation of a few signals in the C runtime library. There's no support for SIGSTOP. It could be emulated, but it's not, and the GUI task manager hasn't implemented support for suspending and resuming processes. Thus on Windows you could just map Ctrl+Z to the "delete" command, and update the delete class to also exit the REPL if the raw event character is "\x1a".

CPython versions tested on:

CPython main branch

Operating systems tested on:

Windows

Linked PRs

@eryksun eryksun added type-bug An unexpected behavior, bug, or error OS-windows stdlib Python modules in the Lib dir triaged The issue has been accepted as valid by a triager. 3.13 bugs and security fixes 3.14 new features, bugs and security fixes topic-repl Related to the interactive shell labels Jun 1, 2024
@eryksun
Copy link
Contributor Author

eryksun commented Jun 1, 2024

This post is just an FYI for the developers of the new REPL.

On the subject of control characters, I see that Ctrl+C is mapped to the "interrupt" command. In the commands module, the interrupt class is implemented to call os.kill(os.getpid(), signal.SIGINT). On Windows, the closest one could get to this is calling os.kill(0, signal.CTRL_C_EVENT), which sends the Ctrl+C event to every process in the console session, including the current process.

Currently, it appears that this isn't an issue in practice, since the REPL doesn't clear the ENABLE_PROCESSED_INPUT flag in the console input mode, which would disable normal processing of Ctrl+C. Thus the REPL never sees the low-level Ctrl+C keydown character code (0x03). The console host has already consumed it to generate a Ctrl+C event. All that remains in the input buffer is the Ctrl+C keyup event, which the REPL rightly ignores.

@vstinner
Copy link
Member

vstinner commented Jun 3, 2024

Full output:

vstinner@WIN C:\victor\python\main>python  
Running Release|Win32 interpreter...
Python 3.14.0a0 (heads/main-dirty:4c387a76f3, May 31 2024, 15:49:06) [MSC v.1940 32 bit (Intel)] on win32
Type "help", "copyright", "credits" or "license" for more information.
>>> 
^Z
Traceback (most recent call last):
  File "<frozen runpy>", line 198, in _run_module_as_main
  File "<frozen runpy>", line 88, in _run_code
  File "C:\victor\python\main\Lib\_pyrepl\__main__.py", line 51, in <module>
    interactive_console()
    ~~~~~~~~~~~~~~~~~~~^^
  File "C:\victor\python\main\Lib\_pyrepl\__main__.py", line 48, in interactive_console
    return run_interactive(mainmodule)
  File "C:\victor\python\main\Lib\_pyrepl\simple_interact.py", line 176, in run_multiline_interactive_console
    statement = multiline_input(more_lines, ps1, ps2)
  File "C:\victor\python\main\Lib\_pyrepl\readline.py", line 376, in multiline_input
    return reader.readline()
           ~~~~~~~~~~~~~~~^^
  File "C:\victor\python\main\Lib\_pyrepl\reader.py", line 689, in readline
    self.handle1()
    ~~~~~~~~~~~~^^
  File "C:\victor\python\main\Lib\_pyrepl\reader.py", line 672, in handle1
    self.do_cmd(cmd)
    ~~~~~~~~~~~^^^^^
  File "C:\victor\python\main\Lib\_pyrepl\reader.py", line 619, in do_cmd
    command.do()
    ~~~~~~~~~~^^
  File "C:\victor\python\main\Lib\_pyrepl\commands.py", line 229, in do
    os.kill(os.getpid(), signal.SIGSTOP)
                         ^^^^^^^^^^^^^^
AttributeError: module 'signal' has no attribute 'SIGSTOP'

The error comes from the suspend command in Lib\_pyrepl\commands.py.

@mwichmann
Copy link

mwichmann commented Jun 27, 2024

A meetoo probably isn't of any help, but I was also surprised to run into this harmless-but-alarming-looking traceback on exiting. Definitely, the expectation based on long-time previous behavior is that ^Z exits (cleanly) the repl in Python on Windows.

@mwichmann
Copy link

This is still broken as of 3.13.0b4, the final beta. It's going to be pretty bad look if it goes out the door like this...

@devdanzin
Copy link
Contributor

Would something like this work?

diff --git a/Lib/_pyrepl/reader.py b/Lib/_pyrepl/reader.py
index 8b282a382d3..bbedcf50056 100644
--- a/Lib/_pyrepl/reader.py
+++ b/Lib/_pyrepl/reader.py
@@ -23,6 +23,7 @@

 from contextlib import contextmanager
 from dataclasses import dataclass, field, fields
+import sys
 import unicodedata
 from _colorize import can_colorize, ANSIColors  # type: ignore[import-not-found]

@@ -110,7 +111,7 @@ def make_default_commands() -> dict[CommandName, type[Command]]:
         (r"\C-w", "unix-word-rubout"),
         (r"\C-x\C-u", "upcase-region"),
         (r"\C-y", "yank"),
-        (r"\C-z", "suspend"),
+        (r"\C-z", "interrupt" if sys.platform.startswith("win32") else "suspend"),
         (r"\M-b", "backward-word"),
         (r"\M-c", "capitalize-word"),
         (r"\M-d", "kill-word"),

@devdanzin
Copy link
Contributor

I now believe the best solution is to check for Windows in the suspend command, like so:

diff --git a/Lib/_pyrepl/commands.py b/Lib/_pyrepl/commands.py
index c3fce91013b..de5e02356dd 100644
--- a/Lib/_pyrepl/commands.py
+++ b/Lib/_pyrepl/commands.py
@@ -21,6 +21,7 @@

 from __future__ import annotations
 import os
+import sys

 # Categories of actions:
 #  killing
@@ -229,10 +230,21 @@ def do(self) -> None:

 class suspend(Command):
     def do(self) -> None:
-        import signal
-
         r = self.reader
         p = r.pos
+        b = r.buffer
+        if sys.platform.startswith("win32"):
+            if (
+                p == 0
+                and len(b) == 0
+                and self.event[-1] == "\032"
+            ):
+                r.update_screen()
+                r.console.finish()
+                raise EOFError
+            else:
+                return
+        import signal
         r.console.finish()
         os.kill(os.getpid(), signal.SIGSTOP)
         ## this should probably be done

However, that still changes the behavior compared to the basic REPL, because in it users have to press Ctrl+Z then Enter, while in PyREPL just Ctrl+Z will exit.

@mwichmann
Copy link

The former seemed to work when I hacked in (modulo that it reported sys as undefined).

Personally, not too worried if ^Z quits immediately, since that's what happens on POSIX platforms for ^D. But yes, it's slightly different from the original-REPL behavior.

@hugovk
Copy link
Member

hugovk commented Aug 8, 2024

Triage: closing because the linked PR is merged, please re-open if still needed.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
3.13 bugs and security fixes 3.14 new features, bugs and security fixes OS-windows stdlib Python modules in the Lib dir topic-repl Related to the interactive shell triaged The issue has been accepted as valid by a triager. type-bug An unexpected behavior, bug, or error
Projects
None yet
Development

No branches or pull requests

6 participants