From 78e03411b0bf7d34566e425394aab8f7033f7720 Mon Sep 17 00:00:00 2001 From: Patrick Meinecke Date: Wed, 27 Apr 2022 11:08:30 -0400 Subject: [PATCH 1/4] Sync ReadKeyProc thread with pipeline thread The pipeline thread was returning before the ReadKeyProcThread had finished processing the dummy input. This was occuring 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. --- PSReadLine/ReadLine.cs | 24 ++++++------------------ 1 file changed, 6 insertions(+), 18 deletions(-) diff --git a/PSReadLine/ReadLine.cs b/PSReadLine/ReadLine.cs index e76c78d6..50021159 100644 --- a/PSReadLine/ReadLine.cs +++ b/PSReadLine/ReadLine.cs @@ -144,15 +144,6 @@ 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); _lastNKeys.Enqueue(key); _queuedKeys.Enqueue(key); @@ -170,10 +161,6 @@ private void ReadKeyThreadProc() break; ReadOneOrMoreKeys(); - if (_cancelReadCancellationToken.IsCancellationRequested) - { - continue; - } // One or more keys were read - let ReadKey know we're done. _keyReadWaitHandle.Set(); @@ -292,10 +279,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; @@ -396,7 +385,6 @@ public static string ReadLine( } _singleton._cancelReadCancellationToken = cancellationToken; - _singleton._requestKeyWaitHandles[2] = cancellationToken.WaitHandle; return _singleton.InputLoop(); } catch (OperationCanceledException) @@ -877,7 +865,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 From 12178a701f1dd197f463464cbeb1938718795404 Mon Sep 17 00:00:00 2001 From: Dongbo Wang Date: Wed, 27 Apr 2022 10:23:42 -0700 Subject: [PATCH 2/4] Minor change to restore back to the original code. --- PSReadLine/ReadLine.cs | 3 +-- 1 file changed, 1 insertion(+), 2 deletions(-) diff --git a/PSReadLine/ReadLine.cs b/PSReadLine/ReadLine.cs index 50021159..77eb4fbc 100644 --- a/PSReadLine/ReadLine.cs +++ b/PSReadLine/ReadLine.cs @@ -143,8 +143,7 @@ private void ReadOneOrMoreKeys() } while (_charMap.KeyAvailable) { - ConsoleKeyInfo keyInfo = _charMap.ReadKey(); - var key = PSKeyInfo.FromConsoleKeyInfo(keyInfo); + var key = PSKeyInfo.FromConsoleKeyInfo(_charMap.ReadKey()); _lastNKeys.Enqueue(key); _queuedKeys.Enqueue(key); } From 4af1055ac50d569b01d2588d10169d9792010087 Mon Sep 17 00:00:00 2001 From: Patrick Meinecke Date: Wed, 27 Apr 2022 13:34:31 -0400 Subject: [PATCH 3/4] Update comment and remove unused constant --- PSReadLine/ReadLine.cs | 3 --- 1 file changed, 3 deletions(-) diff --git a/PSReadLine/ReadLine.cs b/PSReadLine/ReadLine.cs index 77eb4fbc..dd87c6f4 100644 --- a/PSReadLine/ReadLine.cs +++ b/PSReadLine/ReadLine.cs @@ -33,8 +33,6 @@ 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 @@ -194,7 +192,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) { From 15b46ca41fd39404c1e1efd56c56b591bd865d69 Mon Sep 17 00:00:00 2001 From: Patrick Meinecke Date: Wed, 27 Apr 2022 13:46:03 -0400 Subject: [PATCH 4/4] Use CancellationToken.None We can now use CancellationToken.None instead of creating a dummy cancellation token because we use `CancellationToken.IsCancellationRequested`. --- PSReadLine/ReadLine.cs | 6 +----- 1 file changed, 1 insertion(+), 5 deletions(-) diff --git a/PSReadLine/ReadLine.cs b/PSReadLine/ReadLine.cs index dd87c6f4..fa7d93a9 100644 --- a/PSReadLine/ReadLine.cs +++ b/PSReadLine/ReadLine.cs @@ -38,8 +38,6 @@ public partial class PSConsoleReadLine : IPSConsoleReadLineMockableMethods // 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 @@ -316,9 +314,7 @@ private void PrependQueuedKeys(PSKeyInfo key) /// The complete command line. 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); } ///