process.kill can't kill process group on Windows

[kflu]

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)

[kflu]

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]

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]

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

[kflu]

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

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]

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]

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]

Awesome. Thanks for the thought!