Skip to content

Resolve cancellation race with PSReadLine #1754

Closed
PowerShell/PSReadLine
#3274
@andyleejordan

Description

@andyleejordan

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."

Metadata

Metadata

Assignees

Type

No type

Projects

Milestone

No milestone

Relationships

None yet

Development

No branches or pull requests

Issue actions