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

Deadlock when pasting >5 KiB into an application that doesn't read it fast enough #17384

Open
godlygeek opened this issue Jun 6, 2024 · 7 comments
Labels
Issue-Bug It either shouldn't be doing this or needs an investigation. Needs-Attention The core contributors need to come back around and look at this ASAP. Needs-Tag-Fix Doesn't match tag requirements Product-Terminal The new Windows Terminal.

Comments

@godlygeek
Copy link

godlygeek commented Jun 6, 2024

Windows Terminal version

1.20.11381.0

Windows build number

10.0.19045.4412

Other Software

WSL

Steps to reproduce

  1. Open a WSL tab under Windows Terminal (I've reproduced with Ubuntu 20.04 and RHEL 7 distros, though I assume this bug is independent of the chosen distro)
  2. Paste this command to your shell, but don't run it yet:
    python3 -c 'print("Paste now:"); import time; time.sleep(10)'
  3. Copy the text of Frankenstein to your clipboard
  4. Run the python3 command you pasted in step 2.
  5. When the "Paste now:" message appears, paste the text of Frankenstein to your terminal, and say "paste anyway" at the dialog box.
  6. The terminal deadlocks. The first 4096 bytes of Frankenstein appear in the terminal. It is no longer possible to Ctrl-C the python3 script.

Expected Behavior

The terminal should remain responsive. Ctrl-C should work. The full pasted text should be displayed.

Actual Behavior

Deadlock. I assume that what's happening here is that Windows Terminal is making blocking write() calls in its paste implementation. If the application doesn't read what's been pasted, or can't keep up, the terminal hangs and becomes totally unresponsive. This was discovered while investigating an issue with the Python 3.13 REPL, but it appears to be a bug in Windows Terminal itself.

@godlygeek godlygeek added Issue-Bug It either shouldn't be doing this or needs an investigation. Needs-Triage It's a new issue that the core contributor team needs to triage at the next triage meeting labels Jun 6, 2024
@carlos-zamora carlos-zamora added Product-Terminal The new Windows Terminal. and removed Needs-Triage It's a new issue that the core contributor team needs to triage at the next triage meeting labels Jun 12, 2024
@carlos-zamora carlos-zamora added this to the Terminal v1.22 milestone Jun 12, 2024
@microsoft-github-policy-service microsoft-github-policy-service bot added Needs-Tag-Fix Doesn't match tag requirements labels Jun 12, 2024
@lhecker
Copy link
Member

lhecker commented Jun 13, 2024

This seems to happen in conhost/OpenConsole as well. It may be easier to use that first to investigate the issue.

@Spinestars
Copy link

In the vim editor, the same is true

@lhecker
Copy link
Member

lhecker commented Aug 8, 2024

Sorry for the delay. I've debugged this issue now and found that WSL simply stops reading from our input buffer in these situations. As such this is almost assuredly a deadlock in WSL. The default pipe size on Windows is 4KiB and I suspect WSL is waiting for its input pipe to be drained by the shell/application.

@lhecker lhecker removed their assignment Aug 22, 2024
@lhecker lhecker added the Needs-Triage It's a new issue that the core contributor team needs to triage at the next triage meeting label Aug 22, 2024
@carlos-zamora carlos-zamora added the Needs-Attention The core contributors need to come back around and look at this ASAP. label Sep 4, 2024
@carlos-zamora
Copy link
Member

@lhecker said he'd look into this a bit. So we put Needs-Attention on it and we'll circle back when he has more info :)

@lhecker
Copy link
Member

lhecker commented Sep 25, 2024

I've realized that the best way to prove that this isn't an issue with either Windows Terminal, conhost, or ConPTY is by testing this issue in https://github.com/mintty/wsltty (which relies on none of those) and it still occurs there.

I'm not 100% certain if this means it's an issue in Python or WSL though as I'm not super familiar with either. But it does show that the issue is not caused by us, from what I can tell.

Let me know if you have any questions! Otherwise, the bot will close the issue in a week. 🙂

@lhecker lhecker added Needs-Author-Feedback The original author of the issue/PR needs to come back and respond to something and removed Needs-Triage It's a new issue that the core contributor team needs to triage at the next triage meeting Needs-Attention The core contributors need to come back around and look at this ASAP. Needs-Tag-Fix Doesn't match tag requirements labels Sep 25, 2024
@microsoft-github-policy-service microsoft-github-policy-service bot added the Needs-Tag-Fix Doesn't match tag requirements label Sep 25, 2024
@godlygeek
Copy link
Author

I disagree, there's an issue here that's different from what's seen with wsltty. You're right that pasting Frankenstein does cause wsltty to hang as well, but it doesn't deadlock. You can press ctrl-c, and it recovers (albeit some of the clipboard contents wind up being consumed by the shell). This is different from what we see with Windows Terminal: it hangs and can't recover. It's unable to send a signal to the process, and deadlocks such that it must be killed.

@microsoft-github-policy-service microsoft-github-policy-service bot added Needs-Attention The core contributors need to come back around and look at this ASAP. and removed Needs-Author-Feedback The original author of the issue/PR needs to come back and respond to something labels Sep 25, 2024
@lhecker
Copy link
Member

lhecker commented Sep 25, 2024

Thank you for saying that! I didn't realize that if wsltty doesn't read past 4KiB it may still have a functional Ctrl-C. I thought the two issues are one and the same.

So, I spent some more time investigating this. WSL (under Windows Terminal) sets the input mode to 0x03d8 via SetConsoleMode. Decoded this means:

0x0008 ENABLE_WINDOW_INPUT
0x0010 ENABLE_MOUSE_INPUT
0x0040 ENABLE_QUICK_EDIT_MODE
0x0080 ENABLE_EXTENDED_FLAGS
0x0100 ENABLE_AUTO_POSITION
0x0200 ENABLE_VIRTUAL_TERMINAL_INPUT

Without ENABLE_PROCESSED_INPUT (= 0x0001) we will not translate Ctrl-C (= '\x03') to a CTRL_C_EVENT for SetConsoleCtrlHandler (= Windows' closest equivalent for SIGINT). The official WSL bridge is waiting for the UNIX to read from stdin. As long as it's blocked there, it won't read stdin from the terminal either. This means it won't ever see the '\x03' that is at the end of the stdin buffer.

I tested whether this is a bug in WSL where it does have a SetConsoleCtrlHandler set up but forgot to set ENABLE_PROCESSED_INPUT by forcing the flag inside conhost / the terminal. That causes WSL to exit which means it doesn't have SetConsoleCtrlHandler set up.

You could argue that we should behave differently (please let me know if you have any suggestions!), but it seems like we are behaving "to spec". WSL asks for no input preprocessing and so we don't handle Ctrl-C. I believe the reason this works with wsltty is because it doesn't use the official WSL bridge, but rather https://github.com/Biswa96/wslbridge2 which is probably more robust. From what I can tell, the official WSL bridge needs to be fixed here.

@lhecker lhecker removed their assignment Oct 8, 2024
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Issue-Bug It either shouldn't be doing this or needs an investigation. Needs-Attention The core contributors need to come back around and look at this ASAP. Needs-Tag-Fix Doesn't match tag requirements Product-Terminal The new Windows Terminal.
Projects
None yet
Development

No branches or pull requests

4 participants