Skip to content

Commit

Permalink
Console.ReadKey should not throw when read returns 0 records (#98684)
Browse files Browse the repository at this point in the history
This change fixes a problem where Console.ReadKey inappropriately throws
an InvalidOperationException.

The Console.ReadKey function can throw an InvalidOperationException when
the "application does not have a console or when console input has been
redirected". That makes sense.

However, there is *another* case where it throws an
InvalidOperationException, which happens when there are multiple
processes attached to the same console, which are waiting for input, and
one dies or is killed. In this case, the native read function returns a
"success" return code, but the "out" parameter indicating the number of
records read is 0, and in this case, Console.ReadKey is throwing, but it
should not be.

Note that this behavior of the underlying platform is "almost certainly"
a bug (microsoft/terminal#15859); however, it
is longstanding behavior, so we would like to avoid blowing up apps who
have the misfortune of stumbling into this situation.

More details in the linked Issue:

Fix #88697
  • Loading branch information
jazzdelightsme authored May 22, 2024
1 parent b8f554d commit 7d8d6ac
Showing 1 changed file with 23 additions and 1 deletion.
24 changes: 23 additions & 1 deletion src/libraries/System.Console/src/System/ConsolePal.Windows.cs
Original file line number Diff line number Diff line change
Expand Up @@ -347,14 +347,36 @@ public static ConsoleKeyInfo ReadKey(bool intercept)
while (true)
{
r = Interop.Kernel32.ReadConsoleInput(InputHandle, out ir, 1, out int numEventsRead);
if (!r || numEventsRead == 0)
if (!r)
{
// This will fail when stdin is redirected from a file or pipe.
// We could theoretically call Console.Read here, but I
// think we might do some things incorrectly then.
throw new InvalidOperationException(SR.InvalidOperation_ConsoleReadKeyOnFile);
}

if (numEventsRead == 0)
{
// This can happen when there are multiple console-attached
// processes waiting for input, and another one is terminated
// while we are waiting for input.
//
// (This is "almost certainly" a bug, but behavior has been
// this way for a long time, so we should handle it:
// https://github.com/microsoft/terminal/issues/15859)
//
// (It's a rare case to have multiple console-attached
// processes waiting for input, but it can happen sometimes,
// such as when ctrl+c'ing a build process that is spawning
// tons of child processes--sometimes, due to the order in
// which processes exit, a managed shell process (like pwsh)
// might get back to the prompt and start trying to read input
// while there are still child processes getting cleaned up.)
//
// In this case, we just need to retry the read.
continue;
}

ushort keyCode = ir.keyEvent.wVirtualKeyCode;

// First check for non-keyboard events & discard them. Generally we tap into only KeyDown events and ignore the KeyUp events
Expand Down

0 comments on commit 7d8d6ac

Please sign in to comment.