cluster: make kill to be just process.kill

Make `Worker.prototype.kill` to be just `process.kill` without
preforming graceful disconnect beforehand.

Refs: #33715
PR-URL: #34312
Reviewed-By: Matteo Collina <[email protected]>
Reviewed-By: Robert Nagy <[email protected]>
Reviewed-By: Antoine du Hamel <[email protected]>

Author: baradm100

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()`][].

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);
 };

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));
+}