From 3eaa41f528b133d1fd9b4fc936605b8095532c6d Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?H=C3=BCseyin=20A=C3=A7acak?= Date: Thu, 24 Oct 2024 14:08:50 +0300 Subject: [PATCH 1/4] src: fix kill signal on Windows Fixes: https://github.com/nodejs/node/issues/42923 --- doc/api/child_process.md | 4 +-- src/process_wrap.cc | 6 ++++ test/parallel/test-child-process-kill.js | 38 ++++++++++++++++++++++++ 3 files changed, 46 insertions(+), 2 deletions(-) diff --git a/doc/api/child_process.md b/doc/api/child_process.md index f70d96a3395300..aafe7f369dcbbc 100644 --- a/doc/api/child_process.md +++ b/doc/api/child_process.md @@ -1698,8 +1698,8 @@ may not actually terminate the process. See kill(2) for reference. On Windows, where POSIX signals do not exist, the `signal` argument will be -ignored, and the process will be killed forcefully and abruptly (similar to -`'SIGKILL'`). +ignored except `'SIGKILL'`, `'SIGTERM'`, `'SIGINT'` and `'SIGQUIT'`, and the +process will be killed forcefully and abruptly (similar to `'SIGKILL'`). See [Signal Events][] for more details. On Linux, child processes of child processes will not be terminated diff --git a/src/process_wrap.cc b/src/process_wrap.cc index 556dea18eca76f..74f46bcb651bef 100644 --- a/src/process_wrap.cc +++ b/src/process_wrap.cc @@ -312,6 +312,12 @@ class ProcessWrap : public HandleWrap { ProcessWrap* wrap; ASSIGN_OR_RETURN_UNWRAP(&wrap, args.This()); int signal = args[0]->Int32Value(env->context()).FromJust(); +#ifdef _WIN32 + if (signal != SIGKILL && signal != SIGTERM && signal != SIGINT && + signal != SIGQUIT) { + signal = SIGKILL; + } +#endif int err = uv_process_kill(&wrap->process_, signal); args.GetReturnValue().Set(err); } diff --git a/test/parallel/test-child-process-kill.js b/test/parallel/test-child-process-kill.js index 1025c69ba1ac28..d5563e70e0976d 100644 --- a/test/parallel/test-child-process-kill.js +++ b/test/parallel/test-child-process-kill.js @@ -39,3 +39,41 @@ assert.strictEqual(cat.signalCode, null); assert.strictEqual(cat.killed, false); cat.kill(); assert.strictEqual(cat.killed, true); + +/* Test different types of kill signals on Windows */ +if (common.isWindows) { + const process1 = spawn('cmd'); + process1.on('exit', (code, signal) => { + assert.strictEqual(code, null); + assert.strictEqual(signal, 'SIGTERM'); + }); + process1.kill('SIGTERM'); + + const process2 = spawn('cmd'); + process2.on('exit', (code, signal) => { + assert.strictEqual(code, null); + assert.strictEqual(signal, 'SIGKILL'); + }); + process2.kill('SIGKILL'); + + const process3 = spawn('cmd'); + process3.on('exit', (code, signal) => { + assert.strictEqual(code, null); + assert.strictEqual(signal, 'SIGQUIT'); + }); + process3.kill('SIGQUIT'); + + const process4 = spawn('cmd'); + process4.on('exit', (code, signal) => { + assert.strictEqual(code, null); + assert.strictEqual(signal, 'SIGINT'); + }); + process4.kill('SIGINT'); + + const process5 = spawn('cmd'); + process5.on('exit', (code, signal) => { + assert.strictEqual(code, null); + assert.strictEqual(signal, 'SIGKILL'); + }); + process5.kill('SIGHUP'); +} From 86229ce35de28db43a97e3ca8c6e893d5d1500fb Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?H=C3=BCseyin=20A=C3=A7acak?= <110401522+huseyinacacak-janea@users.noreply.github.com> Date: Fri, 25 Oct 2024 12:32:31 +0300 Subject: [PATCH 2/4] Update doc/api/child_process.md Co-authored-by: Luigi Pinca --- doc/api/child_process.md | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/doc/api/child_process.md b/doc/api/child_process.md index aafe7f369dcbbc..31416568f0cd20 100644 --- a/doc/api/child_process.md +++ b/doc/api/child_process.md @@ -1698,8 +1698,8 @@ may not actually terminate the process. See kill(2) for reference. On Windows, where POSIX signals do not exist, the `signal` argument will be -ignored except `'SIGKILL'`, `'SIGTERM'`, `'SIGINT'` and `'SIGQUIT'`, and the -process will be killed forcefully and abruptly (similar to `'SIGKILL'`). +ignored except for `'SIGKILL'`, `'SIGTERM'`, `'SIGINT'` and `'SIGQUIT'`, and the +process will always be killed forcefully and abruptly (similar to `'SIGKILL'`). See [Signal Events][] for more details. On Linux, child processes of child processes will not be terminated From 5937e1f9f20c1ee17cea427037db18dc4353af6c Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?H=C3=BCseyin=20A=C3=A7acak?= <110401522+huseyinacacak-janea@users.noreply.github.com> Date: Fri, 25 Oct 2024 12:32:37 +0300 Subject: [PATCH 3/4] Update test/parallel/test-child-process-kill.js Co-authored-by: Luigi Pinca --- test/parallel/test-child-process-kill.js | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/test/parallel/test-child-process-kill.js b/test/parallel/test-child-process-kill.js index d5563e70e0976d..c27a75598f91bf 100644 --- a/test/parallel/test-child-process-kill.js +++ b/test/parallel/test-child-process-kill.js @@ -40,7 +40,7 @@ assert.strictEqual(cat.killed, false); cat.kill(); assert.strictEqual(cat.killed, true); -/* Test different types of kill signals on Windows */ +// Test different types of kill signals on Windows. if (common.isWindows) { const process1 = spawn('cmd'); process1.on('exit', (code, signal) => { From fbbb15eacbb6d838b3cde8134ee1b7c8365e939b Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?H=C3=BCseyin=20A=C3=A7acak?= Date: Wed, 6 Nov 2024 13:03:36 +0300 Subject: [PATCH 4/4] fixup! src: fix kill signal on Windows --- test/parallel/test-child-process-kill.js | 43 +++++++----------------- 1 file changed, 12 insertions(+), 31 deletions(-) diff --git a/test/parallel/test-child-process-kill.js b/test/parallel/test-child-process-kill.js index c27a75598f91bf..26bdc029c04fe8 100644 --- a/test/parallel/test-child-process-kill.js +++ b/test/parallel/test-child-process-kill.js @@ -42,38 +42,19 @@ assert.strictEqual(cat.killed, true); // Test different types of kill signals on Windows. if (common.isWindows) { - const process1 = spawn('cmd'); - process1.on('exit', (code, signal) => { - assert.strictEqual(code, null); - assert.strictEqual(signal, 'SIGTERM'); - }); - process1.kill('SIGTERM'); - - const process2 = spawn('cmd'); - process2.on('exit', (code, signal) => { - assert.strictEqual(code, null); - assert.strictEqual(signal, 'SIGKILL'); - }); - process2.kill('SIGKILL'); - - const process3 = spawn('cmd'); - process3.on('exit', (code, signal) => { - assert.strictEqual(code, null); - assert.strictEqual(signal, 'SIGQUIT'); - }); - process3.kill('SIGQUIT'); - - const process4 = spawn('cmd'); - process4.on('exit', (code, signal) => { - assert.strictEqual(code, null); - assert.strictEqual(signal, 'SIGINT'); - }); - process4.kill('SIGINT'); - - const process5 = spawn('cmd'); - process5.on('exit', (code, signal) => { + for (const sendSignal of ['SIGTERM', 'SIGKILL', 'SIGQUIT', 'SIGINT']) { + const process = spawn('cmd'); + process.on('exit', (code, signal) => { + assert.strictEqual(code, null); + assert.strictEqual(signal, sendSignal); + }); + process.kill(sendSignal); + } + + const process = spawn('cmd'); + process.on('exit', (code, signal) => { assert.strictEqual(code, null); assert.strictEqual(signal, 'SIGKILL'); }); - process5.kill('SIGHUP'); + process.kill('SIGHUP'); }