Add "ConsoleBouncer" option, to terminate stragglers (to fix ctrl+c)

[jazzdelightsme]

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.

[StevenBucher98]

@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]

I'm not sure I understand the proposed implementation.

When enabled, PSReadLine will capture the list of processes currently attached to the console (via GetConsoleProcessList)

Is this option supposed to be enabled at startup of PowerShell? Or, can it be enabled at anytime while using pwsh interactively?

Before displaying the prompt, it will check the attached processes again, and terminate any that are not in the list of allowed processes.

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]

Thanks for taking a look at this, Steven and Dongbo!

how impactful is this to your work? Is the unreliability hitting a lot of folks

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.)

When enabled, PSReadLine will capture the list of processes currently attached to the console (via GetConsoleProcessList)

Is this option supposed to be enabled at startup of PowerShell? Or, can it be enabled at anytime while using pwsh interactively?

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:

    cmd.exe (PID 12)
      \
      powershell.exe (PID 34)
        \
        pwsh.exe (PID 56)

Displaying prompt is done PowerShell console host, not PSReadLine.

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:

PlatformWindows.SetConsoleInputMode(UInt32)
PlatformWindows.Init(Microsoft.PowerShell.ICharMap ByRef)
Microsoft.PowerShell.PSConsoleReadLine.ReadLine(System.Management.Automation.Runspaces.Runspace, System.Management.Automation.EngineIntrinsics, System.Threading.CancellationToken, System.Nullable`1<Boolean>)

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.

Two points:

1. This whole scheme basically does not apply to GUI processes, because they do not get attached to the console. So, for example, if you run 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 call GetConsoleProcessList.
2. If the child process is not a GUI process, and the shell is returning to the prompt, and the child process is still running, you are by definition in a really badly broken state: both the shell and the child process will fight for input, and you will have to kill something to recover--if you can find and kill all such children, your shell might be recoverable, if it doesn't hit the crash covered by that other Issue.

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]

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.

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 GetConsoleProcessList return this child process?

Before displaying the prompt, it will check the attached processes again, and terminate any that are not in the list of allowed processes.

Displaying prompt is done PowerShell console host, not PSReadLine.

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:

So back to your proposal, when should PSReadLine check the attached processes again and do the termination?

[jazzdelightsme]

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 GetConsoleProcessList return this child process?

No, and no.

when should PSReadLine check the attached processes again and do the termination?

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 call SetConsoleInputMode to disable ctrl+c.

[daxian-dbw]

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]

Thanks @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]

see (for example) #1099.

Good point. I have merged #3755.

For the PR, let's go with the name TerminateStragglers for now. @StevenBucher98 any thoughts on the name?

[StevenBucher98]

The "Stragglers" we are terminating are processes right? Maybe it may be more informative to have it be TerminateProcesses or a way more verbose TerminateStragglerProcesses (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 @sdwheeler (also Sean would love you opinion on parameter name too)

[sdwheeler]

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]

It will be a switch parameter on the Set-PSReadLineOption cmdlet. 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, @sdwheeler!

[sdwheeler]

So it sounds like we need new PSConsoleReadLineOptions members to hold the configuration and new parameters to Set-PSReadLineOptions to set and manage the values.

[jazzdelightsme]

@sdwheeler : right. Here is the PR: #3764

[StevenBucher98]

@daxian-dbw and I discussed and for now using TerminateOrphanedConsoleApps seems good to us! @jazzdelightsme if you can make that change in the PR we will look at reviewing/merging it! Thanks!

[jazzdelightsme]

@StevenBucher98 @daxian-dbw already done! :D

[daxian-dbw]

Closed via #3764