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.ReadKey should not throw when read returns 0 records #98684

Merged
merged 1 commit into from
May 22, 2024

Conversation

jazzdelightsme
Copy link
Contributor

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

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 dotnet#88697
@ghost
Copy link

ghost commented Feb 20, 2024

Tagging subscribers to this area: @dotnet/area-system-console
See info in area-owners.md if you want to be subscribed.

Issue Details

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

Author: jazzdelightsme
Assignees: -
Labels:

area-System.Console

Milestone: -

@miloush
Copy link
Contributor

miloush commented May 16, 2024

What is the guarantee this does not end up in an infinite loop?

@jazzdelightsme
Copy link
Contributor Author

@miloush : good question. There is no guarantee. If ReadConsoleInput infinitely successfully reads 0 bytes, then this code will just keep looping. I would classify such behavior as pathological, but the responsibility would be in ReadConsoleInput; it is not the job of this code to try to anticipate such pathological behavior.

Copy link
Member

@adamsitnik adamsitnik left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

LGTM, thank you for the fix @jazzdelightsme !

@adamsitnik adamsitnik merged commit 7d8d6ac into dotnet:main May 22, 2024
111 checks passed
@adamsitnik adamsitnik added this to the 9.0.0 milestone May 22, 2024
@jazzdelightsme jazzdelightsme deleted the gh_88697_Console_ReadKey_IOE branch May 23, 2024 17:36
steveharter pushed a commit to steveharter/runtime that referenced this pull request May 28, 2024
…98684)

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 dotnet#88697
Ruihan-Yin pushed a commit to Ruihan-Yin/runtime that referenced this pull request May 30, 2024
…98684)

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 dotnet#88697
@github-actions github-actions bot locked and limited conversation to collaborators Jun 23, 2024
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Console.ReadKey throws an InvalidOperationException when other console-attached process dies
3 participants