Skip to content

Commit 38d8b84

Browse files
SeeminglyScienceSeeminglyScience
committed
Fix ReadKey not canceling on Unix
- Add native Unix code from corefx to disable and enable input echo. - Refactor ConsoleReadLine.ReadKeyAsync to wait for a key to hit before locking stdin with Console.ReadKey.
1 parent 3ca4139 commit 38d8b84

File tree

5 files changed

+161
-38
lines changed

5 files changed

+161
-38
lines changed

‎.gitignore

+4-1
Original file line numberDiff line numberDiff line change
@@ -61,4 +61,7 @@ PowerShellEditorServices.sln.ide/storage.ide
6161
*.jrs
6262

6363
# Don't include PlatyPS generated MAML
64-
module/PowerShellEditorServices/Commands/en-US/*-help.xml
64+
module/PowerShellEditorServices/Commands/en-US/*-help.xml
65+
66+
# Don't include native build directory
67+
src/Native/Unix/build

‎PowerShellEditorServices.build.ps1

+11
Original file line numberDiff line numberDiff line change
@@ -85,11 +85,20 @@ task Clean {
8585
exec { & $script:dotnetExe clean }
8686
Remove-Item $PSScriptRoot\module\PowerShellEditorServices\bin -Recurse -Force -ErrorAction Ignore
8787
Remove-Item $PSScriptRoot\module\PowerShellEditorServices.VSCode\bin -Recurse -Force -ErrorAction Ignore
88+
Remove-Item $PSScriptRoot/src/Native/Unix/build -Recurse -Force -ErrorAction Ignore
8889
Get-ChildItem -Recurse $PSScriptRoot\src\*.nupkg | Remove-Item -Force -ErrorAction Ignore
8990
Get-ChildItem $PSScriptRoot\PowerShellEditorServices*.zip | Remove-Item -Force -ErrorAction Ignore
9091
Get-ChildItem $PSScriptRoot\module\PowerShellEditorServices\Commands\en-US\*-help.xml | Remove-Item -Force -ErrorAction Ignore
9192
}
9293

94+
task BuildNative -If { $IsUnix } -Before Build {
95+
New-Item $PSScriptRoot/src/Native/Unix/build -ItemType Directory | Out-Null
96+
97+
g++ -o $PSScriptRoot/src/Native/Unix/build/libdisablekeyecho.o $PSScriptRoot/src/Native/Unix/disable_key_echo.cpp -std=c++0x -c -fpic
98+
g++ -shared -o $PSScriptRoot/src/Native/Unix/build/libdisablekeyecho.so $PSScriptRoot/src/Native/Unix/build/libdisablekeyecho.o
99+
}
100+
101+
93102
task GetProductVersion -Before PackageNuGet, PackageModule, UploadArtifacts {
94103
[xml]$props = Get-Content .\PowerShellEditorServices.Common.props
95104

@@ -188,6 +197,8 @@ task LayoutModule -After Build {
188197
if (!$script:IsUnix) {
189198
Copy-Item -Force -Path $PSScriptRoot\src\PowerShellEditorServices.Host\bin\$Configuration\net451\* -Filter Microsoft.PowerShell.EditorServices*.dll -Destination $PSScriptRoot\module\PowerShellEditorServices\bin\Desktop\
190199
Copy-Item -Force -Path $PSScriptRoot\src\PowerShellEditorServices.Host\bin\$Configuration\net451\Newtonsoft.Json.dll -Destination $PSScriptRoot\module\PowerShellEditorServices\bin\Desktop\
200+
} else {
201+
Copy-Item -Force -Path $PSScriptRoot\src\Native\Unix\build\libdisablekeyecho.so $PSScriptRoot\module\PowerShellEditorServices\bin\Core
191202
}
192203

193204
# Lay out the PowerShellEditorServices.VSCode module's binaries

‎src/Native/Unix/disable_key_echo.cpp

+79
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,79 @@
1+
// The majority of this file is taken from the corefx repo. The original version is available here:
2+
// https://github.com/dotnet/corefx/blob/0325187c9d2ef504a2adf984f94655408ec1315e/src/Native/Unix/System.Native/pal_console.cpp#L1
3+
4+
#include <assert.h>
5+
#include <stdint.h>
6+
#include <termios.h>
7+
#include <stdio.h>
8+
#include <unistd.h>
9+
#include <errno.h>
10+
#include "disable_key_echo.h"
11+
12+
13+
static bool g_readInProgress = false; // tracks whether a read is currently in progress, such that attributes have been changed
14+
static bool g_signalForBreak = true; // tracks whether the terminal should send signals for breaks, such that attributes have been changed
15+
static struct termios g_preReadTermios = {}; // the original attributes captured before a read; valid if g_readInProgress is true
16+
static struct termios g_currTermios = {}; // the current attributes set during a read; valid if g_readInProgress is true
17+
18+
static void IncorporateBreak(struct termios *termios, int32_t signalForBreak)
19+
{
20+
assert(termios != nullptr);
21+
assert(signalForBreak == 0 || signalForBreak == 1);
22+
23+
if (signalForBreak)
24+
termios->c_lflag |= static_cast<uint32_t>(ISIG);
25+
else
26+
termios->c_lflag &= static_cast<uint32_t>(~ISIG);
27+
}
28+
29+
// In order to support Console.ReadKey(intecept: true), we need to disable echo and canonical mode.
30+
// We have two main choices: do so for the entire app, or do so only while in the Console.ReadKey(true).
31+
// The former has a huge downside: the terminal is in a non-echo state, so anything else that runs
32+
// in the same terminal won't echo even if it expects to, e.g. using Process.Start to launch an interactive,
33+
// program, or P/Invoking to a native library that reads from stdin rather than using Console. The second
34+
// also has a downside, in that any typing which occurs prior to invoking Console.ReadKey(true) will
35+
// be visible even though it wasn't supposed to be. The downsides of the former approach are so large
36+
// and the cons of the latter minimal and constrained to the one API that we've chosen the second approach.
37+
// Thus, InitializeConsoleBeforeRead is called to set up the state of the console, then a read is done,
38+
// and then UninitializeConsoleAfterRead is called.
39+
extern "C" void InitializeConsoleBeforeRead(uint8_t minChars, uint8_t decisecondsTimeout)
40+
{
41+
struct termios newTermios;
42+
if (tcgetattr(STDIN_FILENO, &newTermios) >= 0)
43+
{
44+
if (!g_readInProgress)
45+
{
46+
// Store the original settings, but only if we didn't already. This function
47+
// may be called when the process is resumed after being suspended, and if
48+
// that happens during a read, we'll call this function to reset the attrs.
49+
g_preReadTermios = newTermios;
50+
}
51+
52+
newTermios.c_iflag &= static_cast<uint32_t>(~(IXON | IXOFF));
53+
newTermios.c_lflag &= static_cast<uint32_t>(~(ECHO | ICANON | IEXTEN));
54+
newTermios.c_cc[VMIN] = minChars;
55+
newTermios.c_cc[VTIME] = decisecondsTimeout;
56+
IncorporateBreak(&newTermios, g_signalForBreak);
57+
58+
if (tcsetattr(STDIN_FILENO, TCSANOW, &newTermios) >= 0)
59+
{
60+
g_currTermios = newTermios;
61+
g_readInProgress = true;
62+
}
63+
}
64+
}
65+
66+
extern "C" void UninitializeConsoleAfterRead()
67+
{
68+
if (g_readInProgress)
69+
{
70+
g_readInProgress = false;
71+
72+
int tmpErrno = errno; // preserve any errors from before uninitializing
73+
IncorporateBreak(&g_preReadTermios, g_signalForBreak);
74+
int ret = tcsetattr(STDIN_FILENO, TCSANOW, &g_preReadTermios);
75+
assert(ret >= 0); // shouldn't fail, but if it does we don't want to fail in release
76+
(void)ret;
77+
errno = tmpErrno;
78+
}
79+
}

‎src/Native/Unix/disable_key_echo.h

+12
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,12 @@
1+
// The majority of this file is taken from the corefx repo. The original version is available here:
2+
// https://github.com/dotnet/corefx/blob/0325187c9d2ef504a2adf984f94655408ec1315e/src/Native/Unix/System.Native/pal_console.cpp#L1
3+
4+
/**
5+
* Initializes the terminal in preparation for a read operation.
6+
*/
7+
extern "C" void InitializeConsoleBeforeRead(uint8_t minChars, uint8_t decisecondsTimeout);
8+
9+
/**
10+
* Restores the terminal's attributes to what they were before InitializeConsoleBeforeRead was called.
11+
*/
12+
extern "C" void UninitializeConsoleAfterRead();

‎src/PowerShellEditorServices/Console/ConsoleReadLine.cs

+55-37
Original file line numberDiff line numberDiff line change
@@ -5,6 +5,7 @@
55

66
using System.Collections.ObjectModel;
77
using System.Linq;
8+
using System.Runtime.InteropServices;
89
using System.Text;
910
using System.Threading;
1011
using System.Threading.Tasks;
@@ -61,7 +62,7 @@ public async Task<SecureString> ReadSecureLine(CancellationToken cancellationTok
6162
{
6263
while (!cancellationToken.IsCancellationRequested)
6364
{
64-
ConsoleKeyInfo keyInfo = await this.ReadKeyAsync(cancellationToken);
65+
ConsoleKeyInfo keyInfo = await ReadKeyAsync(cancellationToken);
6566

6667
if ((int)keyInfo.Key == 3 ||
6768
keyInfo.Key == ConsoleKey.C && keyInfo.Modifiers.HasFlag(ConsoleModifiers.Control))
@@ -129,6 +130,58 @@ public async Task<SecureString> ReadSecureLine(CancellationToken cancellationTok
129130

130131
#region Private Methods
131132

133+
private static async Task<ConsoleKeyInfo> ReadKeyAsync(CancellationToken token)
134+
{
135+
await WaitForKeyAvailableAsync(token);
136+
return Console.ReadKey(true);
137+
}
138+
139+
private static async Task WaitForKeyAvailableAsync(CancellationToken token)
140+
{
141+
DisableInputEcho();
142+
try
143+
{
144+
while (!Console.KeyAvailable)
145+
{
146+
await Task.Delay(50, token);
147+
}
148+
}
149+
finally
150+
{
151+
EnableInputEcho();
152+
}
153+
}
154+
155+
private static void DisableInputEcho()
156+
{
157+
#if CoreCLR
158+
if (RuntimeInformation.IsOSPlatform(OSPlatform.Windows))
159+
{
160+
return;
161+
}
162+
163+
InitializeConsoleBeforeRead(0, 10);
164+
#endif
165+
}
166+
167+
private static void EnableInputEcho()
168+
{
169+
#if CoreCLR
170+
if (RuntimeInformation.IsOSPlatform(OSPlatform.Windows))
171+
{
172+
return;
173+
}
174+
175+
UninitializeConsoleAfterRead();
176+
#endif
177+
}
178+
179+
[DllImport("libdisablekeyecho.so")]
180+
private static extern void InitializeConsoleBeforeRead(byte minChars = 1, byte decisecondsTimeout = 0);
181+
182+
[DllImport("libdisablekeyecho.so")]
183+
private static extern void UninitializeConsoleAfterRead();
184+
132185
private async Task<string> ReadLine(bool isCommandLine, CancellationToken cancellationToken)
133186
{
134187
string inputBeforeCompletion = null;
@@ -154,7 +207,7 @@ private async Task<string> ReadLine(bool isCommandLine, CancellationToken cancel
154207
{
155208
while (!cancellationToken.IsCancellationRequested)
156209
{
157-
ConsoleKeyInfo keyInfo = await this.ReadKeyAsync(cancellationToken);
210+
ConsoleKeyInfo keyInfo = await ReadKeyAsync(cancellationToken);
158211

159212
// Do final position calculation after the key has been pressed
160213
// because the window could have been resized before then
@@ -466,41 +519,6 @@ await this.powerShellContext.ExecuteCommand<PSObject>(
466519
return null;
467520
}
468521

469-
private async Task<ConsoleKeyInfo> ReadKeyAsync(CancellationToken cancellationToken)
470-
{
471-
return await
472-
Task.Factory.StartNew(
473-
() =>
474-
{
475-
ConsoleKeyInfo keyInfo;
476-
477-
lock (this.readKeyLock)
478-
{
479-
if (cancellationToken.IsCancellationRequested)
480-
{
481-
throw new TaskCanceledException();
482-
}
483-
else if (this.bufferedKey.HasValue)
484-
{
485-
keyInfo = this.bufferedKey.Value;
486-
this.bufferedKey = null;
487-
}
488-
else
489-
{
490-
keyInfo = Console.ReadKey(true);
491-
492-
if (cancellationToken.IsCancellationRequested)
493-
{
494-
this.bufferedKey = keyInfo;
495-
throw new TaskCanceledException();
496-
}
497-
}
498-
}
499-
500-
return keyInfo;
501-
});
502-
}
503-
504522
private int CalculateIndexFromCursor(
505523
int promptStartCol,
506524
int promptStartRow,

0 commit comments

Comments
 (0)