Repository navigation
Description
Description of the new feature/enhancement
Ctrl+c cancellation does not work really well on Windows. Each process attached to a console receives a ctrl+c signal (as opposed to just the active shell), and sometimes, when a shell has launched some large tree of child processes (imagine a build system, for example), some processes do not exit (perhaps due to races between process creation and console attachment), leaving multiple processes all concurrently trying to consume console input, which Does Not Work Well(TM). It's usually not too bad when cmd.exe is your shell, because you can just keep mashing on ctrl+c and usually get back to a usable state. But it's considerably worse in PowerShell, because PSReadLine temporarily disables ctrl+c signals when waiting at the prompt, which can lead to the console being completely unrecoverable.
In the ConsoleBouncer project, I sought to solve the problem with a PowerShell module. When loaded, the module installs a ctrl+c handler, and takes inventory of processes currently attached to the console; these are the "allowed processes". When a ctrl+c signal is received, all processes not in the "allowed list" (if any) are terminated.
This approach has some weaknesses. For one, it assumes that ctrl+c means "kill everything and get back to the shell", but that assumption is not always true. The Windows console debugger processes kd.exe and cdb.exe, for example, handle ctrl+c (interpreting it as a signal to break into the target). So ConsoleBouncer is perhaps mostly-good-enough for most people who need it, probably, but is definitely too kludgy for “everyone”. But PSReadLine could implement something similar that would actually solve the problem more directly.
The defining characteristic of a borked shell is that in its mind, it is just sitting at the prompt, waiting for user input, where at the same time, there are other processes attached to the console, also consuming input (leading to wild and unpredictable behavior, as different keystrokes go to different processes). The ConsoleBouncer module works by receiving a ctrl+c signal, and it does not know what PSReadLine in the shell (or anybody else) is up to. It assumes that ctrl+c means “everybody out!” (And it’s that assumption that is “mostly good enough”, but not really completely true.)
But if the solution were baked into PSReadLine (as an off-by-default option), it could be done differently: instead of triggering the “clear out the riff-raff” behavior upon receipt of a ctrl+c signal, it should only kick out loiterers right before displaying the next prompt. I.e. when it believes it is ready to just sit around and wait for user input (right before it calls SetConsoleMode to disable ctrl+c), it could just take steps to make sure that that user input is going to be able to come to it (whack the non-allowed PIDs).
Proposed technical implementation details
Add a new option (off by default), named "???" (TerminateStragglers?). When enabled, PSReadLine will capture the list of processes currently attached to the console (via GetConsoleProcessList). Before displaying the prompt, it will check the attached processes again, and terminate any that are not in the list of allowed processes.
Activity
StevenBucher98 commentedon Jul 24, 2023
Dan Thompson (@jazzdelightsme) I am a little confused by your proposed implementation, what determined if the processes are in the allowed list?
Also how impactful is this to your work? Is the unreliability hitting a lot of folks
daxian-dbw commentedon Jul 24, 2023
I'm not sure I understand the proposed implementation.
Is this option supposed to be enabled at startup of PowerShell? Or, can it be enabled at anytime while using pwsh interactively?
Displaying prompt is done PowerShell console host, not PSReadLine.
Also, do you mean before every time that PowerShell displays the prompt? A user could start a process in background using PowerShell, then that process will be terminated right after it was created before the next prompt is displayed. That doesn't sound right.
jazzdelightsme commentedon Jul 25, 2023
Thanks for taking a look at this, Steven and Dongbo!
Relative to the total install base, it is a small population, but it is an important population: Windows developers using razzle. C.f. this osgwiki page (internal link). It's a significant blocker to pwsh adoption, and pain point for those who do adopt it. (This would also hit anybody else trying to ctrl+c a build-like process where bazillions of console processes are being spawned; I'm sure they exist, I just don't know what they are.)
It could be enabled or disabled at any time. The assumption is that when the feature is enabled, any processes currently attached to the console are okay to have--they should all be either the current process or parent processes, as in the following example process tree:
Oh, right, sure--that was imprecise. I set a breakpoint to see exactly where PSReadLine is disabling ctrl+c signals, it's at this point:
Two points:
notepad, the notepad process starts, and your shell immediately displays the next prompt, and the notepad process will live on: it will not be found when we callGetConsoleProcessList.Now I just noticed that you said the user might "start a process in background"... that should not make a difference: such background processes are not attached to the (current) console; and if somebody is using thread jobs, situation 2 (above) applies: you just cannot, cannot, cannot have a child process fighting for input with the shell--that is the 100% badly broken state.
Does that answer your questions?
daxian-dbw commentedon Jul 25, 2023
What if the child process is a console application that doesn't read or write to the console? For example, let's say the child is a pwsh process running as an out-of-proc server for PowerShell background job, where both its stdin and stdout are redirected. In this case, it's a console application but not competing for console input. Is it attached to the current console? Will
GetConsoleProcessListreturn this child process?So back to your proposal, when should PSReadLine check the attached processes again and do the termination?
jazzdelightsme commentedon Jul 25, 2023
No, and no.
Right before we are going to turn off ctrl+c handling--at the point when PS tells PSReadLine "okay, it's you now"--when we are entering the state where (from the user's point of view) where the prompt/shell is waiting for input. Specifically, in
PlatformWindows.Init, right before we callSetConsoleInputModeto disable ctrl+c.daxian-dbw commentedon Jul 27, 2023
Then I think it's a reasonable change to make. If we address this one, do we still care about #3744? When turned on the option, PSReadLine will guarantee to kill all console-attached processes that are not in the allow-list, so the exception caused by #3744 should never happen then.
Also, can you maybe submit a PR for this issue as well?
jazzdelightsme commentedon Jul 27, 2023
Thanks Dongbo Wang (@daxian-dbw)! Yes, I think we still care about #3744, because to be maximally risk-averse, this feature would be off by default. I know people occasionally hit that exception in the wild; see (for example) #1099. And even though it is a low hit rate, I think it would be nice to make that bucket dry up completely, since we believe we now know what the problem is and have a simple fix.
Yes, I will prep a PR. One question in advance: does anyone have any suggestions / preferences for the name of the option? In the OP I proposed "TerminateStragglers", which I think is fairly concise and descriptive, but I didn't put too much effort into thinking of alternatives and I'm not married to it; so if anyone has better ideas, please let us know.
daxian-dbw commentedon Jul 27, 2023
Good point. I have merged #3755.
For the PR, let's go with the name
TerminateStragglersfor now. Steven Bucher (@StevenBucher98) any thoughts on the name?Add TerminateStragglers option (Windows)
Add TerminateStragglers option (Windows)
StevenBucher98 commentedon Jul 31, 2023
The "Stragglers" we are terminating are processes right? Maybe it may be more informative to have it be
TerminateProcessesor a way more verboseTerminateStragglerProcesses(definitely not great option 😆). Just when I see stragglers I don't know what its referring to.Also which cmdlet is this going to be a switch parameter for? Wanting to make sure I am ahead of any doc changes. cc Sean Wheeler (@sdwheeler) (also Sean would love you opinion on parameter name too)
sdwheeler commentedon Jul 31, 2023
Is this going to be a PSConsoleReadLineOptions member or an experimental feature? How is the allow list managed?
As for a name: what about
TerminateOrphanedConsoleApps?jazzdelightsme commentedon Jul 31, 2023
It will be a switch parameter on the
Set-PSReadLineOptioncmdlet. Since it's off by default, I don't think there's a need for an experimental feature to gate it.Yes, "stragglers" are processes... but I'm not sure if just adding "processes" into the name really makes things much clearer. "Stragglers" in this context are non-GUI, console-attached grandchildren processes; or put another way, non-GUI grandchild processes that are still attached to the console when it's time for the shell to start reading input again (back at the prompt). I think "TerminateOrphanedConsoleApps" captures that pretty well... thanks, Sean Wheeler (@sdwheeler)!
sdwheeler commentedon Jul 31, 2023
So it sounds like we need new PSConsoleReadLineOptions members to hold the configuration and new parameters to
Set-PSReadLineOptionsto set and manage the values.jazzdelightsme commentedon Jul 31, 2023
Sean Wheeler (@sdwheeler) : right. Here is the PR: #3764
StevenBucher98 commentedon Aug 1, 2023
Dongbo Wang (@daxian-dbw) and I discussed and for now using
TerminateOrphanedConsoleAppsseems good to us! Dan Thompson (@jazzdelightsme) if you can make that change in the PR we will look at reviewing/merging it! Thanks!jazzdelightsme commentedon Aug 1, 2023
Steven Bucher (@StevenBucher98) Dongbo Wang (@daxian-dbw) already done! :D
daxian-dbw commentedon Aug 9, 2023
Closed via #3764