Skip to content

Resolve cancellation race with PSReadLine #1754

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

Closed
andyleejordan opened this issue Apr 13, 2022 · 0 comments · Fixed by PowerShell/PSReadLine#3274 or #1758
Closed

Resolve cancellation race with PSReadLine #1754

andyleejordan opened this issue Apr 13, 2022 · 0 comments · Fixed by PowerShell/PSReadLine#3274 or #1758
Assignees
Labels

Comments

@andyleejordan
Copy link
Member

When canceling the ReadKey method sent to PSReadLine, we observed that there's a race where PSReadLine is still consuming (and using) the dummy character we're returning when the method is canceled:

private static readonly ConsoleKeyInfo s_nullKeyInfo = new(
keyChar: ' ',
ConsoleKey.DownArrow,
shift: false,
alt: false,
control: false);
private ConsoleKeyInfo ReadKey(bool intercept)
{
// PSRL doesn't tell us when CtrlC was sent.
// So instead we keep track of the last key here.
// This isn't functionally required,
// but helps us determine when the prompt needs a newline added
// NOTE: This requests that the client (the Code extension) send a non-printing key back
// to the terminal on stdin, emulating a user pressing a button. This allows
// PSReadLine's thread waiting on Console.ReadKey to return. Normally we'd just cancel
// this call, but the .NET API ReadKey is not cancellable, and is stuck until we send
// input. This leads to a myriad of problems, but we circumvent them by pretending to
// press a key, thus allowing ReadKey to return, and us to ignore it.
using CancellationTokenRegistration registration = _readKeyCancellationToken.Register(
() => _languageServer?.SendNotification("powerShell/sendKeyPress"));
// TODO: We may want to allow users of PSES to override this method call.
_lastKey = System.Console.ReadKey(intercept);
// TODO: After fixing PSReadLine so that when canceled it doesn't read a key, we can
// stop using s_nullKeyInfo (which is a down arrow so we don't change the buffer
// content). Without this, the sent key press is translated to an @ symbol.
return _readKeyCancellationToken.IsCancellationRequested ? s_nullKeyInfo : _lastKey.Value;
}

While the chosen dummy character should be a no-op, as observed in PowerShell/vscode-powershell#3225 (comment) it can come through as an à and in PowerShell/vscode-powershell#3881 (comment) it comes through as an ".

@daxian-dbw, @SeeminglyScience and I discovered this when trying to just use default for the returned value, and we think that around here "PSRL is probably checking if key returned, then checking if cancelled. Meaning if we have a key it doesn't check if cancelled."

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Projects
No open projects
Status: Done
1 participant