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

Preview: Inconsistent Behavior When Writing to Stdout While Reading From Stdin #18265

Open
CannibalVox opened this issue Dec 1, 2024 · 11 comments
Labels
Area-CookedRead The cmd.exe COOKED_READ handling Issue-Bug It either shouldn't be doing this or needs an investigation. Needs-Tag-Fix Doesn't match tag requirements Product-Terminal The new Windows Terminal.

Comments

@CannibalVox
Copy link

Windows Terminal version

1.22.3232.0

Windows build number

10.0.19045.5131

Other Software

WSL seems to work fine, but other hosted terminals have this issue. To be clear, this only happens with the preview version of WT. The stable version works fine.

Steps to reproduce

Create any program that begins reading from stdin and then after reading has begun, write to stdout. Here is an example program in golang:

package main

import (
	"bufio"
	"fmt"
	"os"
	"os/signal"
	"syscall"
	"time"
)

func DoScan() {
	scanner := bufio.NewScanner(os.Stdin)
	scanner.Split(bufio.ScanRunes)

	for scanner.Scan() {
		fmt.Print(scanner.Text())
	}
}

func main() {

	go DoScan()
	time.Sleep(1)

	fmt.Print("Here is a prompt! >")

	sigs := make(chan os.Signal, 1)
	signal.Notify(sigs, syscall.SIGINT, syscall.SIGTERM)

	<-sigs
}

Expected Behavior

I expected that my typed keys would appear after the caret of the prompt output. That's what it does in the stable version of WT and that's what it does in WSL even in the preview version.

Actual Behavior

My typed keys appear at the beginning of the line, overwriting the prompt.

@CannibalVox CannibalVox 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 Dec 1, 2024
@carlos-zamora carlos-zamora added Product-Terminal The new Windows Terminal. Area-CookedRead The cmd.exe COOKED_READ handling and removed Needs-Triage It's a new issue that the core contributor team needs to triage at the next triage meeting labels Dec 11, 2024
@microsoft-github-policy-service microsoft-github-policy-service bot added the Needs-Tag-Fix Doesn't match tag requirements label Dec 11, 2024
@carlos-zamora
Copy link
Member

Thanks for filing! Though this worked on the stable version, we recommend against your scenario "reading from stdin and then after reading has begun, write to stdout". It's not something that can be guaranteed to work if you're using line buffered stdin (the default behavior; with the other modes being "unbuffered stdin" aka "raw mode").

We're also missing a real world use case. What's an application that's doing this today and is impacted by the change? With that, we can reconsider working on this.

As such, closing as not planned.

@carlos-zamora carlos-zamora closed this as not planned Won't fix, can't repro, duplicate, stale Dec 11, 2024
@CannibalVox
Copy link
Author

We're also missing a real world use case. What's an application that's doing this today and is impacted by the change? With that, we can reconsider working on this.

It's literally just reading from stdin and writing to stdout. You're saying that I need to stop reading from stdin every time I write to stdout on windows terminal?!?!

There are also issues in raw mode, not just line reading mode. The cursor will do this funky thing where it jiggles back and forth when you echo characters to the terminal.

@CannibalVox
Copy link
Author

Doesn't this pretty badly undermine the utility of conpty considering that this behavior works on every other terminal and also works on WT if I read from the old console libraries instead of the pty?

@CannibalVox
Copy link
Author

Like I feel like there's been a miscommunication here, because the go programming language doesn't even have the ability to cancel a read from stdin so what you are recommending isn't even possible on any application written in that language.

@DHowett DHowett reopened this Dec 12, 2024
@DHowett DHowett marked this as a duplicate of #18319 Dec 12, 2024
@DHowett
Copy link
Member

DHowett commented Dec 12, 2024

Description of the new feature

In #18265 @carlos-zamora informed me that while operating the terminal as full duplex (reading from stdin and writing to stdout at the same time) does work in the stable version of Windows Terminal, Microsoft did not recommend doing so, so a bug with full duplex operation was not actually a bug.

Given that the last half-duplex terminal to exist was decommissioned sometime in the 1990s, however, I suggest that Microsoft undergo a project to add full duplex operation to Windows Terminal. And bring it into line with similar products. While Carlos did not believe there were any known use cases for a full duplex terminal, I'm inclined to disagree. While I can't speak for anyone else, the Go programming language does not have the capability to cancel reading from stdin, meaning that no application written in Go is capable of half-duplex operations. I believe that adding Full Duplex capability to Windows Terminal would be a major boon for Go developers, especially since we all thought Windows Terminal already had it.

Proposed technical implementation details

I'm not clear on the technical specifics, but probably do whatever the current version of Windows Terminal does, since Full Duplex works in that version.

@lhecker
Copy link
Member

lhecker commented Dec 12, 2024

Carlos' comment above is the result of a fairly lengthy discussion internally that day. I also apologize if the shortened response we wrote together seemed like we tried to smother this discussion. That wasn't our intention.

The default mode of input for a terminal is "line-oriented", "line-buffered", or how Windows calls it: "cooked". This means that only when you press Enter, the input is flushed and sent to the application. Until then, the application will not see the input.

It also has the "echo mode" enabled, which means that the terminal driver sends every input character immediately back to the terminal, before the application even sees that input.

So, just to be clear, as I'm not sure if you did this intentionally, the following code does not disable line-oriented input. All it does is change the way the scanner splits the input. It's still only flushed to your Go application when you press enter:

scanner.Split(bufio.ScanRunes)

That aside, from a technical POV what you're doing isn't strictly correct on any operating system, UNIX included. This is because the echo mode you left enabled returns input to the terminal "immediately" while your application will see that input only asynchronously. They're not in sync. If your application lags, your output will be mixed with the already echoed output by the terminal driver, no matter what OS you run your application on.

Windows is a special case. Its line-oriented input is more akin to the GNU readline() function, because it offers "advanced" input functionality, like command history, word-wise cursor navigation, pagination, and so on. And just like readline(), it does not work when the application prints text after the readline call (the line-oriented read) was initiated, because in order to have these features it must make more assumptions (namely: the cursor does not move after the read started). Have you ever wondered why you can't backspace onto a previous line in line-oriented reads under UNIX while it works on Windows? This is why.

In short: Your code is technically incorrect on any OS, but it'll be a very subtle and rare defect on UNIX, while it is an obvious issue on Windows, because our line-oriented input is more feature-rich, like readline().

The issue you have can be hot-fixed, but the problem is that it only fixes your exact, specific issue. If you change your repro to, for instance, continuously print text, it'll be broken again. That's why we asked what scenario exactly your issue is about, because we wanted to estimate whether it's worth hot-fixing it, despite the hot-fix not actually fixing the overarching issue.

Lastly, if you want character-by-character input you can disable line-oriented input and switch stdin into the "raw mode" (mentioned previously). This causes the terminal to immediately send you each character that's typed in and lets you do the full-duplex stdin/stdout that you want. For it to properly work though, you'd have to disable the "echo mode" as well, because of the race condition mentioned above. Coincidentally, enabling raw mode and disabling echo mode is exactly how most shells work, with the exception of cmd.exe.

(Edit: I saw you mention that the cursor jiggles. We've seen that reported for cygwin before and very specific zsh extensions under WSL, but we haven't seen a good repro yet that would help us investigate this. If you have a simple repo, please let us know!)

@CannibalVox
Copy link
Author

No part of your response really addresses the issue at all, so I feel like I maybe haven't made myself clear. The problem is not that my echo is being interleaved with the output. The problem is that after the output has been written, if I provide any input, the cursor will go back to the beginning of the line instead of my echo beginning where the cursor was left as the text is written.

Running the program will make what I'm talking about abundantly clear, and will make obvious that this is not the issue you are describing. The following steps will reproduce the issue:

  1. Begin reading from stdin.
  2. Write text to stdout but don't include a newline. The cursor will be at the end of the written content.
  3. Start typing letters. Again, note that this does not begin until AFTER the stdout text has been fully written. This is not a synchronization issue between the echo text and the written text, because they are not happening at the same time.

What will occur is that the cursor will return to the beginning of the line where the prompt was written for no apparent reason.

The "jiggling" issue will happen if you do the above program with raw mode on and start echoing text to the terminal manually. When you write for instance BS (\x08) to to the output stream, the cursor will not step back a single position immediately, it will return to the beginning of the line and then snap back to where it's supposed to be.

@CannibalVox
Copy link
Author

Here's a loom video so you can see the program in action: https://www.loom.com/share/b79fe88bbef04862869f8c5a9f69cd11?sid=d0984f9e-3309-4ce0-a9a3-612e3ac4f9cc

@lhecker
Copy link
Member

lhecker commented Dec 13, 2024

The problem is not that my echo is being interleaved with the output. The problem is that after the output has been written, if I provide any input, the cursor will go back to the beginning of the line instead of my echo beginning where the cursor was left as the text is written.

But I wrote about more than just the echo mode. I wrote multiple paragraphs about how Windows' line-oriented input is more akin to GNU readline, that I thought would apply very well. If you tell me why those don't apply to your issue, I'd be able to write a better response, or perhaps come to a different conclusion for myself.

This code should demonstrate what I meant:

package main

import (
	"fmt"
	"time"
)

/*
#cgo LDFLAGS: -lreadline
#include <stdio.h>
#include <readline/readline.h>
*/
import "C"

func main() {
	go func() {
		time.Sleep(50 * time.Millisecond)
		fmt.Printf("Here is a prompt! >")
	}()
	C.readline(C.CString(""))
}

This is how it looks like:

qNerYpNDRyeNqXQ2.mp4

I only wrote the paragraphs about interleaved output, because "writing to stdout while reading from stdin" has 2 issues, and not just the one that you reported. If I fix the one you found, your code would still be incorrect, and I'd still strongly recommend not writing to stdout while reading from line-oriented stdin.

The "jiggling" issue will happen if you do the above program with raw mode on and start echoing text to the terminal manually. When you write for instance BS (\x08) to to the output stream, the cursor will not step back a single position immediately, it will return to the beginning of the line and then snap back to where it's supposed to be.

Are you compiling the application with native Go for Windows (i.e. not MSYS/Cygwin)? I tried this code, and I don't see any jitter:

package main

import (
	// ... same import ...
	"golang.org/x/sys/windows"
)

func DoScan() {
	scanner := bufio.NewScanner(os.Stdin)
	scanner.Split(bufio.ScanRunes)

	for scanner.Scan() {
		text := scanner.Text()
		if text == "\b" {
			text = "\b \b"
		}
		fmt.Print(text)
	}
}

func main() {
	handle := windows.Handle(os.Stdin.Fd())
	var originalMode uint32
	windows.GetConsoleMode(handle, &originalMode)
	rawMode := originalMode &^ (windows.ENABLE_ECHO_INPUT | windows.ENABLE_LINE_INPUT)
	windows.SetConsoleMode(handle, rawMode)
	defer windows.SetConsoleMode(handle, originalMode)

	// ... same code as in your example ...
}

I've seen the jitter in screen recordings before, and I do want to fix it ASAP, but I can't debug it without having the issue myself. 🥲


Lastly, I wanted to pick up something again that I said before:

The issue you have can be hot-fixed, but the problem is that it only fixes your exact, specific issue. If you change your repro to, for instance, continuously print text, it'll be broken again. That's why we asked what scenario exactly your issue is about, because we wanted to estimate whether it's worth hot-fixing it, despite the hot-fix not actually fixing the overarching issue.

Continuously printing text while reading from a line-oriented stdin is the main scenario I can imagine for why someone would print to stdout after starting the read from stdin. Like, when printing log output and you want to offer the user an interactive input prompt. However, fixing your issue (= printing exactly 1 time, after starting the read) won't fix that or other scenarios at all. That's the primary reason why I hesitate to hotfix it. Outside of the issue description itself, can you say why this should be hotfixed (= moving the issue goal posts)?

@CannibalVox
Copy link
Author

I understand what you're saying now, and I'm sorry for getting snotty earlier.

Regarding the jiggling issue: I was able to repro it with your code in VSCode before but now I can't. I'll keep an eye out for it as a separate issue.

To be clear about this, go does not offer an input method equivalent to C.readLine, so the following code will also demonstrate the issue:

package main

import (
	"fmt"
	"time"
)

func main() {
	go func() {
		time.Sleep(50 * time.Millisecond)
		fmt.Printf("Here is a prompt! >")
	}()

	var text string 
	fmt.Scanln(&text)
}

The bug doesn't require that stdin is read continuously, just that output is printed while stdin is being read.

Continuously printing text while reading from a line-oriented stdin is the main scenario I can imagine for why someone would print to stdout after starting the read from stdin. Like, when printing log output and you want to offer the user an interactive input prompt. However, fixing your issue (= printing exactly 1 time, after starting the read) won't fix that or other scenarios at all.

That wouldn't even fix my issue, that would just fix the code snippet I made to demonstrate my issue. What I don't agree is with the word "continuous". "Periodically" might be a better word to use. I type something, and then output is printed, and then I type something else. Maybe Go is the only language with this problem, though.

@DHowett
Copy link
Member

DHowett commented Dec 13, 2024

Hey, we're sorry too. We judged this bug as a case of "holding it wrong", when it should have been clear it wasn't1.

This is definitely something we should look at. Zooming out a bit, it may be the root cause of #18080 as well--making Go not the only language impacted!--and a couple other bugs I can't name at the moment.

We've been trying to modernize the console subsystem for a while now, and that means that seven years in we're getting deeper into the behavior model; this almost certainly regressed when we rewrote the cooked read ("readline"-like) implementation back in #17455.

Thanks for your persistence, and for leading us in the right direction!

Footnotes

  1. As the owners of the console subsystem, we get a lot of bug reports that lie somewhere between "holding it wrong" and "Hyrum's law". This wasn't one of them, but we triaged it like it was.

carlos-zamora pushed a commit that referenced this issue Jan 27, 2025
)

As explained in the comment on `_getViewportCursorPosition`, printing
to stdout after initiating a cooked stdin reads is a race condition
between the application and the terminal. But we can significantly
reduce the likelihood of this being obvious with this change.

Related to #18265
Possibly related to #18081

## Validation Steps Performed

Execute the following Go code and start typing:
```go
package main

import (
	"fmt"
	"time"
)

func main() {
	go func() {
		time.Sleep(50 * time.Millisecond)
		fmt.Printf("Here is a prompt! >")
	}()

	var text string
	fmt.Scanln(&text)
}
```

Without this change the prompt will disappear,
and with this change in place, it'll work as expected. ✅
DHowett pushed a commit that referenced this issue Jan 28, 2025
)

As explained in the comment on `_getViewportCursorPosition`, printing
to stdout after initiating a cooked stdin reads is a race condition
between the application and the terminal. But we can significantly
reduce the likelihood of this being obvious with this change.

Related to #18265
Possibly related to #18081

## Validation Steps Performed

Execute the following Go code and start typing:
```go
package main

import (
	"fmt"
	"time"
)

func main() {
	go func() {
		time.Sleep(50 * time.Millisecond)
		fmt.Printf("Here is a prompt! >")
	}()

	var text string
	fmt.Scanln(&text)
}
```

Without this change the prompt will disappear,
and with this change in place, it'll work as expected. ✅

(cherry picked from commit 1040035)
Service-Card-Id: PVTI_lADOAF3p4s4AmhmQzgWppag
Service-Version: 1.22
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Area-CookedRead The cmd.exe COOKED_READ handling Issue-Bug It either shouldn't be doing this or needs an investigation. 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