Skip to content

Commit e035fa3

Browse files
andyleejordanandyleejordan
authored
Add readme about ReadKey workarounds (#1833)
1 parent 2202e8c commit e035fa3

File tree

1 file changed

+166
-0
lines changed
  • src/PowerShellEditorServices/Services/PowerShell/Console

1 file changed

+166
-0
lines changed

‎src/PowerShellEditorServices/Services/PowerShell/Console/README.md

+166
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,166 @@
1+
# How We Handle `Console.ReadKey()` Being Uncancellable
2+
3+
The C# API `Console.ReadKey()` is synchronous and uncancellable. This is problematic in a
4+
asynchronous application that needs to cancel it.
5+
6+
## The Problem
7+
8+
We host a multi-threaded application. One thread is always servicing the REPL, which runs
9+
PSReadLine, which loops on a `ReadKey` call. Other threads handle various PowerShell
10+
requests via LSP, some of which necessitate interrupting PSReadLine and taking over the
11+
foreground (such as: start debugging, run code). While we have a smart task queue which
12+
correctly handles the cancellation of tasks, including the delegate calling PSReadLine, we
13+
cannot cancel `ReadKey` because as a synchronous .NET API, it is uncancellable.
14+
15+
So, no matter what, _at least one key must be consumed_ before PSReadLine's call to
16+
`ReadKey` is actually "canceled" (in this case, returned). This leads to bugs like [#3881]
17+
since the executed code is now using PowerShell's own prompting to get input from the
18+
user. Until our consumer (the `ReadKey` call) returns, the code behind the scenes of
19+
`$Host.UI.PromptForChoice()` won't get input. The actual fix would be for our `ReadKey`
20+
call to return without having received input after we canceled it (but we can't, so it
21+
doesn't).
22+
23+
[#3881]: https://github.com/PowerShell/vscode-powershell/issues/3881
24+
25+
A non-exhaustive list of known issues likely caused by this:
26+
27+
- [#3881](https://github.com/PowerShell/vscode-powershell/issues/3881)
28+
- [#3756](https://github.com/PowerShell/vscode-powershell/issues/3756)
29+
- [#2741](https://github.com/PowerShell/vscode-powershell/issues/2741)
30+
- [#3876](https://github.com/PowerShell/vscode-powershell/issues/3876)
31+
- [#2832](https://github.com/PowerShell/vscode-powershell/issues/2832)
32+
- [#2169](https://github.com/PowerShell/vscode-powershell/issues/2169)
33+
- [#1753](https://github.com/PowerShell/vscode-powershell/issues/1753)
34+
- [#3225](https://github.com/PowerShell/vscode-powershell/issues/3225)
35+
36+
For what it's worth, Tyler and have had conversations with the .NET team about making
37+
`ReadKey` cancelable. [#801] is an ancient GitHub issue with .NET, and we have had
38+
internal conversations.
39+
40+
[#801]: https://github.com/dotnet/runtime/issues/801
41+
42+
## Previous Workaround(s)
43+
44+
A previous workaround for this was to reinvent PowerShell's prompt handlers so they use
45+
our `ReadKey` call, see [#1583]. This is awful! It duplicates a lot of code when
46+
everything works so almost right without any of this. Except a key needs to be entered to
47+
"cancel" `ReadKey`.
48+
49+
[#1583]: https://github.com/PowerShell/PowerShellEditorServices/issues/1583
50+
51+
Now when I say "our `ReadKey`" call that's because we _already_ had some other workaround
52+
in place for this. Once upon a time (in the days of PowerShell 6 with older .NET Core
53+
versions), on macOS and Linux, if a thread was sitting in a `Console.ReadKey` loop, other
54+
`System.Console` APIs could not safely be called. For instance, `Console.CursorTop` is
55+
readily queried other events (such as the window resizing) and would deadlock, see
56+
[#1748]. So on these OS's we actually didn't use `Console.ReadKey` at all, but implemented
57+
a fake "`ReadKey`" that sits in a loop polling `Console.KeyAvailable`, see the
58+
[`ConsolePal.Unix`] implementation.
59+
60+
[#1748]: https://github.com/PowerShell/PowerShellEditorServices/pull/1748#issuecomment-1079055612
61+
[`ConsolePal.Unix`]: https://github.com/dotnet/runtime/blob/3ff8d262e504d03977edeb67da2b83d01c9ed2db/src/libraries/System.Console/src/System/ConsolePal.Unix.cs#L121-L138
62+
63+
This workaround led to other terrible behaviors, like the "typewriter" effect when pasting
64+
into our terminal, see [#3756]. Note that this issue only occurred on macOS and Linux,
65+
because on Windows we were still calling `Console.ReadKey`, but with a buffer to make it
66+
sort of cancellable, see [`ConsolePal.Windows`]. This is also the reason that [#3881] is
67+
Windows-specific. This makes pasting no macOS and Linux almost unusable, it takes minutes
68+
if you're pasting in a script to run.
69+
70+
[#3756]: https://github.com/PowerShell/vscode-powershell/issues/3756
71+
[`ConsolePal.Windows`]: https://github.com/dotnet/runtime/blob/3ff8d262e504d03977edeb67da2b83d01c9ed2db/src/libraries/System.Console/src/System/ConsolePal.Windows.cs#L307-L400
72+
73+
Another issue that is probably caused by these alternative "`ReadKey`" implementations is
74+
[#2741] where pasting totally fails. It seems like this has appeared before, and was
75+
previously fixed in [#2291].
76+
77+
[#2741]: https://github.com/PowerShell/vscode-powershell/issues/2741
78+
[#2291]: https://github.com/PowerShell/vscode-powershell/issues/2291
79+
80+
As an aside, but important note: these custom "`ReadKey`" implementations are the reason
81+
we have a private [contract] with PSReadLine, where we literally override the `ReadKey`
82+
method in that library when we load it, because PSReadLine is what is actually looping
83+
over `ReadKey`.
84+
85+
[contract]: https://github.com/PowerShell/PSReadLine/blob/dc38b451bee4bdf07f7200026be02516807faa09/PSReadLine/ConsoleLib.cs#L12-L17
86+
87+
## Explored But Inviable Workarounds
88+
89+
Back to [#3881] ("PowerShell prompts ignore the first input character"): one workaround
90+
could be to use the macOS/Linux `KeyAvailable`-based `ReadKey` alternative. But this
91+
should be avoided for several reasons (typewriter effect, battery drain, kind of just
92+
plain awful). It could be better if we improved the polling logic to slow way down after
93+
no input and speed up to instantaneous with input (like when pasting), but it would still
94+
be just a workaround.
95+
96+
An option I explored was to send a known ASCII control character every time the integrated
97+
console _received focus_ and have our `ReadKey` implementation ignore it (but return,
98+
since it received the key its stuck waiting for). This seemed like an ingenious solution,
99+
but unfortunately Visual Studio Code does not have an API for "on terminal focus" and it
100+
won't be getting one any time soon (I explored all the options in the [window] API, and
101+
confirmed with Tyler Leonhardt and Johannes Rieken, two VS Code developers). Theoretically
102+
we could have instead sent the character when our `RunCode` command is called but that
103+
only solves the problem some of the time. However, through this experiment I discovered
104+
that there is now an API to send arbitrary text over `stdin` to our extension-owned
105+
terminal, which is going to useful.
106+
107+
[window]: https://code.visualstudio.com/api/references/vscode-api#window
108+
109+
Another option explored was a custom `CancelReadKey` function that manually wrote a
110+
character to the PSES's own process's `stdin` in order to get `ReadKey` to return. While I
111+
was able to write the character (after using a P/Invoke to libc's `write()` function,
112+
because C#'s own `stdin` stream is opened, aptly, in read-only mode), it was not
113+
sufficient. For some reason, although the data was sent, `ReadKey` ignored it. Maybe
114+
`stdin` is redirected, or something else is going on, unfortunately I'm not sure. However,
115+
this exploration gave me the idea to hook up an LSP notification and have Code send a
116+
non-printing character when `CancelReadKey` is called, since Code is already hooked up to
117+
PSES's `stdin` and now has an API to directly write to it. More on this later.
118+
119+
Another workaround for all these issues is to write our own `ReadKey`, in native code
120+
instead of C# (so as to avoid all the issues with the `System.Console` APIs).
121+
Theoretically, we could write a cross-platform Rust app that simply reads `stdin`,
122+
translates input to `KeyInfo` structures, and passes that over a named pipe back PSES,
123+
which consumes that input in a "`ReadKey`" delegate using a channel pattern queue. This
124+
delegate would spawn the subprocess and hook the parent process's `stdin` to its own
125+
`stdin` (just pipe it across), and when the delegate is canceled, it kills the child
126+
process and unhooks the `stdin` redirect, essentially making this native app a
127+
"cancellable `ReadKey`." We did not end up trying this approach due to the cost involved
128+
in prototype it, as we would essentially be writing our own native replacement for:
129+
[`Console`]. We'd also need to deal with the fact that `Console` doesn't like `stdin`
130+
being [redirected], as PSReadLine indicates is already an issue.
131+
132+
[`Console`]: https://github.com/dotnet/runtime/blob/main/src/libraries/System.Console/src/System/Console.cs
133+
[redirected]: https://github.com/PowerShell/PSReadLine/blob/f46f15d2d634e2060bc0eabe4c81fc13a5a64a3a/PSReadLine/ReadLine.cs#L343-L356
134+
135+
## Current Working Solution
136+
137+
After trying a few different workarounds, something finally clicked and I combined several
138+
of the ideas. I realized that we already have an external process writing to PSES's
139+
`stdin`, and that's VS Code itself. Moreover, it now has a `SendText` API for the object
140+
representing the extension owned terminal (which is hosting PSES). So in [#1751], I wired
141+
up the cancellation token in our "safe, cancellable" `ReadKey` to send an LSP notification
142+
to the client called `sendKeyPress`, which on the client side simply uses that API to send
143+
a character (we eventually chose `p` because it's easy to see if something has gone wrong)
144+
to the terminal, _just as if the user had pressed a key_. This causes `Console.ReadKey` to
145+
return, since it received the input it was waiting on, and because we know that we
146+
requested a cancellation (through the token), we can ignore that input and move forward
147+
just as if the .NET API itself were canceled. Several things came together to make this
148+
solution viable:
149+
150+
- The pipeline execution threading rewrite meant that we don't have race conditions around
151+
processing input
152+
- VS Code added an API for us to write directly to our process's `stdin`.
153+
- We dropped support for PowerShell 6 meaning that the .NET `System.Console` APIs on macOS
154+
and Linux no longer deadlock each other.
155+
156+
This workaround resolved many issues, and the same workaround could be able applied to
157+
other LSP clients that host a terminal (like Vim). Moreover, we deleted over a thousand
158+
lines of code and added less than eighty! We did have to bake this workaround for a while,
159+
and it required PR [#3274] to PSReadLine, as well as a later race condition fix in PR
160+
[#3294]. I still hope that one day we can have an asynchronous `Console.ReadKey` API that
161+
accepts a cancellation token, and so does not require this fake input to return and free
162+
up our thread.
163+
164+
[#1751]: https://github.com/PowerShell/PowerShellEditorServices/pull/1751
165+
[#3274]: https://github.com/PowerShell/PSReadLine/pull/3274
166+
[#3294]: https://github.com/PowerShell/PSReadLine/pull/3294

0 commit comments

Comments
 (0)