python / cpython

The Python programming language
https://www.python.org
Other
62.64k stars 30.05k forks source link

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

Closed eryksun closed 1 month ago

eryksun commented 4 months ago

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 commented 4 months ago

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 commented 4 months ago

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 commented 3 months ago

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 commented 2 months ago

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 commented 2 months ago

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 commented 2 months ago

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 commented 2 months ago

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 commented 1 month ago

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