Remove Runaway Process Killer #440
Merged
Conversation
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
There was a problem hiding this comment.
I wish I could say this is fine, but I do not have enough expertise to confirm that easily. IMHO it needs more discussion before merging. Some notes:
- It is not really a deprecation, it is a removal. Some options like stopTimeout and parent termination have no replacement in the new implementation.
- The CREATE_BREAKAWAY_FROM_JOB flag is supported in Windows versions we implicitly target. https://docs.microsoft.com/en-us/windows/win32/procthread/process-creation-flags
- The change will enable the process termination behavior by default. So it will alter the behavior for those who have not enabled Runaway Process Killer. I do not mind doing that, but maybe we should offer an escape hatch parameter which disables process termination
My preference would be to keep it for 3.0. We could also introduce a new opt-in native system process termination support without removing the RunawayProcessKiller first
doc/selfRestartingService.md
Outdated
| @@ -10,6 +10,6 @@ This hidden command is a flavor of the `restart` operation, | |||
| where winsw creates another winsw process in a separate process group, | |||
| and restarts the service from there. | |||
|
|
|||
| This additional indirection is necessary because Windows Service Control Manager (SCM) will kill child processes recursively when it stops a service. | |||
| SCM doesn't provide the restart operation as an atomic operation either, so winsw implements restart by a sequence of stop and start. | |||
| This additional indirection is necessary because winsw will kill child processes recursively when stopping. | |||
There was a problem hiding this comment.
Suggested change
| This additional indirection is necessary because winsw will kill child processes recursively when stopping. | |
| This additional indirection is necessary because WinSW will kill child processes recursively when stopping. |
Closed
|
I suggest moving forward. With a few bug fixes and stability updates WinSW rarely crashes or leaves child processes aside. This will work when a serious crash (CSE/AVE/SOE) or process kill (by a user or SCM) occurs. If the service stops normally (under control) we can turn it off.
|
|
I agree
…On Sun, Aug 2, 2020, 05:29 Next Turn ***@***.***> wrote:
I suggest moving forward. With a few bug fixes and stability updates WinSW
rarely crashes or leaves child processes aside. This will work when a
serious crash (CSE/AVE/SOE) or process kill (by a user or SCM) occurs. If
the service stops normally (under control) we can turn it off.
- The CREATE_BREAKAWAY_FROM_JOB flag is supported since Windows XP.
—
You are receiving this because you were mentioned.
Reply to this email directly, view it on GitHub
<#440 (comment)>, or
unsubscribe
<https://github.com/notifications/unsubscribe-auth/AAW4RIGOW5IUAG76RFAMQ6LR6TMSFANCNFSM4LKRLDHA>
.
|
Closed
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
Shifts the responsibility onto the operating system.
This feature is now safely enabled by default.