process.kill can't kill process group on Windows #3617

kflu · 2015-10-31T21:33:35Z

Per process.kill doc, it "Send a signal to a process". That indicates the API is intended to be used against a single process. However, it then refers to kill(2) man page, which states that the API can also be used against a process group, when pid < -1. On Windows, it clearly doesn't support the pid < -1 case.

Node.js is intended to provide a consistent bahavior as much as possible for cross-platform developing. That means either this support should be added for Windows, or document should be updated and warns developers on the usage.

See Unitech/pm2/issues/1664 for more detail.

> var x = child_process.exec('notepad');
undefined
> x.pid
3040
> process.kill(-x.pid)
Error: kill ESRCH
    at exports._errnoException (util.js:874:11)
    at process.kill (node.js:774:15)
    at repl:1:9
    at REPLServer.defaultEval (repl.js:164:27)
    at bound (domain.js:280:14)
    at REPLServer.runBound [as eval] (domain.js:293:12)
    at REPLServer.<anonymous> (repl.js:393:12)
    at emitOne (events.js:82:20)
    at REPLServer.emit (events.js:169:7)
    at REPLServer.Interface._onLine (readline.js:210:10)

The text was updated successfully, but these errors were encountered:

kflu · 2015-11-01T06:57:49Z

Took a further look. It calls into uv_process_kill (node\deps\uv\src\win\process.c), -> uv__kill -> TerminateProcess. It looks like it needs to be smarter than just TerminateProcess.

bnoordhuis · 2015-11-01T13:20:11Z

Node.js is intended to provide a consistent bahavior as much as possible for cross-platform developing. That means either this support should be added for Windows, or document should be updated and warns developers on the usage.

Stress on 'as much as possible'. :-)

Windows doesn't have a direct equivalent to UNIX process groups. Documentation pull requests welcome.

Fishrock123 · 2015-11-02T15:35:20Z

Marked as doc since I believe @bnoordhuis is suggesting this isn't possible on windows.

kflu · 2015-11-02T17:43:00Z

@Fishrock123 @bnoordhuis No that is not true. Windows does have process groups and is capable of sending signals to the group. It looks like it's something completely doable and it's really good to have a matching behavior to make Node API consistent across platforms. Can we consider fixing this rather than just changing the docs?

Quoted from documentation.

When a process uses the CreateProcess function to create a new console process, it can specify the CREATE_NEW_PROCESS_GROUP flag to make the new process the root process of a console process group. The process group includes all processes that are descendants of the root process.
A process can use the GenerateConsoleCtrlEvent function to send a CTRL+C or CTRL+BREAK signal to all processes in a console process group. The signal is only received by those processes in the group that are attached to the same console as the process that called GenerateConsoleCtrlEvent.

bnoordhuis · 2015-11-02T19:05:12Z

Windows does have process groups and is capable of sending signals to the group.

Windows process groups are a pale shadow of UNIX process groups: you cannot send arbitrary signals, only C-Break. Job objects come closer in some ways but still not close enough.

Hacking in support for either is not worth the effort IMO. You're welcome to try though.

kflu · 2015-11-03T05:36:17Z

Windows process groups are a pale shadow of UNIX process groups: you cannot send arbitrary signals, only C-Break.

That is true even today with Node's process.kill() implementation on Windows.

Hacking in support for either is not worth the effort IMO. You're welcome to try though.

Considering that the hack already exists for single process signaling, it may not be hard to extend it for process groups. I'll see if I can find sometime working on this. Thanks for all the thoughts 😄

bnoordhuis · 2015-11-03T10:25:46Z

I don't disagree exactly but the current emulation mode, limited though it is, at least lets you send a handful of signals.

Grep deps/uv/src/win/process.c for UV_PROCESS_DETACHED when you start working on this. When { detached: true } is passed to child_process.spawn(), the new process is created with the CREATE_NEW_PROCESS_GROUP and DETACHED_PROCESS flags. Some thought should be put into making everything work together without introducing regressions.

kflu · 2015-11-03T15:59:06Z

Awesome. Thanks for the thought!

This commit adds a warning for Windows platforms. `process.kill` wont kill a process group on Windows and instead it throws an error. Refs: nodejs#3617

This commit adds a warning for Windows platforms. `process.kill` wont kill a process group on Windows and instead it throws an error. Refs: #3617 PR-URL: #3681 Reviewed-By: Ben Noordhuis <[email protected]> Reviewed-By: James M Snell <[email protected]>

Trott · 2016-06-05T05:02:31Z

It looks like there was a documentation update about this, so I'm going to close this issue. (Of course, a PR to add process group support on Windows would be welcome.)

If anyone feels strongly that this really ought to remain open, feel free to re-open or comment to that effect.

UV_PROCESS_DETACHED compels libuv:uv__process_child_init() to call setsid() in the child just after fork(). That ensures the process and its descendants are grouped in a separate session (and process group). The following jobstart() call correctly groups `sh` and `sleep` in a new session (and process-group), where `sh` is the "session leader" (and process-group leader): :call jobstart(['sh','-c','sleep 60']) SESN PGRP PID PPID USER Command 30383 30383 30383 3620 vagrant │ ├─ -bash 30383 31432 31432 30383 vagrant │ │ └─ nvim -u NORC 30383 31432 31433 30383 vagrant │ │ ├─ nvim -u NORC 8105 8105 8105 31432 vagrant │ │ └─ sh -c sleep 60 8105 8105 8106 8105 vagrant │ │ └─ sleep 60 closes neovim#6530 ref: https://stackoverflow.com/q/1046933 ref: https://unix.stackexchange.com/a/404065 Helped-by: Marco Hinz <[email protected]> Discussion ------------------------------------------------------------------------ On my linux box before this patch, the termclose_spec.lua:'kills job trapping SIGTERM' test indirectly causes cmake/busted to wait for 60s. That's because the test spawns a `sleep 60` descendant process which hangs around even after nvim exits: nvim killed the parent PID, but not PGID (process-group), so the grandchild "reparented" to init (PID 1). Session contains processes (and process-groups) which are logically part of the same "login session". Process-group is a set of logically/informally-related processes within a session; for example, shells assign a process group to each "job". Session IDs and PGIDs both have type pid_t (like PIDs). These OS-level mechanisms are, as usual, legacy accidents whose purpose is upheld by convention and folklore. We can use session-level grouping (setsid), or we could use process-group-level grouping (setpgid). Vim uses setsid() if available, otherwise setpgid(0,0). Windows ------------------------------------------------------------------------ UV_PROCESS_DETACHED on win32 sets CREATE_NEW_PROCESS_GROUP flag. But uv_kill() does not kill the process-group: nodejs/node#3617 Ideas: - Set UV_PROCESS_WINDOWS_HIDE (CREATE_NEW_PROCESS_GROUP), then call GenerateConsoleCtrlEvent(CTRL_BREAK_EVENT, pid) - Maybe won't work because MSDN says "Only processes that share the same console as the calling process receive the signal." https://docs.microsoft.com/en-us/windows/console/generateconsolectrlevent But CREATE_NEW_PROCESS_GROUP creates a new console ... ref https://stackoverflow.com/q/1453520 - Group processes within a "job". libuv does that *globally* for non-detached processes: uv__init_global_job_handle. - Iterate through CreateToolhelp32Snapshot(). - https://stackoverflow.com/q/1173342 - Vim does this, see terminate_all() TODO ------------------------------------------------------------------------ - test-case from neovim#6530: call jobstop(jobstart('/bin/bash -c ''trap true SIGTERM; sleep 20000000000000''')) - win: iterate through children - implement nvim_get_proc_children(): libuv/libuv#836 - remove useless "watched children" (`loop->children`) from `children_kill_cb` ? - get parent process id: uv_os_getppid() (libuv 1.16+)

UV_PROCESS_DETACHED compels libuv:uv__process_child_init() to call setsid() in the child just after fork(). That ensures the process and its descendants are grouped in a separate session (and process group). The following jobstart() call correctly groups `sh` and `sleep` in a new session (and process-group), where `sh` is the "session leader" (and process-group leader): :call jobstart(['sh','-c','sleep 60']) SESN PGRP PID PPID USER Command 30383 30383 30383 3620 vagrant │ ├─ -bash 30383 31432 31432 30383 vagrant │ │ └─ nvim -u NORC 30383 31432 31433 30383 vagrant │ │ ├─ nvim -u NORC 8105 8105 8105 31432 vagrant │ │ └─ sh -c sleep 60 8105 8105 8106 8105 vagrant │ │ └─ sleep 60 closes neovim#6530 ref: https://stackoverflow.com/q/1046933 ref: https://unix.stackexchange.com/a/404065 Helped-by: Marco Hinz <[email protected]> Discussion ------------------------------------------------------------------------ On my linux box before this patch, the termclose_spec.lua:'kills job trapping SIGTERM' test indirectly causes cmake/busted to wait for 60s. That's because the test spawns a `sleep 60` descendant process which hangs around even after nvim exits: nvim killed the parent PID, but not PGID (process-group), so the grandchild "reparented" to init (PID 1). Session contains processes (and process-groups) which are logically part of the same "login session". Process-group is a set of logically/informally-related processes within a session; for example, shells assign a process group to each "job". Session IDs and PGIDs both have type pid_t (like PIDs). These OS-level mechanisms are, as usual, legacy accidents whose purpose is upheld by convention and folklore. We can use session-level grouping (setsid), or we could use process-group-level grouping (setpgid). Vim uses setsid() if available, otherwise setpgid(0,0). Windows ------------------------------------------------------------------------ UV_PROCESS_DETACHED on win32 sets CREATE_NEW_PROCESS_GROUP flag. But uv_kill() does not kill the process-group: nodejs/node#3617 Ideas: - Set UV_PROCESS_WINDOWS_HIDE (CREATE_NEW_PROCESS_GROUP), then call GenerateConsoleCtrlEvent(CTRL_BREAK_EVENT, pid) - Maybe won't work because MSDN says "Only processes that share the same console as the calling process receive the signal." https://docs.microsoft.com/en-us/windows/console/generateconsolectrlevent But CREATE_NEW_PROCESS_GROUP creates a new console ... ref https://stackoverflow.com/q/1453520 - Group processes within a "job". libuv does that *globally* for non-detached processes: uv__init_global_job_handle. - Iterate through CreateToolhelp32Snapshot(). - https://stackoverflow.com/q/1173342 - Vim does this, see terminate_all()

UV_PROCESS_DETACHED compels libuv:uv__process_child_init() to call setsid() in the child just after fork(). That ensures the process and its descendants are grouped in a separate session (and process group). The following jobstart() call correctly groups `sh` and `sleep` in a new session (and process-group), where `sh` is the "session leader" (and process-group leader): :call jobstart(['sh','-c','sleep 60']) SESN PGRP PID PPID Command 30383 30383 30383 3620 │ ├─ -bash 30383 31432 31432 30383 │ │ └─ nvim -u NORC 30383 31432 31433 30383 │ │ ├─ nvim -u NORC 8105 8105 8105 31432 │ │ └─ sh -c sleep 60 8105 8105 8106 8105 │ │ └─ sleep 60 closes neovim#6530 ref: https://stackoverflow.com/q/1046933 ref: https://unix.stackexchange.com/a/404065 Helped-by: Marco Hinz <[email protected]> Discussion ------------------------------------------------------------------------ On my linux box before this patch, the termclose_spec.lua:'kills job trapping SIGTERM' test indirectly causes cmake/busted to wait for 60s. That's because the test spawns a `sleep 60` descendant process which hangs around even after nvim exits: nvim killed the parent PID, but not PGID (process-group), so the grandchild "reparented" to init (PID 1). Session contains processes (and process-groups) which are logically part of the same "login session". Process-group is a set of logically/informally-related processes within a session; for example, shells assign a process group to each "job". Session IDs and PGIDs both have type pid_t (like PIDs). These OS-level mechanisms are, as usual, legacy accidents whose purpose is upheld by convention and folklore. We can use session-level grouping (setsid), or we could use process-group-level grouping (setpgid). Vim uses setsid() if available, otherwise setpgid(0,0). Windows ------------------------------------------------------------------------ UV_PROCESS_DETACHED on win32 sets CREATE_NEW_PROCESS_GROUP flag. But uv_kill() does not kill the process-group: nodejs/node#3617 Ideas: - Set UV_PROCESS_WINDOWS_HIDE (CREATE_NEW_PROCESS_GROUP), then call GenerateConsoleCtrlEvent(CTRL_BREAK_EVENT, pid) - Maybe won't work because MSDN says "Only processes that share the same console as the calling process receive the signal." https://docs.microsoft.com/en-us/windows/console/generateconsolectrlevent But CREATE_NEW_PROCESS_GROUP creates a new console ... ref https://stackoverflow.com/q/1453520 - Group processes within a "job". libuv does that *globally* for non-detached processes: uv__init_global_job_handle. - Iterate through CreateToolhelp32Snapshot(). - https://stackoverflow.com/q/1173342 - Vim does this, see terminate_all()

UV_PROCESS_DETACHED compels libuv:uv__process_child_init() to call setsid() in the child just after fork(). That ensures the process and its descendants are grouped in a separate session (and process group). The following jobstart() call correctly groups `sh` and `sleep` in a new session (and process-group), where `sh` is the "session leader" (and process-group leader): :call jobstart(['sh','-c','sleep 60']) SESN PGRP PID PPID Command 30383 30383 30383 3620 │ ├─ -bash 30383 31432 31432 30383 │ │ └─ nvim -u NORC 30383 31432 31433 30383 │ │ ├─ nvim -u NORC 8105 8105 8105 31432 │ │ └─ sh -c sleep 60 8105 8105 8106 8105 │ │ └─ sleep 60 closes neovim#6530 ref: https://stackoverflow.com/q/1046933 ref: https://unix.stackexchange.com/a/404065 Helped-by: Marco Hinz <[email protected]> Discussion ------------------------------------------------------------------------ On my linux box before this patch, the termclose_spec.lua:'kills job trapping SIGTERM' test indirectly causes cmake/busted to wait for 60s. That's because the test spawns a `sleep 60` descendant process which hangs around even after nvim exits: nvim killed the parent PID, but not PGID (process-group), so the grandchild "reparented" to init (PID 1). Session contains processes (and process-groups) which are logically part of the same "login session". Process-group is a set of logically/informally-related processes within a session; for example, shells assign a process group to each "job". Session IDs and PGIDs both have type pid_t (like PIDs). These OS-level mechanisms are, as usual, legacy accidents whose purpose is upheld by convention and folklore. We can use session-level grouping (setsid), or we could use process-group-level grouping (setpgid). Vim uses setsid() if available, otherwise setpgid(0,0). Windows ------------------------------------------------------------------------ UV_PROCESS_DETACHED on win32 sets CREATE_NEW_PROCESS_GROUP flag. But uv_kill() does not kill the process-group: nodejs/node#3617 Ideas: - Set UV_PROCESS_DETACHED (CREATE_NEW_PROCESS_GROUP), then call GenerateConsoleCtrlEvent(CTRL_BREAK_EVENT, pid) - Maybe won't work because MSDN says "Only processes that share the same console as the calling process receive the signal." https://docs.microsoft.com/en-us/windows/console/generateconsolectrlevent But CREATE_NEW_PROCESS_GROUP creates a new console ... ref https://stackoverflow.com/q/1453520 - Group processes within a "job". libuv does that *globally* for non-detached processes: uv__init_global_job_handle. - Iterate through CreateToolhelp32Snapshot(). - https://stackoverflow.com/q/1173342 - Vim does this, see terminate_all()

Almenon · 2018-03-31T23:37:43Z

I don't think adding documentation counts as fixing the issue - although documentation is helpful to have. The issue should be reopened IMO.

For those reading this thread trying to figure out how to properly murder your children, (with multi-platform support), Krasimir goes the simple route of using taskkill:

var isWin = /^win/.test(process.platform);
if(!isWin) {
    kill(processing.pid);
} else {
    var cp = require('child_process');
    cp.exec('taskkill /PID ' + processing.pid + ' /T /F', function (error, stdout, stderr) {
        // console.log('stdout: ' + stdout);
        // console.log('stderr: ' + stderr);
        // if(error !== null) {
        //      console.log('exec error: ' + error);
        // }
    });             
}

bnoordhuis · 2018-04-01T16:46:59Z

It was closed because the current behavior is as good as it's going to get without gross hacks.

Tools like taskkill call TerminateProcess() in a loop until there is nothing left to terminate. That doesn't map well to process.kill() except for the process.kill('SIGKILL') case.

Libuv would probably accept a pull request to implement that last case. If you or someone else want to work on that, open an issue for discussion first because there are details to be hashed out.

fengerzh · 2019-11-13T10:46:54Z

taskkill is the keypoint for Windows

turbobuilt · 2024-04-27T16:12:08Z

I think this needs to be added because this functionality is necessary. If the hack is the only way (taskkill), it should be seen as the only way Microsoft allows it to be done, and thus it should be implemented.

The primary goal of a cross platform solution is easy of use across platforms. That means getting it work is more important than making it fun on the backed. Ultimately, if taskkill is the only solution, then we are going to have to do a ton of if-thens on windows, with each developer having to figure out how to do this hack themself.

So ultimately, I think it is a poor decision to avoid doing taskkill because it's just gonna make 100% of us do it ourselves. process.kill needs to work no matter what for the best experience. yes it may be a hack, but if you do the hack, then we don't need to!

Think this through. If you maintain the "hack" that saves thousands of people from having to do it... making you heroes.

kflu mentioned this issue Oct 31, 2015

Fix process not recycled properly on Windows Unitech/pm2#1727

Merged

mscdex added windows Issues and PRs related to the Windows platform. process Issues and PRs related to the process subsystem. labels Nov 1, 2015

ChALkeR added the feature request Issues that request new features to be added to Node.js. label Nov 1, 2015

Fishrock123 added doc Issues and PRs related to the documentations. and removed feature request Issues that request new features to be added to Node.js. labels Nov 2, 2015

Fishrock123 removed the doc Issues and PRs related to the documentations. label Nov 2, 2015

r-52 mentioned this issue Nov 5, 2015

doc: add warning about Windows process groups #3681

Closed

Fishrock123 mentioned this issue Nov 12, 2015

Propose v5.1.0 #3736

Merged

r-52 added a commit to r-52/node that referenced this issue Nov 13, 2015

doc: add warning about Windows process groups

8859f54

This commit adds a warning for Windows platforms. `process.kill` wont kill a process group on Windows and instead it throws an error. Refs: nodejs#3617

Trott closed this as completed Jun 5, 2016

justinmk mentioned this issue Mar 20, 2018

cargo, rust: cargo-check zombie instances neomake/neomake#1216

Closed

Almenon mentioned this issue Mar 31, 2018

birdseye is not fully closed upon exit of birdseye Almenon/birdseye-vscode#11

Closed

DominikGuzei mentioned this issue Nov 7, 2018

[DDW-462] Use "taskkill" to kill processes with pid on Windows input-output-hk/daedalus#1162

Merged

24 tasks

lox mentioned this issue Dec 20, 2018

Use windows console groups for process management buildkite/agent#879

Merged

kernelwhisperer mentioned this issue Mar 2, 2019

Port e2e tests to node aragon/aragon-cli#390

Merged

1 task

boydc2014 mentioned this issue May 3, 2020

fix: bot content not updating after restart microsoft/BotFramework-Composer#2861

Merged

dcdenu4 mentioned this issue Oct 13, 2020

Terminating invest subprocesses | Automated browser tests natcap/invest-workbench#50

Merged

zemccartney mentioned this issue May 4, 2021

Prevent bin process from crashing on sigint when sent during npm init dialog hapipal/hpal#55

Open

edigaryev mentioned this issue Mar 17, 2022

Recent agent changes broke postgres CI on windows cirruslabs/cirrus-ci-agent#218

Closed

niclim mentioned this issue Aug 8, 2023

handle windows kill app opticdev/optic#2206

Merged

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

process.kill can't kill process group on Windows #3617

process.kill can't kill process group on Windows #3617

kflu commented Oct 31, 2015

kflu commented Nov 1, 2015

bnoordhuis commented Nov 1, 2015

Fishrock123 commented Nov 2, 2015

kflu commented Nov 2, 2015

bnoordhuis commented Nov 2, 2015

kflu commented Nov 3, 2015

bnoordhuis commented Nov 3, 2015

kflu commented Nov 3, 2015

Trott commented Jun 5, 2016

Almenon commented Mar 31, 2018

bnoordhuis commented Apr 1, 2018

fengerzh commented Nov 13, 2019

turbobuilt commented Apr 27, 2024

process.kill can't kill process group on Windows #3617

process.kill can't kill process group on Windows #3617

Comments

kflu commented Oct 31, 2015

kflu commented Nov 1, 2015

bnoordhuis commented Nov 1, 2015

Fishrock123 commented Nov 2, 2015

kflu commented Nov 2, 2015

bnoordhuis commented Nov 2, 2015

kflu commented Nov 3, 2015

bnoordhuis commented Nov 3, 2015

kflu commented Nov 3, 2015

Trott commented Jun 5, 2016

Almenon commented Mar 31, 2018

bnoordhuis commented Apr 1, 2018

fengerzh commented Nov 13, 2019

turbobuilt commented Apr 27, 2024