From 38007df999430798908f442ea187cd83978515e5 Mon Sep 17 00:00:00 2001 From: Bar Admoni Date: Sat, 5 Feb 2022 01:15:24 +0200 Subject: [PATCH] cluster: make `kill` to be just `process.kill` Make `Worker.prototype.kill` to be just `process.kill` without preforming graceful disconnect beforehand. Refs: https://github.com/nodejs/node/issues/33715 PR-URL: https://github.com/nodejs/node/pull/34312 Reviewed-By: Matteo Collina Reviewed-By: Robert Nagy Reviewed-By: Antoine du Hamel --- doc/api/cluster.md | 21 +++----- lib/internal/cluster/primary.js | 11 +---- .../test-cluster-worker-kill-signal.js | 49 +++++++++++++++++++ 3 files changed, 59 insertions(+), 22 deletions(-) create mode 100644 test/parallel/test-cluster-worker-kill-signal.js diff --git a/doc/api/cluster.md b/doc/api/cluster.md index ac199c34a3755a..0a5af1bcf27cda 100644 --- a/doc/api/cluster.md +++ b/doc/api/cluster.md @@ -452,8 +452,8 @@ added: v6.0.0 * {boolean} -This property is `true` if the worker exited due to `.kill()` or -`.disconnect()`. If the worker exited any other way, it is `false`. If the +This property is `true` if the worker exited due to `.disconnect()`. +If the worker exited any other way, it is `false`. If the worker has not exited, it is `undefined`. The boolean [`worker.exitedAfterDisconnect`][] allows distinguishing between @@ -577,19 +577,14 @@ added: v0.9.12 * `signal` {string} Name of the kill signal to send to the worker process. **Default:** `'SIGTERM'` -This function will kill the worker. In the primary, it does this -by disconnecting the `worker.process`, and once disconnected, killing -with `signal`. In the worker, it does it by disconnecting the channel, -and then exiting with code `0`. +This function will kill the worker. In the primary worker, it does this by +disconnecting the `worker.process`, and once disconnected, killing with +`signal`. In the worker, it does it by killing the process with `signal`. -Because `kill()` attempts to gracefully disconnect the worker process, it is -susceptible to waiting indefinitely for the disconnect to complete. For example, -if the worker enters an infinite loop, a graceful disconnect will never occur. -If the graceful disconnect behavior is not needed, use `worker.process.kill()`. +The `kill()` function kills the worker process without waiting for a graceful +disconnect, it has the same behavior as `worker.process.kill()`. -Causes `.exitedAfterDisconnect` to be set. - -This method is aliased as `worker.destroy()` for backward compatibility. +This method is aliased as `worker.destroy()` for backwards compatibility. In a worker, `process.kill()` exists, but it is not this function; it is [`kill()`][]. diff --git a/lib/internal/cluster/primary.js b/lib/internal/cluster/primary.js index 69e97eb8a6837f..ed5b06d798868c 100644 --- a/lib/internal/cluster/primary.js +++ b/lib/internal/cluster/primary.js @@ -374,14 +374,7 @@ Worker.prototype.disconnect = function() { Worker.prototype.destroy = function(signo) { const proc = this.process; + const signal = signo || 'SIGTERM'; - signo = signo || 'SIGTERM'; - - if (this.isConnected()) { - this.once('disconnect', () => proc.kill(signo)); - this.disconnect(); - return; - } - - proc.kill(signo); + proc.kill(signal); }; diff --git a/test/parallel/test-cluster-worker-kill-signal.js b/test/parallel/test-cluster-worker-kill-signal.js new file mode 100644 index 00000000000000..2f1a8ae991dde4 --- /dev/null +++ b/test/parallel/test-cluster-worker-kill-signal.js @@ -0,0 +1,49 @@ +'use strict'; +// test-cluster-worker-kill-signal.js +// verifies that when we're killing a worker using Worker.prototype.kill +// and the worker's process was killed with the given signal (SIGKILL) + + +const common = require('../common'); +const assert = require('assert'); +const cluster = require('cluster'); + +if (cluster.isWorker) { + // Make the worker run something + const http = require('http'); + const server = http.Server(() => { }); + + server.once('listening', common.mustCall(() => { })); + server.listen(0, '127.0.0.1'); + +} else if (cluster.isMaster) { + const KILL_SIGNAL = 'SIGKILL'; + + // Start worker + const worker = cluster.fork(); + + // When the worker is up and running, kill it + worker.once('listening', common.mustCall(() => { + worker.kill(KILL_SIGNAL); + })); + + // Check worker events and properties + worker.on('disconnect', common.mustCall(() => { + assert.strictEqual(worker.exitedAfterDisconnect, false); + assert.strictEqual(worker.state, 'disconnected'); + }, 1)); + + // Check that the worker died + worker.once('exit', common.mustCall((exitCode, signalCode) => { + const isWorkerProcessStillAlive = common.isAlive(worker.process.pid); + const numOfRunningWorkers = Object.keys(cluster.workers).length; + + assert.strictEqual(exitCode, null); + assert.strictEqual(signalCode, KILL_SIGNAL); + assert.strictEqual(isWorkerProcessStillAlive, false); + assert.strictEqual(numOfRunningWorkers, 0); + }, 1)); + + // Check if the cluster was killed as well + cluster.on('exit', common.mustCall(() => {}, 1)); +}