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

console redirection is broken #22314

Closed
jkotas opened this issue Jun 15, 2017 · 14 comments · Fixed by #94414
Closed

console redirection is broken #22314

jkotas opened this issue Jun 15, 2017 · 14 comments · Fixed by #94414
Assignees
Milestone

Comments

@jkotas
Copy link
Member

jkotas commented Jun 15, 2017

From @Spongman on June 15, 2017 21:5

using System;
class Test {
  public static void Main () {
    while(true)
      Console.WriteLine(Console.ReadLine());
  }
}

stdout redirection:

test command: dotnet run > test.txt
expected behavior is to show typed text in the console/tty, and to write the typed lines out to test.txt

on windows: works fine.
on linux: writes stdin to the file ok, but the typed text is not echoed to tty.

stdin redirection:

test command: dotnet run < test.txt (test.txt exists and contains some lines of text)
expected behavior is to write the contents of test.txt to the console/tty.

on both windows & linux: console clears and the CPU is pegged!!

version: 1.0.4

Microsoft .NET Core Shared Framework Host

  Version  : 1.1.0
  Build    : 928f77c4bc3f49d892459992fb6e1d5542cb5e86

Copied from original issue: dotnet/coreclr#12308

@Spongman
Copy link

ugh, please forget the 'stdin redirection' part.

@joshfree
Copy link
Member

joshfree commented Apr 1, 2018

@msftgits msftgits transferred this issue from dotnet/corefx Jan 31, 2020
@msftgits msftgits added this to the Future milestone Jan 31, 2020
@maryamariyan maryamariyan added the untriaged New issue has not been triaged by the area owner label Feb 23, 2020
@carlossanlop
Copy link
Member

Triage:

on linux: writes stdin to the file ok, but the typed text is not echoed to tty.
@eiriktsarpalis and I were able to reproduce this issue. @Spongman would you be interested in investigating this issue further?

@carlossanlop carlossanlop removed the untriaged New issue has not been triaged by the area owner label Mar 4, 2020
@cfbao
Copy link

cfbao commented May 29, 2023

To add some more details:

For stdout redirection, not only is the input not displayed, the output isn't fine either - it contains the user input + lines written by Console.WriteLine().
I.e. for such a program

Console.WriteLine(Console.ReadLine());

If the user enters "abc" when prompted, it'd behave like this

> ./test > output
> cat output
abc
abc

More context on this issue's impact:

We're rewriting a CLI app from Go to C#, and it's a requirement that user input isn't written to stdout, because one main use case for this app is that user will run it inside $(...), answer a few interactive prompts, and the app will output commands that are directly executed. Having stdin redirected to stdout breaks the output commands, and causes errors.

@cfbao
Copy link

cfbao commented May 29, 2023

In case this issue won't be fixed soon, any comment on this potential workaround on Linux & Mac?

var stdinReader= new StreamReader(new FileStream(
    new Microsoft.Win32.SafeHandles.SafeFileHandle(0, ownsHandle: false),
    FileAccess.Read
));
stdinReader.ReadLine();

Should ownsHandle be true or false in this case? Or maybe it doesn't matter if stdinReader lives for the entire lifetime of the app?

@adamsitnik
Copy link
Member

Should ownsHandle be true or false in this case?

ownsHandle tells us whether the handle should be released when it's disposed (in explicit way) and/or finalized (when it's no longer used by anyone and GC collects memory). In case of handle to standard input, it should rather be false (it will be released by the OS, when the app exits), so other parts of the code that can potentially use it won't report an exception when they try to.

cfbao added a commit to Brightspace/bmx that referenced this issue May 30, 2023
.NET incorrectly copies user's console input into stdout on Unix-y
platforms, which messes up `bmx print`, so we have to work around it.
See dotnet/runtime#22314
@cfbao
Copy link

cfbao commented Jun 2, 2023

I might have an idea of what's going on with user terminal input when stdout is redirected.

When Console.ReadLine() is called and stdin isn't redirected, the runtime calls StdInReader.ReadLineCore, which first sets the terminal to a sort of raw mode (so every key stroke is captured & user input isn't directly displayed on screen):

termios.c_lflag &= (uint32_t)(~(ECHO | ICANON | IEXTEN));

then read input byte-by-byte, and prints the characters back out using Console.Write:

This of course means that user input will end up in stdout (redirected or not) and not on the terminal (when stdout is redirected).

It's a similar process for other read methods like Console.ReadKey.

Might be a naive suggestion, but can the runtime prints user input back out to /dev/tty instead of stdout to resolve the first part of this issue?

@cfbao
Copy link

cfbao commented Jun 5, 2023

Just noticed that the "user input ending up in stdout" problem can occur on Windows too, though to a much smaller extent - only when Console.ReadKey is called.
The problematic code in the runtime:

if (!intercept)
Console.Write(ir.keyEvent.uChar);

@tmds
Copy link
Member

tmds commented Jun 6, 2023

This of course means that user input will end up in stdout (redirected or not) and not on the terminal (when stdout is redirected).

That's right. These writes are are meant to emulate a terminal ECHO. When the output is redirected, we shouldn't write these characters.

@cfbao do you want to make a PR that implements that change?

@cfbao
Copy link

cfbao commented Jun 9, 2023

That's right. These writes are are meant to emulate a terminal ECHO. When the output is redirected, we shouldn't write these characters.

To confirm - When stdout is redirected, I believe users would still expect whatever they type on the terminal to appear on screen (just not end up in stdout), so do we want to write their input to /dev/tty instead? (or maybe a better approach that I'm not aware of)

@cfbao do you want to make a PR that implements that change?

I'm interested, but will probably need some handholding 😅

@tmds
Copy link
Member

tmds commented Jun 10, 2023

I believe users would still expect whatever they type on the terminal to appear on screen (just not end up in stdout), so do we want to write their input to /dev/tty instead?

Yes, that is desired.

I'm interested, but will probably need some handholding

If you want to have a go at it, I'll assist you.

The documentation here explains how you can build and test on your machine.

@krwq
Copy link
Member

krwq commented Sep 11, 2023

@cfbao would be much appreciated to get your help 😄

@tmds
Copy link
Member

tmds commented Sep 11, 2023

@cfbao feel free to pick this up, and I'll provide guidance.
Otherwise, I can look into this in October or so.

@ghost ghost added the in-pr There is an active PR which will close this issue when it is merged label Nov 6, 2023
@tmds
Copy link
Member

tmds commented Nov 6, 2023

I've made a PR that addresses the stdout redirection issue: #94414.

The stdin redirection issue is not a real issue. The problem is that the while loop doesn't quit when ReadLine returns null.

@adamsitnik adamsitnik modified the milestones: Future, 9.0.0 Nov 14, 2023
@ghost ghost removed the in-pr There is an active PR which will close this issue when it is merged label Nov 16, 2023
@github-actions github-actions bot locked and limited conversation to collaborators Dec 17, 2023
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Projects
None yet
10 participants