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

Windows: stderr output leaks into stdout #83

Open
qweeah opened this issue Nov 24, 2023 · 3 comments
Open

Windows: stderr output leaks into stdout #83

qweeah opened this issue Nov 24, 2023 · 3 comments

Comments

@qweeah
Copy link

qweeah commented Nov 24, 2023

I found that on Windows, if a console is generated from stderr, output sent towards the console will go into stdout with extra warning like failed to get console mode for stdout: The handle is invalid.(resovled in 7b7885a).

Verified this with a simple test program in here which simply outputs three messages

// 1. generate console from stderr
c, err := console.ConsoleFromFile(os.Stderr)
if err != nil {
fmt.Println("failed to generate console from stderr", err)
os.Exit(1)
}

// 2. output to stderr, stdout and console
fmt.Fprintln(os.Stderr, "via os.Stderr")
fmt.Fprintln(os.Stdout, "via os.Stdout")
fmt.Fprintln(c, "via stderr console")

In Powershell, the output is like:

> $a=.\console.exe
via os.Stderr     
> $a
failed to get console mode for stdout: The handle is invalid.
via os.Stdout
via stderr console    

The via stderr console log goes wrongly into the variable $a (which should only contain the command's success output stream and stdout output).

As a comparison, below is the behavior of the test program on Zsh(Ubuntu 20.04)

> a=$(./console)
via os.Stderr
via stderr console
> echo $a
via os.Stdout

We can see tht the via stderr console doesn't goes into the stdout output as expected.

@qweeah
Copy link
Author

qweeah commented Nov 24, 2023

BTW I am using github.com/containerd/console v1.0.3

@TBBle
Copy link

TBBle commented Nov 15, 2024

We just noticed in containerd/nerdctl#3629 that this is breaking buildctl's ability to output built images to stdout, e.g., piping to docker load with tty-style progress because the progress writer output is mixed into the image tar stream, even though it was created with console.ConsoleFromFile(os.stderr) (os.stderr is here).

The core problem is that newMaster's Windows implementation does not store f after validating that it is one of stdin, stdout, or stderr, in contrast to the Unix implementation, and so where on Unix, the provided f is used in, e.g., (m *master) Write, on Windows it's unconditionally writing to stdout.

I haven't checked the commits to see if there was a reason it was done this way, but maybe it's as simple as making the Windows version work more like the Linux version by holding and using the f passed to newMaster, or it's as complicated as implementing the Windows ConPTY API to match the use of openpt() and friends in the Unix version.

@thaJeztah
Copy link
Member

Looks like 7a61819

did that, but based on a previously conditional "if windows"; commit before that apparently reverted a change that (from a quick read) did preserve what was passed; #29

(related to containerd/containerd#2560 ?)

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

No branches or pull requests

3 participants