Skip to content

Commit e94afef

Browse files
Sync ReadKeyProc thread with pipeline thread (#3294)
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.
1 parent 77e256a commit e94afef

File tree

1 file changed

+8
-28
lines changed

1 file changed

+8
-28
lines changed

PSReadLine/ReadLine.cs

+8-28
Original file line numberDiff line numberDiff line change
@@ -33,15 +33,11 @@ public partial class PSConsoleReadLine : IPSConsoleReadLineMockableMethods
3333
{
3434
private const int ConsoleExiting = 1;
3535

36-
private const int CancellationRequested = 2;
37-
3836
// *must* be initialized in the static ctor
3937
// because the static member _clipboard depends upon it
4038
// for its own initialization
4139
private static readonly PSConsoleReadLine _singleton;
4240

43-
private static readonly CancellationToken _defaultCancellationToken = new CancellationTokenSource().Token;
44-
4541
// This is used by PowerShellEditorServices (the backend of the PowerShell VSCode extension)
4642
// so that it can call PSReadLine from a delegate and not hit nested pipeline issues.
4743
#pragma warning disable CS0649
@@ -143,17 +139,7 @@ private void ReadOneOrMoreKeys()
143139
}
144140
while (_charMap.KeyAvailable)
145141
{
146-
ConsoleKeyInfo keyInfo = _charMap.ReadKey();
147-
if (_cancelReadCancellationToken.IsCancellationRequested)
148-
{
149-
// If PSReadLine is running under a host that can cancel it, the
150-
// cancellation will come at a time when ReadKey is stuck waiting for input.
151-
// The next key press will be used to force it to return, and so we want to
152-
// discard this key since we were already canceled.
153-
continue;
154-
}
155-
156-
var key = PSKeyInfo.FromConsoleKeyInfo(keyInfo);
142+
var key = PSKeyInfo.FromConsoleKeyInfo(_charMap.ReadKey());
157143
_lastNKeys.Enqueue(key);
158144
_queuedKeys.Enqueue(key);
159145
}
@@ -170,10 +156,6 @@ private void ReadKeyThreadProc()
170156
break;
171157

172158
ReadOneOrMoreKeys();
173-
if (_cancelReadCancellationToken.IsCancellationRequested)
174-
{
175-
continue;
176-
}
177159

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

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

339320
/// <summary>
@@ -396,7 +377,6 @@ public static string ReadLine(
396377
}
397378

398379
_singleton._cancelReadCancellationToken = cancellationToken;
399-
_singleton._requestKeyWaitHandles[2] = cancellationToken.WaitHandle;
400380
return _singleton.InputLoop();
401381
}
402382
catch (OperationCanceledException)
@@ -877,7 +857,7 @@ private void DelayedOneTimeInitialize()
877857
_readKeyWaitHandle = new AutoResetEvent(false);
878858
_keyReadWaitHandle = new AutoResetEvent(false);
879859
_closingWaitHandle = new ManualResetEvent(false);
880-
_requestKeyWaitHandles = new WaitHandle[] {_keyReadWaitHandle, _closingWaitHandle, null};
860+
_requestKeyWaitHandles = new WaitHandle[] {_keyReadWaitHandle, _closingWaitHandle};
881861
_threadProcWaitHandles = new WaitHandle[] {_readKeyWaitHandle, _closingWaitHandle};
882862

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

0 commit comments

Comments
 (0)