Runtime: System.Console.ReadKey() returning Ctrl+J for ENTER on macOS

[TylerLeonhardt]

Description

See the following code:

using System;

var key = Console.ReadKey(true);
Console.WriteLine("KEY: " + key.Key);
Console.WriteLine("MODS: " + key.Modifiers);

Run it with dotnet run and hit ENTER.

NOTE: You may need to SPAM hitting enter as I see this Ctrl+J behavior _only_ when the process is under strain...

Configuration

• Which version of .NET is the code running on? .NET 5 RC1
• What OS and version, and what distro if applicable? macOS Catalina
• What is the architecture (x64, x86, ARM, ARM64)? x64
• Do you know whether it is specific to that configuration? No. It could be others. Not sure.

Regression?

Yes this worked before. It worked in .NET 5.0 preview 7. Then a similar bug showed up in .NET 5.0 preview 8 (except it was Ctrl+Enter instead of Ctrl+J)...

Other information

This was discovered in PowerShellEditorServices. The end result is the user hit's ENTER in the PowerShell Integrated Console and the console does nothing making the console useless.

[msftbot[bot]]

Tagging subscribers to this area: @eiriktsarpalis, @jeffhandley
See info in area-owners.md if you want to be subscribed.

[wfurt]

cc: @tmds @adamsitnik

[jeffhandley]

@eiriktsarpalis Is it possible that this has the same root cause as #42333 and was therefore fixed by #42371 (and #42381)?

[eiriktsarpalis]

@jeffhandley Sounds like it is.

@TylerLeonhardt I couldn't reproduce the issue as reported, just to confirm, does your code include calls to Console.KeyAvailable before Console.Readkey()?

[TylerLeonhardt]

In the actual code, yes. Due to the blocking nature of ReadKey, we have to use essentially a loop of Console.KeyAvailable... And then once a key is available, we run ReadKey.

With that said, I can repro this issue consistently with the steps I've provided above which don't use Console.KeyAvailable. - happy to get on a Teams call or something to show you the exact behavior. Whatever helps.

[eiriktsarpalis]

After discussing with @TylerLeonhardt I'm now able to reproduce the issue consistently, even after the fix in #42371 has been applied:

public static void Main(string[] args)
{
    Thread.Sleep(1000);
    var key = Console.ReadKey(true);
    Console.WriteLine("KEY:" + key.Key);
    Console.WriteLine("MODS: " + key.Modifiers);
}

If I hit the enter key before the sleep call returns, it will be reported as \n, otherwise it will be reported as \r. I believe this is happening because the keystroke is being recorded before we are able to configure the terminal, and therefore terminals that have the ICRNL flag enabled will have applied the conversion already. Therefore the following change mitigates the issue (though it doesn't eliminate it completely):

public static void Main(string[] args)
{
    _ = Console.KeyAvailable; // configures the terminal
    Thread.Sleep(1000);
    var key = Console.ReadKey(true);
    Console.WriteLine("KEY:" + key.Key);
    Console.WriteLine("MODS: " + key.Modifiers);
}

I don't believe we can address the issue, this seems to be a termios limitation. In light of this my recommendation would be to revert #37491 altogether cc @tmds @stephentoub @jeffhandley

[jeffhandley]

OK -- @eiriktsarpalis can you please provide a summary for me that lists out:

1. For our current state in RC2:
   
   
   
   1. What known issues we have
   
   
   2. What other risks exist if we ship as-is
   
   
2. If we revert #37491:
   
   
   
   1. What known issues we'll have
   
   
   2. What other risks would be introduced (for instance, we mentioned yesterday it's not a straightforward revert because other changes have gone in since #37491)
   
   

[eiriktsarpalis]

The RC2 branch currently contains a race condition on Unix where Console.ReadKey() calls will report Enter keystrokes as \n (rather than '\r') if they happen _before_ System.Console has had the chance to configure the terminal. This regresses from behaviour in .NET Core 3.1. While this is expected to impact most console apps, this should only concern cases where users type Enter _before any other console IO has taken place_.

If we revert #37491, then Console.ReadKey() calls on Unix will be unable to distinguish between \n and \r characters (which is consistent with .NET Core 3.1 behaviour). Reverting the change could also result in unanticipated breaks because of other System.Console changes that have been applied since and could depend on the new behaviour.

[tmds]

I also prefer to revert https://github.com/dotnet/runtime/pull/37491. The changes that went into Console after it are https://github.com/dotnet/runtime/pull/41317 and https://github.com/dotnet/runtime/pull/39192, and don't rely on its behavior.

[jeffhandley]

Here's the summary of the late-breaking Console issues as I understand it:

• #37491 has so far introduced the following issues:
  
  
  
  • #42333, which is fixed by #42371 and ported into 5.0 with #42381
  
  
  • #42418, for which we don't see a clear fix (other than reverting #37491)
  
  
  • By reverting #37491, we would address #42418 and possibly other unreported bugs. But we would reintroduce the part of #802 dealing with distinguishing between CR and LF inputs.
  
  
• #39206 also introduced a regression
  
  
  
  • #42423, which is fixed by #42432 and is being ported into 5.0 with #42449
  
  
  • This is independent of the regressions introduced by #37491
  
  
  • We still want to proceed with this fix
  
  

Because we're now seeing that we have a class of bugs introduced by #37491 that will not be categorically fixed, we would be choosing to accept the risk of the revert and reintroducing the known issue in #802 because that situation has fewer unknowns and would likely be easier to service if needed.

Is this all correct, @eiriktsarpalis, @tmds, and @stephentoub?

[eiriktsarpalis]

lgtm, apart from a minor correction which I edited in your post.