Skip to content

Sync ReadKeyProc thread with pipeline thread #3294

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.

Sign up for GitHub

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

Merged
merged 4 commits into from
Apr 27, 2022
Merged

Sync ReadKeyProc thread with pipeline thread #3294

Changes from all commits
Commits
Show all changes
4 commits
Select commit Hold shift + click to select a range
78e0341
Sync ReadKeyProc thread with pipeline thread
SeeminglyScience Apr 27, 2022
12178a7
Minor change to restore back to the original code.
daxian-dbw Apr 27, 2022
4af1055
Update comment and remove unused constant
SeeminglyScience Apr 27, 2022
15b46ca
Use CancellationToken.None
SeeminglyScience Apr 27, 2022
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
36 changes: 8 additions & 28 deletions PSReadLine/ReadLine.cs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -33,15 +33,11 @@ public partial class PSConsoleReadLine : IPSConsoleReadLineMockableMethods
{
private const int ConsoleExiting = 1;

private const int CancellationRequested = 2;

// *must* be initialized in the static ctor
// because the static member _clipboard depends upon it
// for its own initialization
private static readonly PSConsoleReadLine _singleton;

private static readonly CancellationToken _defaultCancellationToken = new CancellationTokenSource().Token;

// This is used by PowerShellEditorServices (the backend of the PowerShell VSCode extension)
// so that it can call PSReadLine from a delegate and not hit nested pipeline issues.
#pragma warning disable CS0649
Expand Down Expand Up @@ -143,17 +139,7 @@ private void ReadOneOrMoreKeys()
}
while (_charMap.KeyAvailable)
{
ConsoleKeyInfo keyInfo = _charMap.ReadKey();
if (_cancelReadCancellationToken.IsCancellationRequested)
{
// If PSReadLine is running under a host that can cancel it, the
// cancellation will come at a time when ReadKey is stuck waiting for input.
// The next key press will be used to force it to return, and so we want to
// discard this key since we were already canceled.
continue;
}

var key = PSKeyInfo.FromConsoleKeyInfo(keyInfo);
var key = PSKeyInfo.FromConsoleKeyInfo(_charMap.ReadKey());
_lastNKeys.Enqueue(key);
_queuedKeys.Enqueue(key);
}
Expand All @@ -170,10 +156,6 @@ private void ReadKeyThreadProc()
break;

ReadOneOrMoreKeys();
if (_cancelReadCancellationToken.IsCancellationRequested)
{
continue;
}

// One or more keys were read - let ReadKey know we're done.
_keyReadWaitHandle.Set();
Expand Down Expand Up @@ -208,7 +190,6 @@ internal static PSKeyInfo ReadKey()
// - a key is pressed
// - the console is exiting
// - 300ms timeout - to process events if we're idle
// - ReadLine cancellation is requested externally
handleId = WaitHandle.WaitAny(_singleton._requestKeyWaitHandles, 300);
if (handleId != WaitHandle.WaitTimeout)
{
Expand Down Expand Up @@ -292,10 +273,12 @@ internal static PSKeyInfo ReadKey()
throw new OperationCanceledException();
}

if (handleId == CancellationRequested)
if (_singleton._cancelReadCancellationToken.IsCancellationRequested)
{
// ReadLine was cancelled. Save the current line to be restored next time ReadLine
// is called, clear the buffer and throw an exception so we can return an empty string.
// ReadLine was cancelled. Dequeue the dummy input sent by the host, save the current
// line to be restored next time ReadLine is called, clear the buffer and throw an
// exception so we can return an empty string.
_singleton._queuedKeys.Dequeue();
_singleton.SaveCurrentLine();
_singleton._getNextHistoryIndex = _singleton._history.Count;
_singleton._current = 0;
Expand Down Expand Up @@ -331,9 +314,7 @@ private void PrependQueuedKeys(PSKeyInfo key)
/// <returns>The complete command line.</returns>
public static string ReadLine(Runspace runspace, EngineIntrinsics engineIntrinsics, bool? lastRunStatus)
{
// Use a default cancellation token instead of CancellationToken.None because the
// WaitHandle is shared and could be triggered accidently.
return ReadLine(runspace, engineIntrinsics, _defaultCancellationToken, lastRunStatus);
return ReadLine(runspace, engineIntrinsics, CancellationToken.None, lastRunStatus);
}

/// <summary>
Expand Down Expand Up @@ -396,7 +377,6 @@ public static string ReadLine(
}

_singleton._cancelReadCancellationToken = cancellationToken;
_singleton._requestKeyWaitHandles[2] = cancellationToken.WaitHandle;
return _singleton.InputLoop();
}
catch (OperationCanceledException)
Expand Down Expand Up @@ -877,7 +857,7 @@ private void DelayedOneTimeInitialize()
_readKeyWaitHandle = new AutoResetEvent(false);
_keyReadWaitHandle = new AutoResetEvent(false);
_closingWaitHandle = new ManualResetEvent(false);
_requestKeyWaitHandles = new WaitHandle[] {_keyReadWaitHandle, _closingWaitHandle, null};
Copy link
Member

Choose a reason for hiding this comment

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

Ah ha, the questions I had earlier but hadn't written yet are now irrelevant. Yay!

_requestKeyWaitHandles = new WaitHandle[] {_keyReadWaitHandle, _closingWaitHandle};
_threadProcWaitHandles = new WaitHandle[] {_readKeyWaitHandle, _closingWaitHandle};

// This is for a "being hosted in an alternate appdomain scenario" (the
Expand Down