winpty icon indicating copy to clipboard operation
winpty copied to clipboard

Ctrl-C handling is inadequate for interactive cmd and PowerShell

Open unshare opened this issue 7 years ago • 23 comments

If I use a combination of Cygwin + OpenSSH + WinPTY to run interactive cmd or PowerShell they don't treat Ctrl-C as a command input cancellation.

Ctrl-C works for command abort, so this is not a pressing issue. Just an annoyance due to my keyboard skills.

I've reviewed Ctrl-C handling (the one with GenerateConsoleCtrlEvent(CTRL_C_EVENT, 0)) and I have no idea what is wrong. But I'm not a Win32 API specialist.

unshare avatar May 16 '17 12:05 unshare

Yeah, I've noticed this before. I think synthesizing Ctrl-C requires more than just calling GenerateConsoleCtrlEvent. My impression was that the official API was insufficient. Maybe it had something to do with interrupting an in-progress ReadConsole API?

I wonder if winpty should synthesize Ctrl-C GUI events, like it does for WSL navigation keys.

rprichard avatar May 17 '17 09:05 rprichard

It looks like ConEmu also takes the send-a-GUI-message route.

// (wParam == 'C' || wParam == VK_PAUSE/*Break*/)
// Ctrl+C & Ctrl+Break are very special signals in Windows
// After many tries I decided to send 'C'/Break to console window
// via simple `SendMessage(ghConWnd, WM_KEYDOWN, r.Event.KeyEvent.wVirtualKeyCode, 0)`
// That is done inside ../ConEmuCD/Queue.cpp::ProcessInputMessage
// Some restriction applies to it also, look inside `ProcessInputMessage`
//
// Yep, there is another nice function - GenerateConsoleCtrlEvent
// But it has different restrictions and it doesn't break ReadConsole
// because that function loops deep inside Windows kernel...
// However, user can call it via GuiMacro:
// Break() for Ctrl+C or Break(1) for Ctrl+Break
// if simple Keys("^c") does not work in your case.

https://github.com/Maximus5/ConEmu/blob/694d71c27f83628e65d2fbbb54c4f2c94547933f/src/ConEmu/RealConsole.cpp#L6798

if (lbProcessEvent)
{
    // Issue 590: GenerateConsoleCtrlEvent does not break ReadConsole[A|W] function!
    SetLastError(0);
    LRESULT lSendRc =
    SendMessage(ghConWnd, WM_KEYDOWN, r.Event.KeyEvent.wVirtualKeyCode, 0);
    DWORD nErrCode = GetLastError();
    msprintf(szLog, countof(szLog), L"  ---  CtrlC/CtrlBreak sent (%u,%u)", LODWORD(lSendRc), nErrCode);
    LogString(szLog);
}

https://github.com/Maximus5/ConEmu/blob/694d71c27f83628e65d2fbbb54c4f2c94547933f/src/ConEmuCD/Queue.cpp#L106

rprichard avatar May 17 '17 09:05 rprichard

I noticed a related issue: https://github.com/rprichard/winpty/issues/117

rprichard avatar May 17 '17 10:05 rprichard

Thanks a lot, really, but it's only partial success. cmd works great, but it's not fixed for PowerShell (just tested) Should I report a new issue?

unshare avatar May 18 '17 08:05 unshare

I can just reopen this issue. Can you provide more details? It's working with both CMD and PowerShell for me.

e.g. I type some text into at the PowerShell prompt, then press Ctrl-C, and it moves to the next line. I'm testing on Windows 7, though -- maybe I should look at Windows 10.

rprichard avatar May 18 '17 08:05 rprichard

OK, it's broken on Windows 10 v15063. It just adds a ^C to the end of the line.

rprichard avatar May 18 '17 08:05 rprichard

Correct. I can test against a Windows Server 2016 in a couple of hours. Or there's no need?

unshare avatar May 18 '17 08:05 unshare

Probably not necessary, I've found something that's obviously wrong, but I'm wondering why I got it wrong. I think I'll have it fixed in a few hours / a day or two.

i.e For Ctrl-C, winpty is generating:

$ ./build/winpty ./build/winpty-agent.exe  --show-input

Press any keys -- Ctrl-D exits

buffer-resized: dwSize=(132,3000)
key: dn rpt=1 scn=0x46 CANCEL ch=0x3
key: up rpt=1 scn=0x46 CANCEL ch=0x3

The normal console generates:

C:\rprichard\proj\winpty>build\winpty-agent.exe --show-input

Press any keys -- Ctrl-D exits

key: dn rpt=1 scn=0x1d LCtrl-CONTROL ch=0
key: dn rpt=1 scn=0x2e LCtrl-C ch=0x3
key: up rpt=1 scn=0x2e LCtrl-C ch=0x3
key: up rpt=1 scn=0x1d CONTROL ch=0

I bet that fixing the discrepancy will fix the PowerShell problem.

rprichard avatar May 18 '17 08:05 rprichard

i.e. The issue is that winpty isn't matching what Windows would use in a KEY_EVENT_RECORD object in the console input buffer. winpty is using VK_CANCEL with no modifiers, but the real Windows console uses a Ctrl modifier and a virtualkey of 'C'. We both use 0x3 for the character.

rprichard avatar May 18 '17 08:05 rprichard

The exact same situations happens in WinXP, so it's not a new bug. I probably didn't notice it, because most programs use SetConsoleCtrlHandler.

Here's one way to fix the bug:

$ git diff
diff --git a/src/agent/DefaultInputMap.cc b/src/agent/DefaultInputMap.cc
index b4dd7b4..e3a4dba 100644
--- a/src/agent/DefaultInputMap.cc
+++ b/src/agent/DefaultInputMap.cc
@@ -235,6 +235,7 @@ static void addSimpleEntries(InputMap &inputMap) {

         {   "\x7F",         { VK_BACK,  '\x08', 0,                                  } },
         {   ESC "\x7F",     { VK_BACK,  '\x08', LEFT_ALT_PRESSED,                   } },
+        {   "\x03",         { 'C',      '\x03', LEFT_CTRL_PRESSED,                  } },

         // Handle special F1-F5 for TERM=linux and TERM=cygwin.
         {   ESC "[[A",      { VK_F1,    '\0',   0                                   } },

Maybe I'll remove the (inputConsoleMode() & ENABLE_PROCESSED_INPUT) restriction on the GUI-message code path, though.

rprichard avatar May 18 '17 09:05 rprichard

I tried reusing the GUI-message code path for unprocessed Ctrl-C, but the character code was always NUL, and PowerShell didn't work. The diff I posted should work, though.

rprichard avatar May 18 '17 09:05 rprichard

Let me know if my change fixed it. It worked for me on Windows 10.

rprichard avatar May 18 '17 10:05 rprichard

Well, I've tested the current HEAD (0c86e4f) against

  • Windows 10 1703 64-bit locally
  • Windows Server 2016 Core and Windows Hyper-V Server 2016 through Cygwin + OpenSSH The former works great, thank you. The later not quite. This change has broken something.
  • In PowerShell, Ctrl-C does not interrupt the current command (e.g. sleep 30)
  • In cmd, on the contrary, command interrupt works, but Ctrl-C doesn't cancel command input

unshare avatar May 18 '17 11:05 unshare

I reverted the first part of this issue's fix -- winpty uses GenerateConsoleCtrlEvent again. I reopened this issue, but I don't know how to fix it, so maybe it's just a limitation. Maybe a compromise is possible...

What I expect now:

  • Ctrl-C will always interrupt a command.
  • Ctrl-C will cancel command input in new versions of PowerShell (e.g Win10), but not in cmd or in old versions of PowerShell (e.g. Win7).

I don't really know how Windows' input handling works -- I'll describe what I think happens. When an event like WM_KEYDOWN is added to a message queue, Windows reads the entire keyboard state (256 bytes) and associates it with the event. When the event is dispatched, the event's keyboard state is made available through GetKeyboardState. When the console sees a 'C' key-down message, it checks GetKeyboardState for modifiers keys. The SSH server uses a background window station/desktop, where the keyboard state is always "no keys pressed." Therefore, it's impossible to send a Ctrl-C WM_KEYDOWN message to a backgrounded console window.

I tried using SendInput and keybd_event to hold down Ctrl while sending just WM_KEY{DOWN,UP} for a 'C' character. This worked in a foreground desktop, but the SendInput call failed with an "Access Denied" error in the background desktop. Even if it worked, a window station can have multiple desktops -- window stations have independent keyboard state, but what about desktops? I don't want winpty to interfere with the keyboard state for other programs (including other winpty instances).

I noticed that the KEY_EVENT_RECORD had a NUL UnicodeChar with SendMessage, but a non-NUL value if I used PostMessage. On the other hand, when I tried sending four message with PostMessage (Ctrl-down, C-down, C-up, Ctrl-up), Windows reordered the messages to (Ctrl-down, Ctrl-up, C-down, C-up). A Sleep(1) call "fixes" the issue, but might be introducing a race. (Maybe not, though? I see that SendInput uses KEYBDINPUT, which has a time timestamp in milliseconds. Maybe a Sleep(1) would guarantee unique timestamps, preventing reordering.)

The existing use of WM_KEYDOWN for WSL navigation keys isn't quite right, as I commented in the code. Example: If I run a foreground SSH server somehow, then hold down Ctrl, while a remote user logs in and runs WSL mc, I expect the navigation keys to break. I think it can be fixed by enabling winpty's desktop backgrounding for all Windows versions, not just XP and Vista.

As a compromise, maybe a winpty user can indicate that they're using winpty in the foreground, and then winpty can use WM_KEYDOWN for Ctrl-C. It could work for IDEs, I think. It's a race, because winpty's WM_KEYDOWN will be misinterpreted if the user lifts Ctrl too quickly. (If winpty uses PostMessage, then it will be misinterpreted as an ordinary 'C' keypress. If it uses SendMessage, then it'll produce an unusual key event where wVirtualKeyCode is 'C' but UnicodeChar is NUL.)

rprichard avatar May 19 '17 08:05 rprichard

Whoa, that's a lot to process. Will do during spare time. One more point: there is Server Core. There's a lot of desktop components missing there. Actually, I use winpty on my Windows 10 developer desktop and sometimes launch something on headless Windows Server 2016 Core servers using Cygwin + OpenSSH. I'm aware of SSH transport for PowerShell, but it's not quite usable for me now, that's why I rely on winpty.

unshare avatar May 19 '17 09:05 unshare

Hi,

I hope this helps..

I tried to winpty use in GitSCM (bash on MINGW32) and it works well. winpty emacs -nw Except that Ctrl-C produces a instead of Ctrl-C (Any help to make it recognizable as Ctrl-C? )

$ winpty --version winpty version 0.2.2-dev

$ winpty winpty-agent.exe --show-input

Press any keys -- Ctrl-D exits focus: gained key: dn rpt=1 scn=70 CANCEL ch=0x3 key: up rpt=1 scn=70 CANCEL ch=0x3 key: dn rpt=1 scn=29 LCtrl-CONTROL ch=0 key: dn rpt=1 scn=44 LCtrl-Z ch=0x1a key: up rpt=1 scn=44 LCtrl-Z ch=0x1a key: up rpt=1 scn=29 CONTROL ch=0

johnsonc avatar May 24 '17 18:05 johnsonc

I fixed this here - my understanding of it is unprocessed mode needs both the GenerateConsoleCtrlEvent and the keypress to work. I've only tested this in the VSCode integrated terminal on Windows 10 though, I'm not sure about any other cases.

parkovski avatar Sep 18 '17 16:09 parkovski

I think MS is going to have to solve this one, hopefully whatever they do will work here too - https://github.com/PowerShell/Win32-OpenSSH/issues/914

parkovski avatar Oct 16 '17 20:10 parkovski

I think I figured this one out. In processed mode, GenerateConsoleCtrlEvent should be enough. In unprocessed mode, you just need to write ^C. The thing that's been messing everyone up I think is that there is a cmd.exe bug - if input is a standard console handle, it calls GetKeyboardState to decide whether to process the event. This only works if its thread has keyboard focus - e.g. the conhost window is focused - otherwise it ignores the event. What really makes this frustrating, because I want to say "just use PowerShell", but lots of programs use a batch file as kind of a pseudo-symlink, and cmd displays the 'Stop batch job' prompt where this issue shows up.

I consider this a cmd.exe bug and not a bug in winpty, node-pty, or any dependent project, and I'm not sure if there's a feasible workaround in all cases, but if there is I think it's worth implementing. I'm not familiar with the restrictions on background desktops - does anyone know if it's possible to AttachThreadInput from the agent to the conhost process, and set the appropriate state when generating the event?

parkovski avatar Dec 19 '17 17:12 parkovski

As @parkovski mentioned it seems to be fixed in the PowerShell Integrated Console but not in powershell.

If you're unsure what terminal is used you can see it in the terminal drop-down menu.

2018-01-29_110921

You can launch the integrated console without opening a ps1 file from the command panel. Press CTRL+P and enter: PowerShell: Show Integrated Console

vscode_powershell_show_integrated

vscode_powershell_show_integrated_terminal

Unfortunately, the problem still persists in the powershell terminal interface...

vscode_powershell_ctrl_c

PowerShell Version

PS C:\Users\glenn\OneDrive\Projects\Learning\docker> $PSVersionTable

Name                           Value
----                           -----
PSVersion                      5.1.16299.98
PSEdition                      Desktop
PSCompatibleVersions           {1.0, 2.0, 3.0, 4.0...}
BuildVersion                   10.0.16299.98
CLRVersion                     4.0.30319.42000
WSManStackVersion              3.0
PSRemotingProtocolVersion      2.3
SerializationVersion           1.1.0.1

vscode version

Version 1.19.3
Commit 7c4205b5c6e52a53b81c69d2b2dc8a627abaa0ba
Date 2018-01-25T10:36:34.867Z
Shell 1.7.9
Renderer 58.0.3029.110
Node 7.9.0
Architecture x64

Cheers, Glenn

glego avatar Jan 29 '18 10:01 glego

Quickly skimming over the PowerShell editor services libraries, it looks like that's because the integrated console handles ^C processing differently.

parkovski avatar Jan 29 '18 20:01 parkovski

Hey @rprichard! Is 0.44 far off? There's a bunch of us hanging for the fix to be included in our favorite win-pty projects 🙂

mikemaccana avatar Feb 16 '18 15:02 mikemaccana

I'd like to add one passing note -- I think it's an additional symptom of the problem being discussed here.

I'm running git bash under Windows 10 via Mingw64 terminal window. I want the git editor to be emacs -nw. However, I can't run it directly (as it reports that "standard input is not tty"). The partial solution is to preface it with winpty. However, emacs command including Ctrl-C don't work (and as exit is C-x, C-c that can become irritating quickly). Instead, emacs reports it received the command C-x, <pause>. So, clearly C-x isn't propagating to emacs in a form that emacs recognizes.

SeanCurtis-Roblox avatar Jan 20 '22 13:01 SeanCurtis-Roblox