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

On Windows sys.stdin.readline() doesn't handle Ctrl-C properly #62797

Closed
Drekin mannequin opened this issue Jul 30, 2013 · 14 comments
Closed

On Windows sys.stdin.readline() doesn't handle Ctrl-C properly #62797

Drekin mannequin opened this issue Jul 30, 2013 · 14 comments
Assignees
Labels
OS-windows type-bug An unexpected behavior, bug, or error

Comments

@Drekin
Copy link
Mannequin

Drekin mannequin commented Jul 30, 2013

BPO 18597
Nosy @vstinner, @tjguk, @eryksun, @Vgr255
Files
  • issue18597_3_6_0.patch
  • Note: these values reflect the state of the issue at the time it was migrated and might not reflect the current state.

    Show more details

    GitHub fields:

    assignee = 'https://github.com/tjguk'
    closed_at = <Date 2021-03-01.15:32:00.172>
    created_at = <Date 2013-07-30.09:13:12.193>
    labels = ['type-bug', 'OS-windows']
    title = "On Windows sys.stdin.readline() doesn't handle Ctrl-C properly"
    updated_at = <Date 2021-03-01.15:32:00.172>
    user = 'https://bugs.python.org/Drekin'

    bugs.python.org fields:

    activity = <Date 2021-03-01.15:32:00.172>
    actor = 'eryksun'
    assignee = 'tim.golden'
    closed = True
    closed_date = <Date 2021-03-01.15:32:00.172>
    closer = 'eryksun'
    components = ['Windows']
    creation = <Date 2013-07-30.09:13:12.193>
    creator = 'Drekin'
    dependencies = []
    files = ['42132']
    hgrepos = []
    issue_num = 18597
    keywords = ['patch']
    message_count = 14.0
    messages = ['193918', '193919', '193936', '193938', '196120', '224402', '246053', '256134', '261564', '261569', '278182', '278193', '278194', '323464']
    nosy_count = 7.0
    nosy_names = ['vstinner', 'tim.golden', 'kumba', 'eryksun', 'Drekin', 'abarry', 'troyhirni']
    pr_nums = []
    priority = 'normal'
    resolution = 'out of date'
    stage = 'resolved'
    status = 'closed'
    superseder = None
    type = 'behavior'
    url = 'https://bugs.python.org/issue18597'
    versions = ['Python 2.7', 'Python 3.5']

    @Drekin
    Copy link
    Mannequin Author

    Drekin mannequin commented Jul 30, 2013

    When I run sys.stdin.readline() interactivelly (on Windows and Python 3.3.2) and hit Ctrl-C, sometimes it returns an empty string just before KeyboardInterrupt is raised. Sometimes it isn't even raised and instead after hitting Return some weird SyntaxtError: unknown decode error (on line 0) occurs. Seems like propagation of KeyboardInterrupt is somehow out of sync. sys.stdin.read(n) has the same issue. May be related to recently fixed http://bugs.python.org/issue17619 where was similar situation with input().

    @Drekin Drekin mannequin added the OS-windows label Jul 30, 2013
    @tjguk
    Copy link
    Member

    tjguk commented Jul 30, 2013

    The Ctrl-C handling in Python on Windows is a bit strange in places. I'll add this to my list of things to look at. If you'd care to walk through the code to produce a patch or at least to point to suspect code, that would make it more likely that it be fixed.

    @tjguk tjguk self-assigned this Jul 30, 2013
    @tjguk tjguk added the type-bug An unexpected behavior, bug, or error label Jul 30, 2013
    @Drekin
    Copy link
    Mannequin Author

    Drekin mannequin commented Jul 30, 2013

    I haven't experience with Python C code but I tried to find some clues in the code. First for input(): it call PyOS_Readline which may call PyOS_StdioReadline > my_fgets > fgets in Parser/myreadline.c. There is Windows related comment on line 56:

    “Ctrl-C anywhere on the line or Ctrl-Z if the only character on a line will set ERROR_OPERATION_ABORTED. Under normal circumstances Ctrl-C will also have caused the SIGINT handler to fire which will have set the event object returned by _PyOS_SigintEvent. This signal fires in another thread and is not guaranteed to have occurred before this point in the code.
    Therefore: check whether the event is set with a small timeout. If it is, assume this is a Ctrl-C and reset the event. If it isn't set assume that this is a Ctrl-Z on its own and drop through to check for EOF.”

    For sys.stdin.readline and .read: it goes down the IO machinery from text IO, buffered IO and raw IO (in this case FileIO) to Modules/_io/fileio.c where it ends calling function read(fd, buf, len), probably from <unistd>. I don't know how read is implemented on Windows.

    I also tried calling ReadConsoleW from winapi via ctypes to read Unicode charactes from console (see http://bugs.python.org/issue1602). And there was similar issue with Ctrl-C occurring. What seems to work here is to put time.sleep(0.01) after ReadConsoleW.

    So the general pattern is following: when calling some low-level Windows function to read input from user and when he hits Ctrl-C, the function returns and SIGINT is generated. However it takes time for this signal to arrive. Because it may arrive anywhere in the following code, the strange behaviour may occur. In the input() case, when PyOS_Readline returns, it was probably enough time, so added PyErr_CheckSignals() catched that SIGINT/KeyboardInterrupt.

    We can find out about Ctrl-C having been pressed by calling winapi function GetLastError() and testing against ERROR_OPERATION_ABORTED. Then we should wait for the signal.

    @tjguk
    Copy link
    Member

    tjguk commented Jul 30, 2013

    Thanks for doing the investigation. Yes, that comment was added by me
    as part of the fix for bpo-1677. I'll try to have a look at the
    codepath you describe to see if we can add a similar workaround. The
    Ctrl-C / SIGINT handling on Windows is less than ideal, I admit.

    There was a similar problem in bpo-18040 which I closed as "won't fix"
    since the fix was arguably too intrusive for the extremely unlikely
    problem it was fixing. It might be worth seeing if the same root cause
    applies, though.

    @Drekin
    Copy link
    Mannequin Author

    Drekin mannequin commented Aug 25, 2013

    Why are there actually more codepaths which may raise this issue? My naive idea would be that input() is just thin wrapper around sys.stdout.write() (for prompt) and sys.stdin.readline() which leads to sys.stdin.buffer.raw.read* where is the place where some low level OS-dependent function to actually get input from user is called (unistd.read or GNU readline or whatever). And also there is the place where the waiting for KeyboardInterrupt on Windows should occur.

    @Drekin
    Copy link
    Mannequin Author

    Drekin mannequin commented Jul 31, 2014

    Shouldn't there be a Ctrl-C check in Modules/_io/fileio.c:fileio_read* functions? I think some of these are called by standard sys.stdin.readline().

    @eryksun
    Copy link
    Contributor

    eryksun commented Jul 2, 2015

    In Windows 10 ReadFile doesn't set ERROR_OPERATION_ABORTED (995) for Ctrl+C when reading console input, but ReadConsole does.

        >>> from ctypes import *
        >>> kernel32 = WinDLL('kernel32', use_last_error=True)
        >>> buf = (c_char * 1)()
        >>> n = c_uint()
        >>> kernel32.ReadFile(kernel32.GetStdHandle(-10), buf, 1, byref(n), None)
        Traceback (most recent call last):
          File "<stdin>", line 1, in <module>
        KeyboardInterrupt
        >>> get_last_error()
        0
        >>> kernel32.ReadConsoleA(kernel32.GetStdHandle(-10), buf, 1, byref(n), None)
        Traceback (most recent call last):
          File "<stdin>", line 1, in <module>
        KeyboardInterrupt
        >>> get_last_error()
        995

    Add this to the list of reasons Python should be using the console API for interactive standard streams. As is Ctrl+C is killing the REPL since it gets interpreted as EOF. This bug probably applies to Windows 8, too. Could someone check?

    Background:
    In Windows 7 reading from the console is implemented with a common code path to make an LPC call (NtRequestWaitReplyPort) to the console host process, conhost.exe. This was all completely redesigned for Windows 8, which instead uses the ConDrv device driver. Now ReadFile calls NtReadFile, and ReadConsole calls NtDeviceIoControlFile. When splitting this up they apparently forgot to set ERROR_OPERATION_ABORTED for Ctrl+C in ReadFile.

    @troyhirni
    Copy link
    Mannequin

    troyhirni mannequin commented Dec 9, 2015

    I'm also experiencing this on Windows 8 and 10. In the bare example below, I can Ctrl-C to exit the loop. When I press Enter again, the exception at the bottom appears.

    try:
    while True:
    input("? ")
    except:
    pass

    >>>
    >>> try:
    ...     while True:
    ...             input("? ")
    ... except:
    ...     pass
    ...
    ? asdf
    'asdf'
    ? qqwer
    'qqwer'
    ? >>>
      File "<stdin>", line 0
    ^
    

    SyntaxError: decoding with 'cp437' codec failed (KeyboardInterrupt: )

    >>

    @eryksun
    Copy link
    Contributor

    eryksun commented Mar 11, 2016

    This problem has come up in several issues now (see bpo-25376 and bpo-26531). I'm adding a patch for Python 3.6 to call ReadConsoleA instead of fgets in PyOS_Readline. This fixes Ctrl+C and EOF handling in Windows 10 for both the interactive shell and the built-in input() function.

    As noted previously, changes to the console in Windows 8 and 10 introduced a bug in ReadFile. It no longer sets the last error to ERROR_OPERATION_ABORTED (995) when a console read is interrupted by Ctrl+C or Ctrl+Break. This is a problem for the current implementation of PyOS_Readline, which calls ReadFile indirectly via C fgets.

    This bug can be avoided by calling ReadConsoleA instead of fgets when stdin is a console. Note that isatty() is insufficient to detect the console, since it's true for all character devices, such as NUL. I instead call GetConsoleMode to check for a console handle.

    I'm also looking into modifying Modules/signalmodule.c to set _PyOS_SigintEvent() when SIGBREAK is tripped, which matters when there's a non-default SIGBREAK handler. Also, PyErr_CheckSignals is a logical place to reset the event. Actually, it seems to me that everywhere in signalmodule.c where the signal flag is untripped should reset the event, in which case there should be an untrip_signal() to match trip_signal().

    @eryksun
    Copy link
    Contributor

    eryksun commented Mar 11, 2016

    Background Discussion

    The Windows 10 console uses the condrv.sys device driver, which is set up as follows in the NT namespace:

    C:\>odir \ -r -n con;con*$;cond*
    Directory of \
    
    Device
        ConDrv <Device>
    Driver
        condrv <Driver>
    GLOBAL??
        CON -> \Device\ConDrv\Console
        CONIN$ -> \Device\ConDrv\CurrentIn
        CONOUT$ -> \Device\ConDrv\CurrentOut
    

    Previously the base console API used an NT LPC port to communicate with the attached console process (i.e. an instance of conhost.exe). There wasn't a real console device. Instead, opening "CON", "CONIN$", or "CONOUT$" was special cased to call OpenConsoleW (undocumented).

    With the new console device driver, opening the DOS "CON" device gets translated to the NT path "\Device\ConDrv\Console", i.e. it opens the file named "Console" on the ConDrv device.

    Opening the Console file returns a handle for a regular kernel File object. To that end, you may have noticed that console handles in Windows 10 are no longer tagged for routing by setting the lower two bits (e.g. 3, 7, 11, etc). For example:

        >>> kernel32.GetStdHandle(STD_INPUT_HANDLE)
        32
        >>> kernel32.DebugBreak()
        (e1c.e20): Break instruction exception - code 80000003 (first chance)
        KERNELBASE!DebugBreak+0x2:
        00007ffa`60280262 cc              int     3
    0:000> !handle 32
    Handle 32
      Type          File
    

    Previously, all operations on console handles were internally routed to special console functions, such as ReadFile => ReadConsoleA. Thus with the old LPC-based console, a ReadFile basically has the behavior of ReadConsoleA (with the addition of special casing input lines that start with Ctrl+Z).

    The new design scraps a lot of the special-cased code. For example, reading from a console handle in Windows 10 uses a regular NtReadFile system call. So the error it sets, if any at all, depends on translating the NTSTATUS code that's returned by NtReadFile. Let's see what status the console sets here:

    C:\Temp>cdb -xi ld python ccbug.py
    
    [...]
    
    ntdll!LdrpDoDebuggerBreak+0x30:
    00007ffb`170de260 cc              int     3
    0:000> g
    3.5.1 (v3.5.1:37a07cee5969, Dec  6 2015, 01:54:25)
    [MSC v.1900 64 bit (AMD64)]
    
    calling DebugBreak...
    (8d0.62c): Break instruction exception - code 80000003 (first chance)
    KERNELBASE!DebugBreak+0x2:
    00007ffb`13f40262 cc              int     3
    0:000> bp ntdll!NtReadFile
    0:000> g
    Breakpoint 0 hit
    ntdll!NtReadFile:
    00007ffb`170b35d0 4c8bd1          mov     r10,rcx
    0:000> pt
    ntdll!NtReadFile+0xa:
    00007ffb`170b35da c3              ret
    0:000> r rax
    rax=0000000000000101
    

    The console weirdly returns a success code, STATUS_ALERTED (0x101, "the delay completed because the thread was alerted"), which is why ReadFile doesn't set an error. STATUS_ALERTED is normally returned when an NT wait function gets alerted by NtAlertThread (note that this is not the same as getting alerted by an asynchronous procedure call). For example:

    tid = threading.get_ident()
    h = kernel32.OpenThread(MAXIMUM_ALLOWED, 0, tid)
    t = threading.Timer(5, ntdll.NtAlertThread, (h,))
    delay = LARGE_INTEGER(10 * -10\*\*7) # 10 seconds
    t.start()
    r = ntdll.NtDelayExecution(True, byref(delay))
    
        >>> hex(r)
        '0x101'

    NtAlertThread is rarely used because WinAPI wait functions (e.g. SleepEx) automatically restart a wait when the underlying NT wait returns STATUS_ALERTED.

    The ReadConsole implementation has always translated STATUS_ALERTED to ERROR_OPERATION_ABORTED. This still exists in the Windows 10 implementation of ReadConsole. However, the correct error status for this case is STATUS_CANCELLED (0xC0000120, "the I/O request was cancelled"):

        >>> ntdll.RtlNtStatusToDosError(0xC0000120)
        995

    Whoever reimplemented the console IPC using a device driver should have updated the console to return STATUS_CANCELLED when an I/O operation is interrupted by Ctrl+C or Ctrl+Break. Then nothing would need to be special cased.

    @Drekin
    Copy link
    Mannequin Author

    Drekin mannequin commented Oct 6, 2016

    Maybe this was fixed with the recent fix of bpo-1602.

    @eryksun
    Copy link
    Contributor

    eryksun commented Oct 6, 2016

    Switching to ReadConsoleW in 3.6+ solves the problem with not seeing ERROR_OPERATION_ABORTED in Windows 8+, and with proper handling this potentially solves issues with Ctrl+C handling (when I last checked there were still bugs with this in the 3.6 beta). However, the problem still exists in 2.7 and 3.5, where the only possible solution is to switch to ReadConsoleA. Maybe once the new PyOS_StdioReadline code in 3.6 is stable, it can be backported to 3.5 using ReadConsoleA instead of ReadConsoleW. 2.7 will probably remain broken.

    @Drekin
    Copy link
    Mannequin Author

    Drekin mannequin commented Oct 6, 2016

    The main reason I have extended the support of win_unicode_console to Python 2.7 was that the related issues won't be fixed there, so using win_unicode_console may fix this as well.

    @kumba
    Copy link
    Mannequin

    kumba mannequin commented Aug 13, 2018

    I was able to modify eryksun's patch for Python-2.7.15, at least the bulk of it. It looks like the '_PyOS_SigintEvent' function is new to the 3.x branch, and despite it being a fairly simple function, it's used in a few core Python modules. So I'm not sure the difficulty of adding it for Python-2.7. Is there an alternative function available in the 2.7 code that I can replace it with?

    I also don't have a clue how anyone even builds Python-2.7 on Windows, because getting the required tools properly installed borders on the arcane (esp VS2008 tools). So testing it might be a bit difficult. Is there a cheat sheet somewhere?

    @eryksun eryksun closed this as completed Mar 1, 2021
    @ezio-melotti ezio-melotti transferred this issue from another repository Apr 10, 2022
    Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
    Labels
    OS-windows type-bug An unexpected behavior, bug, or error
    Projects
    None yet
    Development

    No branches or pull requests

    2 participants