Sitelet https://web.archive.org/web/20210313061108/https://github.com/PowerShell/PowerShell/issues/13664
Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Bug: Stop-Process does not issue SIGTERM as expected. #13664

Open
robertbaker opened this issue Sep 20, 2020 · 52 comments
Open

Bug: Stop-Process does not issue SIGTERM as expected. #13664

robertbaker opened this issue Sep 20, 2020 · 52 comments

Comments

@robertbaker
Copy link

@robertbaker robertbaker commented Sep 20, 2020 •

Steps to reproduce

taskkill /IM "App With Tray Icon.exe"

Without moving mouse, tray icon disappears from tray immediately.

Stop-Process "App With Tray Icon"

Without moving mouse, tray icon is stays visible. Additionally, starting the process again will display a duplicate icon, the old one disappears when hovering over it.

Expected behavior

Stop-Process by default should be SIGTERM, graceful.
SIGKILL should only be used after timeout or -FORCE is used.

Actual behavior

The old tray icon stays visible because app is killed. (This is not actually PowerShell specific, but a technical caveat when processes are killed.)

Workaround

Use taskkill in place of stop-process for processes that have tray icons.

Environment data

Name                           Value
----                           -----
PSVersion                      7.0.3
PSEdition                      Core
GitCommitId                    7.0.3
OS                             Microsoft Windows 10.0.20215
Platform                       Win32NT
PSCompatibleVersions           {1.0, 2.0, 3.0, 4.0…}
PSRemotingProtocolVersion      2.3
SerializationVersion           1.1.0.1
WSManStackVersion              3.0
@robertbaker robertbaker changed the title Stop-Process does not act the same as taskkill (without force) Bug: Stop-Process does not act the same as taskkill (without force) Sep 20, 2020
@robertbaker robertbaker changed the title Bug: Stop-Process does not act the same as taskkill (without force) Bug: Stop-Process does not issue SIGTERM first Sep 20, 2020
@robertbaker robertbaker changed the title Bug: Stop-Process does not issue SIGTERM first Bug: Stop-Process does not issue SIGTERM as expected. Sep 20, 2020
@iSazonov
Copy link
Collaborator

@iSazonov iSazonov commented Sep 20, 2020

@robertbaker Please check with latest PowerShell 7.1 Preview build.

@iSazonov
Copy link
Collaborator

@iSazonov iSazonov commented Jan 18, 2021

I guess it is related to .Net API.
Close as stale issue.

@iSazonov iSazonov closed this Jan 18, 2021
@PsychoData
Copy link

@PsychoData PsychoData commented Jan 25, 2021

This seems to be the case that it jumps straight to Kill instead of requesting close

I believe it traces back to here

if (!process.HasExited)
{
process.Kill();
}

or possible here

private void StopProcess(Process process)
{
Exception exception = null;
try
{
if (!process.HasExited)
{
process.Kill();
}
}

It seems like you could send CloseMainWindow and since it returns True or False, just fall back to .Kill() if .CloseMainWindow() if it was not applicable or cannot send and returned False instead.

This would enable "soft" closes of something like Notepad that has an unsaved document opened in it, as well as -Force closing the same document if desired.

It would also give programs a chance to handle being closed (unless they were Forced) to do things like write save a final recovery copy of unsaved documents, which they won't get if a Kill is sent

@iSazonov
Copy link
Collaborator

@iSazonov iSazonov commented Jan 25, 2021

@PsychoData Thanks for your investigations! Do you want to pull PR? I'd review and merge.

@iSazonov iSazonov reopened this Jan 25, 2021
@PsychoData
Copy link

@PsychoData PsychoData commented Jan 25, 2021

not sure I would be best for it - this would be my first foray into Posh Core coding and in a pretty core area

Not sure if there are any other best practices I should hit for it too, or creating tests for it.

If someone else wanted to go for it, I would feel better about it

@iSazonov
Copy link
Collaborator

@iSazonov iSazonov commented Jan 26, 2021

@PsychoData
I see you are already reading the code. If you are wasting your time then you have interest. If you have any interest then welcome to contribute!
Some parts of PowerShell are really confusing, but you can contribute to the parts that you are interested in and that you understand. You could read about Working Groups

The change you propose is not complex. I don't think we can create a reliable test for this - I think manual testing the scenario enough (with good comments in code).

@PsychoData
Copy link

@PsychoData PsychoData commented Jan 26, 2021

well - fine if you twist my arm :)

@PsychoData
Copy link

@PsychoData PsychoData commented Jan 26, 2021

@iSazonov there's what I threw in
ideally, I would be thinking some sort of .... timeout.... like Stop-Process -timeout 500 and it sends a "close" request at first and if it hasn't closed within the timeout, then it sends a Kill, but that is beyond my current C# skills

@iSazonov
Copy link
Collaborator

@iSazonov iSazonov commented Jan 27, 2021 •

@PsychoData Thanks for your contribution!

Original behavior on the cmdlet is silently kill a process. We can not change the default behavior otherwise this will be a breaking change. I mean Stop-Process notepad.exe shouldn't wait while ask an user to save a file. So all we can do here is to make the cmdlet more smart and before calling Kill() we could call CloseMainWindow(). If the method blocks current thread we should run it in async, wait some ms and fallback to Kill() if the process is still running. This is a rough description, you could think more and experiment.

Update: CloseMainWindow() doesn't block and we can do something like http://csharp-slackers.blogspot.com/2008/09/terminate-process.html


In separate PR we could add new parameter like -GracefullyShutdown [timeout ms] so that change the default behavior and allow an user interaction and wait until timeout.

@PsychoData
Copy link

@PsychoData PsychoData commented Jan 27, 2021

The Powershell process doesn't wait while it asks the user to close, that thread moves right along and leaves Notepad.exe to finish closing itself, or cancel closing itself

You can see the prompt in Powershell is returning immediately and not waiting for the save/noSave/cancel prompts
Running the second time while the save prompt is up in a Modal window, it can't send the CloseMainWindow again, so it goes through as a Kill on that window, but still send Close to that one Window I had cancelled the previous close

and finally, the Stop-Process -Force closes directly as Kill
but in any of the cases the pipelines are sending a close or kill command and then moving on
image

@iSazonov
Copy link
Collaborator

@iSazonov iSazonov commented Jan 30, 2021

On further thought, I see that the current proposed implementation is a breaking change. I thing we should avoid a breaking change and follow the original behavior on the cmdlet that is silently kill a process.
However, I suppose we could make two improvements.

  1. Improve base behavior and first try gracefully close process
  2. Add new graceful shutdown behavior with new parameter like -GracefullyShutdown [timeout ms]

For 1:

  1. Call CloseMainWindow() and WaitForExit(20ms) (20ms here is an empiric value.)
  2. If the process is still running we call Kill()

For 2:

  1. If GracefullyShutdown == 0 we call Kill()
  2. Otherwise, only call CloseMainWindow() and WaitForExit()

/cc @mklement0 What do you think?

@PsychoData
Copy link

@PsychoData PsychoData commented Jan 31, 2021

I am not crazy about the -GracefullyShutdown name it makes me think that you have to specify that parameter for it to GracefullyShutdown

Well, if we are going to keep it with force close (Kill) function on Get-Process Notepad | Stop-Process then it would make sense to do something like Get-Process Notepad | Stop-Process -CloseTimeout <Millis> and default to like ..... 200 milliseconds?

and then it could iterate through the Notepad Processes, start the thread timer for the Kill and then send the Close signal.
On Expiration of the "Kill" timer, it can check if it has stopped and kill if needed

I suppose we could do some testing - but I'm not sure 20 milliseconds would be enough time to the random processes to handle the Close event, close anything down (remember, some of that might be trying to send network session close requests, etc). But that's just deciding on a good default without tying up the timeline for too long.

For example, sometimes Outlook can close in a fraction of a second, sometimes it takes 1/2 a second, sometimes a few seconds, or sometimes it hangs for what seems like unlimited amount of time (I've seen hours)

a param like -CloseTimeout might be a good medium to preserve the existing behavior of forcing the process to close, but giving it an (adjustable) timeout to shut itself down before killing. If they specifically need it to Kill directly instead of Close then Kill, then I think -Force should still skip the Close and should go to Kill directly still, as it currently does.

@iSazonov
Copy link
Collaborator

@iSazonov iSazonov commented Jan 31, 2021

Well, if we are going to keep it with force close (Kill) function on Get-Process Notepad | Stop-Process then it would make sense to do something like Get-Process Notepad | Stop-Process -CloseTimeout <Millis> and default to like ..... 200 milliseconds?

Yes, I believe it is mandatory requirement to avoid a breaking change. I like the idea about a default value for the new parameter. As for the parameter name, perhaps we could use GracefullyStopTimeout name.

I think -Force should still skip the Close and should go to Kill directly still, as it currently does.

It would be again a breaking change for Force parameter (today it allows only to close a process of other users). I believe we could use zero timeout (in GracefullyStopTimeout parameter) for the scenario - it is easy understandable for users than complicating Force parameter.

@mklement0
Copy link
Contributor

@mklement0 mklement0 commented Jan 31, 2021 •

@iSazonov, I agree regarding the concerns about the breaking changes, but let us take a step back:

Conceptually, we are dealing with two modes of termination:

  • Cooperative, via signal SIGTERM: the process is asked to shut itself down, allowing it to clean up, but it may or may not honor the termination request.

  • Forced, via signal SIGKILL: the process is forcefully terminated, without any awareness of that act.

As for default behavior:

  • taskkill on Windows and kill on Unix default to SIGTERM - which, given the word "kill" in their name, is unfortunate, but that ship has sailed a long time ago.

  • Stop-Process defaults to SIGKILL

This discrepancy is unfortunate, but I think we're stuck with it.

In terms of terminology, it is similarly unfortunate that "stopping" a process (SIGSTOP) in the Unix world means suspending (pausing) it, with the option to resume (continue) it later (SIGCONT).

As for synchronous vs. asynchronous behavior:

Both modes of termination are inherently asynchronous - they send a signal and return instantly:

  • In practice, the SIGKILL method typically acts quasi-synchronously, because the kernel handles the termination.

    • However, according to the comments at https://stackoverflow.com/q/8762228/45375 re Unix platforms, "the process won't terminate immediately. If a process is doing a system call, it will only end when the system call returns. So if it does some real heavy processing, it may take time.""

    • Also, on Unix-like platforms termination may even fail altogether if the target process is in an uninterruptible state.

  • By contrast, SIGTERM is much more likely to be asynchronous in practice, since the process itself processes the signal, processing doesn't happen until the process is given CPU time next, and the process may block termination indefinitely with a modal confirmation prompt, or generally refuse to terminate altogether.


Given the above, I think the conceptually cleanest approach would be:

  • Leave the existing behavior as is (SIGKILL by default, potentially - though typically not - asynchronous behavior; keep existing -Force semantics)

  • Introduce two new switches:

    • -Request or -ByRequest (as a shorter alternative to -GracefullyShutdown; name negotiable), which sends SIGTERM instead of the default SIGKILL.

      • As an aside: It's worth considering support for sending arbitrary signals to processes. On Unix, this is somewhat counterintuitively done via the kill utility as well, so you could argue we should extend Stop-Process analogously, but I don't know to what extent that makes sense on Windows, and it is a separate debate.
    • -Wait, which waits indefinitely for the process(es) to terminate, irrespective of whether termination is performed via SIGKILL or SIGTERM.

In case a timeout is needed, use -PassThru and pipe to Wait-Process -Timeout.
This follows the pattern of Start-Process, which has a -Wait switch, but lacks a way to specify a timeout.

Another aside: It is unfortunate that the current -TimeOut parameters across the cmdlets that support them are [int]-typed, allowing for whole-second wait times only. We should consider changing the type to [double] instead to support fractional seconds (instead of a separate -Milliseconds parameter, as was implemented for Start-Sleep), which I would expect to be a Bucket 3: Unlikely Grey Area change.

Stack Overflow
Is the kill function in Linux synchronous? Say, I programatically call the kill function to terminate a process, will it return only when the intended process is terminated, or it just sends the si...
@PsychoData
Copy link

@PsychoData PsychoData commented Feb 1, 2021

right, re: the nomenclature, I was usually referring back to the .NET .Kill() method which will force-stop immediately and is somewhat analogous to SIGKILL. I am not very familiar with Unix or it's Kill method, so I didn't realize the kill binary would also send more types of terminations too.

To keep things clear I'm going to call it SIGKILL and SIGTERM from here on out, though I could not find a 100% certain source that .NET .Kill() equates to SIGKILL, or more particularly that .NET .CloseMainWindow() would equate to SIGTERM

On a separate point, it would probably be useful to get some extra information about what kind of terminations it was able to send (SIGKILL/SIGTERM).
Example psuedocode in Begin, Process,End mentality for Stop-Process :

Get-Process  Notepad 
<3 Processes found>
Get-Process  Notepad | Stop-Process -CloseRequest 400
Begin: <nothing relevant here?> 
Process: Sent Close request to Process1
Process: Start Timer for CloseRequestProcess1
Process: Sent Close request to Process2
Process: Start Timer for CloseRequestProcess2
Process: Sent Close request to Process3
Process: Start Timer for CloseRequestProcess3
End: Wait for timers to expire OR all processes to be closed, and Kill any Processes that haven't stopped yet
End: Process2 closed itself succesfully
End: `CloseRequestProcess1` and `CloseRequestProcess3` timer expired
End: Send `SIGKILL` to Process3
End: Send `SIGKILL` to Process3
End: Wait for any Timeout to expire

I don't know if there is a way to do the "wait and fall back to SIGKILL" End section without it having to block the pipeline up for the SIGTERM timeout to expire (unless it is -PassThru, obviously) but ideally, I would like to let it just keep going asynchronously and not stop up the pipeline.
If we can have it fall back to SIGKILL after a timer/timeout expires without blocking the pipeline, then we could also potentially make the CloseTimeout much larger, say 500 milliseconds, to allow for things like Outlook to close. Otherwise I think any timeout ought to stay much lower to keep blocking the pipeline to a minimum

@PsychoData
Copy link

@PsychoData PsychoData commented Feb 1, 2021

On the sidenotes, I think enabling the other types of signals to send besides SIGKILL and SIGTERM sound like a great idea, but most likely seem like it ought to be a whole separate cmdlet?

Converting the -TimeOut to double would be nice, or maybe separate -TimeoutMilliseconds

But those should probably be separate issues to talk about those suggestions

@iSazonov
Copy link
Collaborator

@iSazonov iSazonov commented Feb 1, 2021 •

@mklement0 @PsychoData Thanks for sharing your thoughts!

I think enabling the other types of signals to send besides SIGKILL and SIGTERM sound like a great idea, but most likely seem like it ought to be a whole separate cmdlet?

.Net does not support signals at all - it is fundamental limitation. We shouldn't go in the direction (until something will be changed in .Net.)

Converting the -TimeOut to double would be nice, or maybe separate -TimeoutMilliseconds

It would confusing users. We use Timeout parameter of int type in some cmdlets. See Get-Command -ParameterName Timeout | gcm -Syntax. I don't think we should discuss this here.

-Wait, which waits indefinitely for the process(es) to terminate, irrespective of whether termination is performed via SIGKILL or SIGTERM.

This functionality is in Wait-Process. We have no need to move it to Stop-Process.


Main question in the issue is should we try to make the cmdlet more smart on Windows so that call CloseMainWindow() before fallback to Kill()? If no this simplify all. Now I think this would be the best way. Can you vote for this?
In this case, we would just make two parameter sets- (1) Kill as default, (2) Terminate (or GracefullyStop) for new functionality.

@mklement0
Copy link
Contributor

@mklement0 mklement0 commented Feb 1, 2021

@iSazonov, I agree regarding the issues not to discuss here; it is exactly why I called them asides: something to perhaps inspire a separate discussion, though I get that that's problematic without actually creating and/or pointing to such separate discussions, because the temptation is there to respond here. To close the one tangent: Point taken re signals in general, but see below.

@PsychoData, re nomenclature: I should have made it clearer that I used SIGKILL or SIGTERM loosely, as shorthand to refer to the two termination modes I've described.

In terms of implementation, SIGKILL (forced termination) corresponds to .Kill() in .NET both on Windows and on Unix; SIGTERM (cooperative termination) corresponds to .CloseMainWindow() in .NET on Windows, but on Unix we'd have to send the actual SIGTERM signal to get equivalent functionality - and it sounds like we'll have to go native for that, correct?


This functionality is in Wait-Process. We have no need to move it to Stop-Process.

Yes, we have Wait-Process, but just like Start-Process -Wait and Receive-Job -Wait exist - despite the existence of the dedicated Wait-Process and Wait-Job cmdlets - implementing -Wait on Stop-Process would be a convenience switch to make the typical use case easier to implement; more fine-grained waiting - notably via -TimeOut - would then require explicit use of Wait-Process.

In other words: For convenience, Stop-Process -ByRequest $somePid -Wait would be the equivalent of
Stop-Process -ByRequest $somePid -PassThru | Wait-Process - just like it works for Start-Process.


Main question in the issue is should we try to make the cmdlet more smart on Windows so that call CloseMainWindow() before fallback to Kill()? If no this simplify all. Now I think this would be the best way.

I see two basic approaches:

Option A: Focus on separation of concerns, as suggested by @iSazonov and also in my previous comment:

This means not implementing any fallback logic and not implementing any timeout in Stop-Process.

You'd get asynchronous behavior by default, as currently, but can opt-in to wait indefinitely with -Wait. Otherwise, use -PassThru and pipe to Wait-Process

In terms of syntax, this means (for brevity I'm only showing the -Id-based input; name of -ByRequest negotiable):

Stop-Process [-Id] <int[]> [-PassThru] [-Force] [-ByRequest] [-Wait]

That is, to implement a timeout with cooperative termination you'd have to use something like:

Stop-Process $somePid -ByRequest -PassThru | Wait-Process -Timeout 1

Wait-Process reports a non-terminating error for (each) target process that doesn't terminate in the timeout period.

This means that in order to fall back to forced termination you'd have to handle that error and call Stop-Process again, this time without -ByRequest.

The question is how common this scenario is. If it is common, we should make things easier, in which case my suggestion is to add a -StopOnTimeout switch to Wait-Process, which would call .Kill() after the timeout has elapsed, followed by .WaitForExit() - or go with Option B (see below).


Option B: Focus on high-level logic, along the lines of @PsychoData's proposal:

# FORCED termination
Stop-Process [-Id] <int[]> [-PassThru] [-Force] [-Wait]

# COOPERATIVE termination
Stop-Process [-Id] <int[]> [-PassThru] [-Force] -ByRequest [-Wait] [-Timeout <double>]

That is, asynchronous behavior would remain the default (also with -ByRequest), unless you specify -Wait.

Only with -ByRequest (cooperative termination) do you get to specify a timeout, after which .Kill() and .WaitForExit() are called.

(The assumption is that .Kill() either succeeds - in which the process does get terminated and indefinite waiting is guaranteed to end (shortly) - or fails altogether, so that there's no point in supporting a timeout).


I can see arguments for both options; if needing to fall back to forced termination is a common scenario, I can see the appeal of option B.

@PsychoData
Copy link

@PsychoData PsychoData commented Feb 1, 2021

Well, .Kill() returns void, so it either presumably succeeds in sending the signal and ends/returns, or throws an error that it couldn't send the .Kill() signal.

Originally my thinking was a third, I will call Option C

Option C: Have Stop-Process send SIGTERM/.CloseMainWindow() by default, and only send .Kill() if it failed to signal "Close" or if -Force was specified then default to .Kill()

After all the discussion I see why we wouldn't want to change that functionality of expecting Stop-Process to always result in a process that is .Kill()/Forced to close if necessary, since someone may be expecting that for their existing code that was in place - Pester Tests for example.

my thinking with Option B is that we preserve the high-level function of Stop-Process that it will forcibly kill something if necessary, without having to specify -Force for it to .Kill() a process if necessary, while only adding a small (minimum) and adjustable timeout.

Option B is a nice medium between the current "Always default to kill everything" and my originally provided code of "Try to send Close, but Kill if .NET says that failed to send"

@PsychoData
Copy link

@PsychoData PsychoData commented Feb 1, 2021

I think the only point we are getting hung on talking about is that I am thinking
Stop-Process 'Notepad','Outlook' (with a default -ByRequest implied)
Would result in

  • Send Outlook and Notepad a Close signal (repeat for any additional processes specified)
    • Start ByRequest timer here
    • This is starting all of the ByRequest timers as close as possible in time
    • Still attempt to send .Close() but if that fails jump straight to .Kill()
  • Wait until ByRequest timeout expires in the End section of the processing
    • Loop until their ByRequest Timers have expired, or the processes are all stopped
    • Could Optionally add a "NoWait" or just turn ByRequest timeout to 0 or 1 milliseconds
    • If timeouts expire, and there are still valid Processes, send the .Kill() and let the method return/continue onward

In many... probably most... cases this timeout would not even be needed because the program will handle being sent a .Close() signal and just close itself before you're checking on the timeout anyway. If this used a Low default for ByRequest (20 millis? 50? 100? No idea of a best value for that yet) then we should be looking at.

Keep in mind, that if we are running on 5 processes with a 500 millis ByRequest timeout, this won't be 500+500+500+500 it would be around 500 + time to send all the .Close() signals. For me, using three processes, that took about 100 millis to send the .Close() commands and finish the cmdlet
image

So if it had a 500 milli ByRequest timer, you are done sending the Close() signals long before the ByRequest time expires... and I would figure it took maybe .... 600 millis total execution time with the send close(), Wait for ByRequest to expire, and Some Processes did not gracefully close, Kill them depending on how busy the machine was and could revisit the timer thread

@iSazonov
Copy link
Collaborator

@iSazonov iSazonov commented Feb 2, 2021

Yes, we have Wait-Process, but just like Start-Process -Wait and Receive-Job -Wait exist - despite the existence of the dedicated Wait-Process and Wait-Job cmdlets - implementing -Wait on Stop-Process would be a convenience switch to make the typical use case easier to implement; more fine-grained waiting - notably via -TimeOut - would then require explicit use of Wait-Process.

In the case this could be separate enhancement since it is not mandatory for enhancement we consider here.

I should have made it clearer that I used SIGKILL or SIGTERM loosely, as shorthand to refer to the two termination modes I've described.

I feel most of users follow intuitively the terms. I believe we need to follow this in parameter names too. If we ask users what is:

Stop-Process -Kill
Stop-Process -Terminate

most of them give us right description. But Stop-Process -ByRequest will force them to read docs before get understanding its semantic.

If we start with adding new Terminate switch (and perhaps Kill for symmetric) this will address current issue in simplest way and open ways for future enhancements.

@mklement0
Copy link
Contributor

@mklement0 mklement0 commented Feb 2, 2021 •

@iSazonov:

On a meta note, I think at this point it is clear that before implementing anything we need to write up a new, focused proposal, following this discussion.
Therefore I think it's worth having this discussion, even as it gets lengthy.


Re -Wait switch:

In the case this could be separate enhancement since it is not mandatory for enhancement we consider here.

Based on my new proposal below we won't need the -Wait switch, after all, but in general I find that argument problematic: While with Option A -Wait would technically not be necessary (it would be with Option B), it would greatly increase the utility of the enhancement. If -Wait were still in the picture, I wouldn't see a reason to do this separately, not least because I suspect it would then never be implemented.


Stop-Process -Kill
Stop-Process -Terminate

While I like the idea of these contrasting switches to make the two modes explicit, there is the awkwardness of then having a switch being true by default, namely-Kill.

Also:

  • Familiarity with the term terminate likely only applies to Unix users; in the context of the GUI window-centric Windows, close is used, as reflected in the .CloseMainWindow() method name; also, terminate by itself, from a natural-language perspective, doesn't adequately convey the aspect of cooperative / graceful stopping.

  • (Kill, while unambiguous in terms of common usage, is tainted by the conceptual confusion of both taskkill.exe and kill exhibiting terminate / close rather than kill behavior by default, but there's obviously nothing we can do about this.)

  • Stop invariably means something different in PowerShell, which in signal terms in Unix (SIGSTOP) - regrettably - means suspending a process.

    • As an approved verb, Stop has no precise definition with respect to cooperative vs. forced, but in practice Stop-Process is the only outlier in that it performs forced stopping (Stop-Job, Stop-Computer, Stop-Transcript do not).

    • However, given that the approved-verbs documentation lists under "synonyms to avoid" for Stop "End, Kill, Terminate, Cancel", the implication is that Stop is meant to cover both cooperative and forced stopping.

    • Of course, it would then make sense to consistently make cooperative stopping the default, with an opt-int for forced stopping. Regrettably, Stop-Process not only doesn't offer cooperative stopping, but invariably performs forced stopping.

In short: We won't be able to use existing terminology from one platform without it clashing with that of another.

My suggestion was motivated by using names based on platform-neutral abstractions that express the conceptual intent; perhaps -ByRequest isn't great, and something like -Graceful (a shorter version of the previously suggested -GracefullyShutdown) is more descriptive.

If we had established semantics of Stop without further qualification implying cooperative (graceful) stopping, then we'd only ever need a switch to opt-into forced stopping - and the obvious name for such a switch would be -Force.


@PsychoData:

(with a default -ByRequest implied)

We cannot default to cooperative stopping without breaking backward compatibility - users may have come to rely on unconditional, forced, quasi-synchronous termination.

Given the conceptual musings above, I sincerely wish we could break backward compatibility, which makes this a candidate for #6745.

As for timeouts: For simplicity and predictability, I'd use the timeout as a single, overall waiting period, irrespective of how many processes are targeted: I would start a single timing in the End block (not sure what Wait-Process does).


Let me propose Option C:

  • Make Stop-Process synchronous by default, with an opt-in kill timeout that _only applies if -Graceful (-ByRequest) is also specified. This is similar to Option B, except that -Wait is no longer required.

  • Conversely, a new -NoWait switch must be used to request asynchronous behavior.

Note: For consistency, I suggest also making the by-default kill operation synchronous (call .WaitForExit() after .Kill()): this would technically also be a breaking change, albeit an acceptable one (bucket 3: Unlikely Grey Area): I think it would have virtually no impact on existing code (given that .Kill() forcefully terminates, I would expect the extra time spent waiting for actual termination to be negligible) while making the behavior slightly more predictable (even though it's unlikely that the current asynchronous behavior surfaces as such).

# FORCED termination, (now) synchronous by default, except if -NoWait is passed.
Stop-Process [-Id] <int[]> [-PassThru] [-Force] [-NoWait]

# COOPERATIVE (graceful) termination: synchronous (indefinite wait) by default, except if -NoWait is passed.
Stop-Process [-Id] <int[]> [-PassThru] [-Force] -Graceful [-NoWait]

# COOPERATIVE (graceful) termination: synchronous, but with optional timeout resulting in
# *synchronous killing*  if the processes don't terminate in time.
Stop-Process [-Id] <int[]> [-PassThru] [-Force] -Graceful [-Timeout <double>]

@PsychoData, note that this proposal intentionally does not include a built-in, automatic kill timeout with -Graceful termination: Users should be free to decide whether they want to:

  • wait indefinitely (by default), which is the safest behavior.
  • not wait at all (-NoWait)
  • wait for a while and, in case of non-termination within that period, kill (synchronously) (-TimeOut <n>)
  • wait for a while and, in case of non-termination, only report an error, via Wait-Process:
    Stop-Process $somePid -Graceful -NoWait -PassThru | Wait-Process -Timeout $n
@mklement0
Copy link
Contributor

@mklement0 mklement0 commented Feb 3, 2021 •

Thanks, @iSazonov, but let me spell out the implications of your proposal, from which I conclude that it is not worth implementing as such:

  • The behavior will be inconsistent and potentially ineffective, because basing the behavior on whether CloseMainWindow() returns $false is insufficient - see below.

  • The solution will only work on Windows - even though sending SIGTERM would amount to equivalent functionality on Unix.


Note: I am partly out of my depth here, but I hope I'm at least fundamentally correct:

  • CloseMainWindow() returns $false only in two cases:

    • The application does have a (main) message loop (it is a GUI-subsystem application) but is currently in a state where it cannot process messages - a typical example is a modal dialog currently being shown.

    • The application does not have a message loop (a console-subsystem application), except if it has one indirectly, by running directly in a console window, which itself does have a message loop. This means:

      • A console application running directly in a console window - typically a shell - will typically respond with $true via its console window's message loop, and it is the console window that then terminates the application.

      • By contrast, a console application launched from a shell has no associated window and always returns $false.

      • In either case, a console application can optionally register for console events such as CTRL_CLOSE_EVENT, via the SetConsoleCtrlHandler WinAPI function, which allows them to stop gracefully, analogous to a process opting to handle SIGTERM on Unix.


This implies for your proposal that if -Graceful is specified:

  • All console applications (except shells) will effectively always be killed instead (as they always return $false).

    • To fix that, the target process' controlling console window would have to be identified and the .CloseMainWindow() method must be called on it - only that would give such console applications a chance to terminate gracefully, via their CTRL_CLOSE_EVENT handlers, if any; edit: however, that wouldn't be appropriate, because that means that other processes would be terminated too, at the very least also the parent shell process.
  • GUI applications may end up not terminated at all, because returning $true only indicates that the message was processed, not that termination will actually be performed.

    • In fact, a GUI application whose message loop is responsive may put up a modal dialog in response to .CloseMainWindow() (as in the Notepad example above) and indefinitely wait for user input. From PowerShell's perspective, such a Stop-Process -Graceful call would then amount to quiet failure: the process isn't terminated, and no error is reported.

Even if we address all the problems above - i.e. if we truly give all all targeted processes a chance to terminate gracefully - enforcing (ultimate) termination should (a) be opt-in and (b) can, as stated, only be achieved by waiting for actual termination based on a timeout, given that SIGTERM / CTRL_CLOSE_EVENT handlers can refuse to terminate in response to a request.

@SteveL-MSFT
Copy link
Member

@SteveL-MSFT SteveL-MSFT commented Feb 3, 2021

@PowerShell/powershell-committee reviewed this and agrees to not make a breaking change where automation will expect processes to be killed. .NET currently does not provide a way to send SIGTERM on Unix systems and no corresponding capability on Windows (where CloseMainWindow() is similar, but not the same as noted for console processes). This may be better served by a community module/cmdlet that is OS specific.

@iSazonov
Copy link
Collaborator

@iSazonov iSazonov commented Feb 4, 2021 •

Even if we address all the problems above - i.e. if we truly give all all targeted processes a chance to terminate gracefully - enforcing (ultimate) termination should (a) be opt-in and (b) can, as stated, only be achieved by waiting for actual termination based on a timeout, given that SIGTERM / CTRL_CLOSE_EVENT handlers can refuse to terminate in response to a request.

I believe we should do the cmdlet too smart and complex.
It is currently impossible to send an event or signal to a process so that it terminates gracefully.

The suggestion is to just add such a feature - just send the signal and nothing else.

All other smart things the user can do himself (or we can add later after receiving feedback).


We can implement this on Unix too https://stackoverflow.com/questions/41041730/net-core-app-how-to-send-sigterm-to-child-processes

Stack Overflow
Is it possible for .net-core app running on Linux to send SIGTERM signal to a child process?

We're thinking to port our .net app to .net-core and run it on Linux, to avoid current signal implement...

@mklement0
Copy link
Contributor

@mklement0 mklement0 commented Feb 5, 2021

Thanks, @iSazonov - good find, and I do think that starting small is an option, but let me flesh your suggestion out to see its full implications:

  • Existing Stop-Process behavior will remain as-is: unconditional kill behavior, asynchronous (even though in practice it will in effect typically be synchronous).

    • Unless killing fails (such as due to lack of permissions), it can safely be assumed that the target processes will terminate (and may (likely) or may not already have, by the time the next PowerShell statement executes).
  • Introduce only one new switch, -Graceful, which does the following:

    • On Unix, it sends signal SIGTERM, and on Windows it calls CloseMainWindow().
    • If sending the signal fails (typically due to a permissions problem) or CloseMainWindow() returns $false (see below), report a non-terminating error (per process).

Either way (after either failing or succeeding to send SIGTERM / call CloseMainWindow()), Stop-Process's job is done, which means:

  • Termination may not occur at all (only in the failure case do you have instant certainty), because the target processes may ignore the signal / close request.

  • If termination does occur, it may not occur until some time later (more noticeably so than with the default kill behavior).

To detect whether termination occurred, a separate Wait-Process call is needed, sensibly with a -Timeout argument.
If the processes don't terminate within the timeout period and termination must be enforced, another Stop-Process call is needed, this time without -Graceful, to effect killing.


When use of -Graceful is then meaningfully supported:

  • On Unix-like platforms:

    • Any process can be targeted; if a given process doesn't have a SIGTERM handler, the system will kill it.
  • On Windows:

    • GUI applications can be targeted.
    • (Console-based) shells can be targeted; more accurately, any console application that directly has a console (window) attached to it.

This means that console applications launched from a shell can NOT be targeted (such as node.exe started from a shell with a script that runs a local webserver, for instance):

  • CloseMainWindow() invariably returns $false for them, so Stop-Process will report a non-terminating error.

    • Unless we want to go to the trouble of detecting the application type (GUI vs. console), the error message should mention both potential failure reasons: the target is either (a) a GUI app that is in a state where it cannot process messages or (b) an app that has no associated window and therefore no message loop, such as a console app (with no console directly attached).
  • It seems that Windows currently fundamentally doesn't support sending a terminating request (CTRL_C_EVENT, CTRL_BREAK_EVENT, via GenerateConsoleCtrlEvent) to a single process running (indirectly) inside a console - except if it was explicitly created as part of a process group; see microsoft/terminal#335 for (very in-depth) background information.

    • The only way to get such processes to (potentially) terminate gracefully is if the (indirectly attached) console window as a whole is closed, but that is inappropriate, if only a single process running inside that window is to be targeted.

If everyone agrees that this - initially minimal - functionality is still beneficial and the implications are understood and well-documented, I think it's worth doing.

@iSazonov
Copy link
Collaborator

@iSazonov iSazonov commented Feb 6, 2021 •

@mklement0 I think it makes no sense for us to try to do something too clever since not even the platforms themselves do it. (Moreover, there are differences in async/sync behavior.)
If SIGTERM always fallback to SIGKILL then we are forced to follow this. The same should be the case on Windows - if CloseMainWindow() returns false then just call Kill().

if (Graceful.Present && TryStopProcessGacefully())
{
    // return;
}
else
{
    process.Kill();
}
...

void TryStopProcessGacefully()
{
#if UNIX
    SendTerminateSignal(process);
    return true;
#else
    return CloseMainWindow();
}

For reference - Process.Kill() on Unix to implement SendTerminateSignal() https://source.dot.net/#System.Diagnostics.Process/System/Diagnostics/Process.Unix.cs,57

@mklement0
Copy link
Contributor

@mklement0 mklement0 commented Feb 6, 2021 •

it makes no sense for us to try to do something too clever

I was proposing the very opposite:
I was proposing to simply use the underlying platform feature, without trying to superimpose any additional logic (which I thought you were advocating for):

SIGTERM always fallback to SIGKILL

  • SIGTERM doesn't fall back to termination by the system:

    • Termination by the system is the default behavior that any process may choose to modify by implementing a signal handler.
  • Similarly, .CloseMainWindow() doesn't fall back to closing the window:

    • The system closing the window is the default behavior that any process (that actually has a main window) may choose to modify by responding to the close message.
    • Calling .CloseMainWindow() on a process for which there is fundamentally no window to close - i.e., one without a message loop (such as a console app launched from a shell) - predictably and sensibly fails ($false).

In the success case - being able to send the signal / close message - the uncertainty over whether that signal / message will eventually, possibly asynchronously be honored is built into both mechanisms.

Not trying to resolve this uncertainty through superimposed logic is the gist of the previous proposal.

Users who care about the eventual outcome must use follow-up commands, as described, at least for now.

The only challenge I see is to make users understand the limitations of what processes can be targeted on Windows.

@mklement0
Copy link
Contributor

@mklement0 mklement0 commented Feb 6, 2021 •

@iSazonov: Sorry, I misread your previous comment: there is a fundamental disagreement here: I think we should not fall back to killing, for the reasons stated.

Later, we can implement superimposed high-level logic, through additional parameters.

@PsychoData
Copy link

@PsychoData PsychoData commented Feb 6, 2021 •

The only reason that we started talking about sending SIGterm, but then maybe waiting for a time out of some sort and then sending sigkill was that way we could preserve the high-level functionality that the processes will be stopped once the cmdlet is done running. Or if there was some error, throw an exception/error.

There should probably be some option to just send the close event without sending the kill, but foremost we should preserve the existing functionality so we don't break current deployments.

I really don't think that using a separate wait-process or synchronously waiting with .WaitForExit would be a good idea at all, because that process doesn't have a way to short circuit out if it is taking 20 minutes to close.

I looked at the code in the dotnet core clr , and there is definitely going to be no benefit to this on Unix currently, but windows still could benefit.
On the windows side the process for sending the actual commands to the processes currently only lists like three commands - close, kill, stop, resume - And it looks like that might could be extended, but that would be a separate argument for a separate repo with a separate team most likely.

I'll try to hack together some code to demo the functionality, because I feel like all of these other extensions that have been discussed could certainly be useful, but the feature bloat from the original goal is significant.

If the dotnetCLR finally gets updated to have support for sending more types of events (preferably including arbitrarily sending whichever signal we want - like the Unix kill command) then that seems like it would be the time to revisit this and abstract the sending signals to some other function possibly. In the meantime, sending closeMainWindow would be sufficient for many Windows services, tray agents, and other processes the gracefully close themselves rather than having to be killed, and it is the closest option that we have

@mklement0
Copy link
Contributor

@mklement0 mklement0 commented Feb 6, 2021 •

that the process will be stopped

  • With killing (the current behavior), you get that for free (although not strictly synchronously), but not by explicit high-level logic design, but by a straight pass-through to the underlying system functionality.

  • With (potentially cooperative) termination, the underlying system functionality offers no stopping guarantee.

There should probably be some option to just send the close event without sending the kill

That's the current proposal: -Graceful by itself would just be a platform-abstracted way to send SIGTERM on Unix, and to call .CloseMainWindow() on Windows.

I definitely would like to see the high-level functionality of ensuring that the process is stopped, but the above would be a fairly simple and straightforward start whose behavior doesn't deviate from the underlying system mechanisms.

If SIGTERM / .CloseMainRequest() defer to the target process with respect to whether it actually terminates, so should we by default.


As for a (possibly later) enhancement that builds on the above:

First, I agree that general signal support should be a separate discussion.

I'm always a fan of desired-state functionality, but I believe it should be opt-in here, to modify the underlying system behavior on demand.

If you're asking a process to terminate, it is not a given that your intent is to kill it, if it refuses to / doesn't terminate within a given timeout.

I really don't think that using a separate wait-process or synchronously waiting with .WaitForExit would be a good idea at all

Of course, using a timeout makes sense to prevent infinite waiting.

But users should have a choice as to:

  • what happens on timeout: give up, or kill?

  • how long to wait - different timeouts may be appropriate for different processes / scenarios.

If we do go with a default timeout (and I have no idea what period would make sense, but it definitely must be clearly documented), it should only apply if you've opted into fallback-to-killing behavior, with a switch.

That leads me to Options D and E:

What they share:

  • Retain existing behavior, and make the new -Graceful switch asynchronous too: if -Graceful succeeds, it just mean that the request was successfully sent, and termination may or may not occur, and there's no guarantee when. (So far, this is the minimal enhancement suggested).
    • Stop-Process alone would be PowerShell's cross-platform way of killing a process (which is an unfortunate default, but we're stuck with).
    • Stop-Process -Graceful would be PowerShell's cross-platform way of requesting termination - with no guarantee that the request will be honored - and I'm now again tempted to bring the word request in.

Option D: with a default timeout for the fall-back-to-killing opt-in:

Stop-Process ... -Graceful [-KillOnTimeout [-Timeout <double>]]

That is, -KillOnTimeout is the opt-in to the kill fallback, and by itself uses a default timeout, which can overridden with -Timeout (which must be a nonzero, positive value).

(Unlike without the opt-in, .CloseMainWindow() returning $false should then not result in a non-terminating error and should be treated like an instant timeout that triggers killing.)

Option E: without a default timeout:

This forces users who want to ensure ultimate termination after request-based termination fails to specify a timeout explicitly - though I can see how that would be cumbersome if a reasonable default timeout can be provided.

Stop-Process ... -Graceful [-KillTimeout <double>]
@mklement0
Copy link
Contributor

@mklement0 mklement0 commented Feb 6, 2021 •

Actually I missed that the current behavior actually does make an attempt to gracefully terminate some processes, namely (by definition Windows-only) services (though note that the conditional is not platform-specific and tests just by process name):

if (string.Equals(SafeGetProcessName(process), "SVCHOST", StringComparison.OrdinalIgnoreCase))
{
StopDependentService(process);
}

(As an aside: the comment describing the -Force switch doesn't match its behavior:

/// <summary>
/// Specifies whether to force a process to kill
/// even if it has dependent services.
/// </summary>
/// <value></value>
[Parameter]
[ValidateNotNullOrEmpty]
public SwitchParameter Force { get; set; }

)

That said:

  • On Unix, this doesn't apply, and calling Stop-Process always means: send a kill request.

  • On Windows, this accommodation for services only makes for an awkward inconsistency (especially given that there's a dedicated, graceful-shutdown Stop-Service cmdlet - but, again, one we're stuck with.

@PsychoData
Copy link

@PsychoData PsychoData commented Feb 6, 2021 •

the goal isn't full-synchronous killing.

It's just as close as the libraries let us get to async, short of them being rewritten to send the signals directly and just move on

similar to Unix, while maintaining the vanilla Stop-Process Notepad capability would be to just let it

  • send the CloseMainWindow() event - which just sends the message to the Window Handle through Interop services
  • give the process at least a few milliseconds to handle that close request and work on closing (the timeout)
  • send the Kill event, if our timeout expires and the process hasn't stopped
  • At this point Stop-Process is done with it's functionality, and it exits, with the only final action it does currently being to kill the current Process if it was instructed to do so

https://github.com/PowerShell/PowerShell/blob/master/src/Microsoft.PowerShell.Commands.Management/commands/management/Process.cs#L1281-L1290

A few Example flows,

Stop-Process Notepad where Notepad has NO UNSAVED text entered and will NOT INTERRUPT the CloseMainWindow()

  • Processing Starts
  • CloseMainWindow() is sent
  • Timer for Timeout is started
  • Processing ends
  • EndProcessing Starts
  • Loop until (Timer ends or Notepad finishes closing)
  • Notepad will likely finish Closing and the loop will exit
  • Stop-Process ends

Stop-Process Notepad where Notepad HAS UNSAVED text entered and WILL INTERRUPT the CloseMainWindow() to prompt the user to save

  • Processing Starts
  • CloseMainWindow() is sent
  • Timer for Timeout is started
  • Processing ends
  • EndProcessing Starts
  • Loop until (Timer ends or Notepad finishes closing)
  • Notepad will NOT stop before the timeout and a .Kill() will be sent to finish the job
  • Stop-Process ends without checking if the process finished exiting

Stop-Process Outlook

  • Processing Starts
  • CloseMainWindow() is sent
  • Timer for Timeout is started
  • Processing ends
  • EndProcessing Starts
  • Loop until (Timer ends or Outlook finishes closing)
  • Outlook may finish closing, or timer might expire
  • Stop-Process ends

I don't think this would be the right place to go adjusting the flow of the Stop-Process way of handling the Services stopping. That's a different discussion in a different issue.

GitHub
PowerShell for every system! Contribute to PowerShell/PowerShell development by creating an account on GitHub.
@mklement0
Copy link
Contributor

@mklement0 mklement0 commented Feb 6, 2021 •

short of them being rewritten to send the signals directly and just move on

The existing mechanisms are:

  • asynchronous from a waiting-for-actual termination perspective
  • synchronous with respect to waiting until the signal has been sent / the message has been posted, both of which are implementation details and happen before the actual point in time of termination, if any.

the goal isn't full-synchronous killing.

Once you superimpose (request-termination-then-)fallback-to-kill logic, you get quasi-synchronicity even if you don't call .WaitForExit() after .Kill(), given that killing is near-synchronous.

And, of course, if the process terminates within the timeout period, you have synchronous behavior by definition.

To put it differently: what you're invariably looking for is synchronicity with respect to knowing that actual termination will occur (was successfully initiated), which, if a termination request is first sent invariably involves waiting.

I don't think this would be the right place to go adjusting the flow of the Stop-Process way of handling the Services stopping.

Agreed - that's why I said we're stuck with the behavior.

Also, just to remind us, the committee has already turned down any enhancement here, so this may never happen or at least not anytime soon.

I'd say the only chance for this to be revisited is if we agree on a way forward that also addresses the committee's concerns, and present that in a new, focused feature-request issue.

The committee's concerns were:

  • The change should not be breaking.

    • I'm not sure your proposal qualifies (see below).
  • .NET currently does not provide a way to send SIGTERM on Unix systems and no corresponding capability

    • @iSazonov has demonstrated that it would be fairly easy to implement it ourselves.
  • no corresponding capability on Windows [...] for console processes

    • With a kill fallback this would be somewhat mitigated, but suboptimally in that such (no-directly-attached-console) console applications would then always be killed.

    • However, given that Windows itself offers no such capability, you could argue that this best-effort approach is still preferable (and once Windows does offer the ability, the cmdlet could be amended).


Do I understand correctly that you want to bake the request-first-then-fallback-to-kill logic into Stop-Process by default?

Even though I can see the appeal of this from the perspective of trying to terminate gracefully while ultimately ensuring termination, it does constitute a breaking change:

  • existing code may rely on forceful termination, i.e. explicitly not giving the process a chance to clean up.

  • the invariably necessary waiting period (except, on Windows, if .CloseMainWindow() returns $false) slows down each call.

Even if everyone were comfortable with this change, you would then need a switch such as -NoKillFallback to allow request-only functionality, and there should still be a user-specifiable -Timeout.

@PsychoData
Copy link

@PsychoData PsychoData commented Feb 6, 2021

I don't know - I just tried to kick something that seemed like a good idea along.

From the very beginning I was saying I didn't think I was best for this because I knew it would be a breaking-ish change and there would likely need to be considerations for it to preserve the .Kill() effect directly by default, even though they had perfectly valid alternatives or fairly minor mitigations, like a single delay of maybe 500 millis for a -ByRequest TimeOut, would probably need additional parameters added (Which I'm not sure how to add in .cs) to bypass the falling back to Kill() after timeout expired, possibly even more.


But it would be very useful for any program that properly handled a Close request, and give IT a MUCH easier way to gently close processes, without them.

For example, if I wanted to issue a restart - nothing would stop me from saying Get-Process | Stop-Process -RequestCloseOnly or maybe Get-Process | where {From Logged in User} | Foreach-Parallel { $_ | Stop-Process -RequestCloseOnly } to close every open process, as long as it wasn't something like Notepad with unsaved text, before sending a restart command.


Someone else can try to chase this down if they want, but my effort to get this done is though, because my skill level was spent before I ever made the PR when @iSazonov was pushing me to, exactly like I said it would be.

@iSazonov
Copy link
Collaborator

@iSazonov iSazonov commented Feb 7, 2021

SIGTERM always fallback to SIGKILL

  • SIGTERM doesn't fall back to termination by the system:

I mean if a process doesn't implement SIGTERM handler a stopping behavior will be like SIGKILL.
Thus, both on Windows (CloseMainWindow() returns a false) and on Unix (no SIGTERM handler) there may be processes that cannot "gracefully stop" and will just kill.
And my code snippet above reflects this.

@mklement0
Copy link
Contributor

@mklement0 mklement0 commented Feb 7, 2021

You're right, but that means that on Unix the else { process.Kill(); } branch will never be reached - yet the process may not terminate, if the process does have a SIGTERM handler but decides not to terminate.

On Windows it shouldn't be reached, at least not by default, because that would mean superimposing destructive logic on the underlying system behavior.

With an opt-in such as -KillOnTimeout it would be fine, however: .CloseMainWindow() returning $false could then rightfully be treated like an instant timeout; however, on successfully sending SIGTERM / calling .CloseMainWindow() you'd still have to wait, because you otherwise cannot ensure that termination will occur.

If the intent is for -Graceful to default to -KillOnTimeout, we would need a switch that opts out of the kill fallback - unless you think that that's not needed.
Either way, this would mean a departure from the behavior of the underlying system call, whereas my thinking was that by default we should simply expose the underlying system behavior as-is.

To summarize:

  • I think touching the default behavior - unconditional killing - is too much of a breaking change (as unfortunate as that default is).

  • This means that termination by request requires at least one opt-in switch, such as -Graceful.

  • This leaves the question of whether this switch alone should effect try-gracefully-with-fallback-to-kill logic, or whether it should simply defer to the underlying system calls - which have no fallback.

    • Deferring to the underlying system calls would make Stop-Process -Graceful behave the same as taskkill on Windows (without /f, the kill switch) and kill on Unix (without -9, the kill switch); in other words: we'd have consistency, albeit with the defaults reversed (unavoidably, for backward compatibility).
    • This means:
      • Like taskkill and kill by default, -Graceful (alone) would be happy to send the termination request, and would consider that alone success, without ensuring ultimate termination (that is up to the target process, if it has a termination-request handler).
      • If sending fails, an error would occur, which on Windows would predictably occur for any console application without a directly attached console (any process without a window message loop); taskkill provides a meaningful error message in this case: ERROR: The process "<pname>" with PID <pid> could not be terminated. Reason: This process can only be terminated forcefully (with /F option).
@mklement0
Copy link
Contributor

@mklement0 mklement0 commented Feb 7, 2021 •

@PsychoData

I just tried to kick something that seemed like a good idea along.

Agreed - I do think we should provide this functionality, but the tricky part is how.

Thanks for the discussion; even if no immediate action follows, I think it was useful to get clarity.

Which I'm not sure how to add in .cs

Note that it's perfectly fine to only contribute conceptually to a discussion, without being the implementer or needing to know all technical details.

I hope that it's clear that the sticking point here is the up-front conceptual work - agreeing on the end-user experience and assessing backward-compatibility concerns - and that just happened to turn out much more complex than originally anticipated.

@iSazonov
Copy link
Collaborator

@iSazonov iSazonov commented Feb 8, 2021

@mklement0 I think we all are in consensus that we don't change the default behavior of the cmdlet and we all find the new Graceful option being useful.
Perhaps we need to rename Graceful switch to TryGraceful to explicitly say users that the cmdlet does not guarantee that a process will necessarily stop (due to the way the operating systems work). I suppose we don't need to worry about how processes will react to SIGTERM and CloseMainWindow(). This specific can be described in the documentation. I guess my code snippet is the best compromise. I'm sure this will cover most custom scenarios. More complex cases can always be resolved with additional commands.

@mklement0
Copy link
Contributor

@mklement0 mklement0 commented Feb 8, 2021 •

@iSazonov

think we all are in consensus that we don't change the default behavior of the cmdlet and we all find the new Graceful option being useful.

👍

Perhaps we need to rename -Graceful switch to -TryGraceful

👍

I suppose we don't need to worry about how processes will react to SIGTERM and CloseMainWindow().

That is definitely an option - we can keep desired-state logic out of the cmdlet for now, and possibly enhance later.

I guess my code snippet is the best compromise.

No, I don't think so, because by default there should be no fallback to killing - just like taskkill without /f (the equivalent of -TryGraceful) does not kill if .CloseMainWindow() returns $false - instead, it reports an error that tells you how to KILL if you really want to.

In terms of your snippet, this means:

if (TryGraceful.Present)
{
   if (! TryStopProcessGacefully()) 
  {
     // Report non-terminating error along the lines of (obviously needs polishing):
     // "Graceful termination not possible (the process either doesn't support it at all (no message loop) 
     // or cannot process messages in its current state); to kill the process, call without -TryGraceful"
  }
}
else
{
    process.Kill();
}

Again, desired-state logic is desirable, but falling back to killing only if the close-request cannot even be sent makes for half of an ensured-termination feature: a successfully sent request may still result in non-termination, which means that you haven't ensured termination overall.

A proper ensured-termination feature would require waiting for termination (if the request was successfully sent, otherwise you can kill instantly), which introduces the need for a timeout (at least a default one, but ideally also a user-specifiable one).

(Note that on Unix the case where SIGTERM cannot even be sent likely means a fundamental problem, such as insufficient permissions, which means that SIGKILL wouldn't work either; therefore, this case should just result in an error; this could also happen with .Kill() on Windows)

In terms of syntax, this means:

  • If we initially go without ensured-termination in the picture we would only need -TryGraceful
Stop-Process ... [-TryGraceful]
  • If and when we decide to implement ensured-termination logic:
Stop-Process ... -TryGraceful [-Force [-Timeout <double>]] 
  • -Force is the best choice to signal the intent to fallback to killing, and I think it's defensible to give the existing -Force switch this semantics when combined with -TryGraceful.

  • Unless -Timeout is also specified, a well-documented default timeout would apply before killing is resorted to; this default timeout should probably apply per input process, whereas a user-specified one should apply overall.

@iSazonov
Copy link
Collaborator

@iSazonov iSazonov commented Feb 9, 2021

@mklement0 If on Windows we can detect whether a process can handle a close event (CloseMainWindow() returns false), on Unix we cannot detect this for SIGTERM. (If we haven't permissions we will get an exception in any case.) I'd prefer to have unified behavior for all OS-s. It is first argument to do not throw and just send a close event.
Second argument is there are not benefits for users to get an error. If users initiate a grace stopping but don't care whether the process was really stopped the error makes no sense. If users care about that the process was really stopped they should explicitly check this (with Wait-Process) in any case (is there an error or not).

@mklement0
Copy link
Contributor

@mklement0 mklement0 commented Feb 9, 2021 •

It is first argument to do not throw and just send a close event.

Sending a close event: yes.

If the system tells you that it cannot, you report a non-terminating error (rather than throwing), just as you would with a permissions problem with SIGTERM on Unix).

This amounts to unified behavior with respect to the underlying system capabilities.

Second argument is there are not benefits for users to get an error. If users initiate a grace stopping but don't care whether the process was really stopped the error makes no sense

They may care about being able to request graceful stopping, but leaving it up to the target process to comply (see below).
That is what the underlying system mechanisms provide - even if that is unsatisfying from a desired state viewpoint.

On Unix, the processes almost always comply - from what I can tell, even GUI applications such as gedit and Firefox, which - unlike GUI applications on Windows - do not pop up a modal dialog if their data is "dirty", and quietly terminate (potentially due to not even having a signal handler).

That is, on Unix, where only truly exceptional conditions (such as lack of permissions) prevent sending the signal, sending SIGTERM will typically result in termination - but that termination is not guaranteed, given that if the process has a signal handler, it may refuse (or the signal handler may crash).

On Windows, you're much more likely to run into non-termination:

  • (a) applications without a message loop (such as console applications called from a shell) fundamentally cannot accept the "signal" (the window message sent by .CloseMainWindow().

  • (b) applications that do have a message loop and accept the "signal" will predictably not terminate if they're GUI applications in a "dirty" state that causes them to pop up a modal confirmation dialog.

In the case of (a), you deserve to know that graceful termination is fundamentally impossible (an unfortunate limitation of Windows) - this is what taskkill already does.

In the case of (b), the only assurance you have is that the signal was "sent" - and there's a definite chance that termination will not occur.

If you implement a fallback to killing just for (a), you introduce an awkward asymmetry that additionally hides a system capability: to request termination without enforcing it.

Conversely, you have not ensured overall that termination will take place.


The only unified platform-neutral, desired-state behavior that is worth providing is the one that would (a) require opt-in via Force (indicating that you want to fall back to killing) and (b) therefore requires a more complex implementation with timeout-based waiting, which is the only way to guarantee termination.

@iSazonov
Copy link
Collaborator

@iSazonov iSazonov commented Feb 10, 2021

You seem to be ignoring the fact that the "grace stopping" is only a function of the application and it works only as the developer implemented it. There can be an infinite number of implementations. There's not even precise feedback (from the API) even on Windows. Moreover, there is not even a predefined semantics, even on Windows - the developer can assign any action to the event. There is no point in looking for something in common and trying to make a universal solution.
We can find many interactive programs that behave differently. So MS Word could ask to save a file, but VS Code couldn't. If we pay attention to services, they behave very differently from interactive applications. For example, if we send a SIGTERM to a web server, it could not mean stopping it, but a cold restart.
All we can do is send a signal and forget. We do this in Stop-Process only because we hope that, in general, application developers follow common practice for this event, however each implementation will be different for each application. We can do more only for a specific application by implementing the necessary logic in the commands following the Stop-Process in a script.

@mklement0
Copy link
Contributor

@mklement0 mklement0 commented Feb 10, 2021 •

All we can do is send a signal and forget.

That's what we can and should do in the simplest case (-TryGraceful alone).

If sending the signal / calling .CloseMainWindow() fails, we should report that as an error, which on Windows - unfortunately, but that can't be helped - categorically includes console applications not directly running in a console (as is typical, given that they're usually launched from a shell, which is the one running directly in the console; should Windows in the future ever provide a termination-request mechanism for such applications, we can amend the cmdlet, if that hypothetical mechanism requires something other than .CloseMainWindow(), which would be a misnomer in that case).

(If sending succeeds, we're done and move on, as you suggest,)

As stated, we could do this in a first step, and implement ensured-termination logic (see below) later, if ever.

We can do more only for a specific application by implementing the necessary logic in the commands following the Stop-Process in a script.

No, as proposed, there is something we can do:

If requested, we can ensure that the process terminates by waiting for termination - whether with a default or specified timeout period - and if termination hasn't occurred within that timeout - for whatever reason (we can't know) - kill the process then.

Of course, anyone can implement that logic themselves using the existing capabilities (assuming -TryGraceful alone has been implemented), but that's fairly cumbersome, so
we would be offering this as built-in desired-state logic, as a courtesy.

@mklement0
Copy link
Contributor

@mklement0 mklement0 commented Feb 10, 2021 •

It sounds like the only sticking point is what to do if .CloseMainWindow() returns $false with -TryGraceful (alone):

  • Approach A: Report an error and do nothing else; this is what taskkill.exe does.
  • Approach B: Automatically and quietly fall back to killing; this is what you're advocating, if I understand correctly.

I find B problematic, but I can also see the appeal of its pragmatism - though it's important to understand that the kill fallback will not apply to GUI applications that accept the message and then pop up a confirmation dialog.

If everyone is comfortable with it, so be it.

@iSazonov
Copy link
Collaborator

@iSazonov iSazonov commented Feb 10, 2021 •

It sounds like the only sticking point is what to do if .CloseMainWindow() returns $false with -TryGraceful (alone):

Yes. It is only about Windows.

Of course, anyone can implement that logic themselves using the existing capabilities (assuming -TryGraceful alone has been implemented), but that's fairly cumbersome, so
we would be offering this as built-in desired-state logic, as a courtesy.

And yes, and no. We could do something smart if CloseMainWindow() did something smart. But see the method implementation:
https://github.com/dotnet/runtime/blob/8a52f1e948b6f22f418817ec1068f07b8dae2aa5/src/libraries/System.Diagnostics.Process/src/System/Diagnostics/Process.Win32.cs#L240-L260

This method does nothing smart. It fast return false if no main window is or it is disabled - why do we need to write an error if such application makes no distinction between killing and grace stopping? For such application killing is the same as grace stopping.
And as we can see if CloseMainWindow() sends WM_Close event it returns always true - not a result from the application as we could expect! It is exactly as on Unix for SIGTERM.
There is recommendation for processing WM_Close in docs but this does not mean that everyone will do exactly that. For example, VS Code does not ask to save the file on close, unlike MS Word.

So on all OS-s the result is unpredictable - a script write is forced to always check the result he expects in his particular scenario - no errors help.

GitHub
.NET is a cross-platform runtime for cloud, mobile, desktop, and IoT apps. - dotnet/runtime
@mklement0
Copy link
Contributor

@mklement0 mklement0 commented Feb 10, 2021

It fast return false if no main window is or it is disabled - why do we need to write an error if such application makes no distinction between killing and grace stopping?

Because you may choose not to kill if graceful termination isn't possible.

Consider this scenario: Notepad is open with an unsaved file, and a modal dialog happens to be displayed in it: even though graceful termination is possible with a GUI application such as Notepad in principle, in this case .CloseMainWindow() returns $false and will result in data loss if you fall back to killing.

With your proposal you'll get reliable termination only when .CloseMainWindow() returns $false.

If it returns $true, given how GUI applications on Windows respond with unsaved data, there's a good chance you won't get termination at all - which you can - cumbersomely - test for manually, or, as a desired-state convenience feature, we can offer this functionality via Stop-Process on an opt-in basis; see next point.

By contrast, what I proposed would amount to -TryGraceful simply being a wrapper for .CloseMainWindow() on Windows, and its situational failure - indicated by a return value of $false - would be surfaced in a PowerShell-appropriate manner, as a non-terminating error.

It is then up to the user to decide whether killing is appropriate.
In the simplest case the fallback-to-kill logic as an explicit choice could then be achieved with:
Stop-Process -TryGraceful $somePid 2>$null || Stop-Process $somePid

We could do something smart

We can do something predictable that ensures a desired outcome, namely reliable termination - covering both the cannot-send-signal and signal-sent-but-process-didn't-terminate scenarios, as previously described.

It would be a convenience feature that compensates for the lack of predictability of the underlying system features.

As stated, it requires additional parameters and a more complex implementation.

@mklement0
Copy link
Contributor

@mklement0 mklement0 commented Feb 12, 2021 •

Let me try to bring closure to this by summarizing our options:

  • Existing behavior remains as-is: target processes are killed by default.

  • A conceptually simple and therefore easy-to-implement solution is to implement (only) a -TryGraceful switch:

    • It sends a request to terminate to a target process, which it may or may not honor, but that's not Stop-Process' concern; the request is sent:

      • in the form of the SIGTERM signal on Unix
      • by calling .CloseMainWindow() on Windows.
    • On Unix, failing to send SIGTERM represents a condition that leaves reporting an error as the only option (it's likely a permissions problem, which means that SIGKILL cannot be sent either).

      • Successfully sending SIGTERM typically but not necessarily results in actual, quiet termination, even of GUI applications.
        • On macOS, native GUI applications are capable of avoiding data loss by preserving unsaved changes (even in new, never-saved documents) through a feature called Resume.
        • On Linux, I've only looked at Ubuntu 18.04: data loss does occur, such as in GEdit; Linux systems come with a variety of GUI shells - I don't know how they behave.
    • On Windows, we have a choice as to how to respond to .CloseMainWindow() indicating failure by way of returning $false:

      • Context:

        • $false is returned in two distinct scenarios:

          • the target process has no message loop, either because it isn't a GUI-subsystem application or because it is a console-subsystem application that doesn't directly run in a console window (which - typically, but not necessarily - only shell processes do)
          • the target process does have a message loop, but is unable to receive messages at the moment, either because it hangs or because its GUI is showing a modal dialog that blocks the main message loop.
        • $true just means that the message was successfully sent, but with GUI applications there is a distinct chance that they will not terminate, namely if they're in a "dirty" state that causes them to pop up a modal dialog to ask the user to confirm the intent to terminate either with or without saving.

      • Implementation options for the case when $false is returned:

        • Option 1: Report a non-terminating error and take no further action.

          • This follows the default behavior of taskkill.exe (using option /f requests kill behavior).
          • If the user wants to ensure termination, they'll have to call Stop-Process again, without -TryGraceful.
        • Option 2: Fall back to killing the process, which has the following implications:

          • Most console applications (except those running directly in console windows, which are typically only shells) are always killed.
          • GUI applications that hang and those that happen to be showing a modal dialog are killed.
          • All other applications either terminate gracefully, refuse to terminate, or, in the case of GUI applications, delegate the termination decision to the user by popping up a modal confirmation dialog.
    • On both platforms, if ultimate termination is to be ensured in all cases, the user will have to use additional commands in order to wait for a while (Wait-Process) and, if waiting times out, call Stop-Process without -TryGraceful to effect termination by killing.

  • A more complex enhancement that provides ensured-termination logic can be added later, which then needs to incorporate the timeout-based waiting and kill fallback into Stop-Process itself.

@iSazonov
Copy link
Collaborator

@iSazonov iSazonov commented Feb 12, 2021

To get a consistency on all platforms we could not use CloseMainWindow() but our custom method without first two checks - only send WM_Close event - we would get a behavior like with SIGTERM on Unix - send and forget. (If an user want exactly CloseMainWindow() behavior he can call it directly.)

@mklement0
Copy link
Contributor

@mklement0 mklement0 commented Feb 12, 2021

If you really wanted to do that, you could simply ignore .CloseMainWindow()'s return value.

However, to me that's not consistency - that's just hiding an error condition from the user, given that he intent of requesting termination could not be fulfilled.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Linked pull requests

Successfully merging a pull request may close this issue.

None yet
5 participants