Skip to content

Commit

Permalink
Sync ReadKeyProc thread with pipeline thread (#3294)
Browse files Browse the repository at this point in the history
The pipeline thread was returning before the `ReadKeyProcThread` had finished processing the dummy input.
This was occurring somewhat frequently when `ReadLine` was invoked multiple times in a row creating a race condition.
With these changes, the pipeline thread will wait for dummy input to be received, dequeue the key, and continue with normal cancellation logic.
  • Loading branch information
SeeminglyScience committed Apr 27, 2022
1 parent 77e256a commit e94afef
Showing 1 changed file with 8 additions and 28 deletions.
36 changes: 8 additions & 28 deletions PSReadLine/ReadLine.cs
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(Runspace runspace, EngineIntrinsics engineIntrinsi
}

_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};
_requestKeyWaitHandles = new WaitHandle[] {_keyReadWaitHandle, _closingWaitHandle};
_threadProcWaitHandles = new WaitHandle[] {_readKeyWaitHandle, _closingWaitHandle};

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

0 comments on commit e94afef

Please sign in to comment.