Skip to content

Commit 38007df

Browse files
authored
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 <matteo.collina@gmail.com> Reviewed-By: Robert Nagy <ronagy@icloud.com> Reviewed-By: Antoine du Hamel <duhamelantoine1995@gmail.com>
1 parent 6c0eb94 commit 38007df

File tree

3 files changed

+59
-22
lines changed

3 files changed

+59
-22
lines changed

doc/api/cluster.md

+8-13
Original file line numberDiff line numberDiff line change
@@ -452,8 +452,8 @@ added: v6.0.0
452452

453453
* {boolean}
454454

455-
This property is `true` if the worker exited due to `.kill()` or
456-
`.disconnect()`. If the worker exited any other way, it is `false`. If the
455+
This property is `true` if the worker exited due to `.disconnect()`.
456+
If the worker exited any other way, it is `false`. If the
457457
worker has not exited, it is `undefined`.
458458

459459
The boolean [`worker.exitedAfterDisconnect`][] allows distinguishing between
@@ -577,19 +577,14 @@ added: v0.9.12
577577
* `signal` {string} Name of the kill signal to send to the worker
578578
process. **Default:** `'SIGTERM'`
579579

580-
This function will kill the worker. In the primary, it does this
581-
by disconnecting the `worker.process`, and once disconnected, killing
582-
with `signal`. In the worker, it does it by disconnecting the channel,
583-
and then exiting with code `0`.
580+
This function will kill the worker. In the primary worker, it does this by
581+
disconnecting the `worker.process`, and once disconnected, killing with
582+
`signal`. In the worker, it does it by killing the process with `signal`.
584583

585-
Because `kill()` attempts to gracefully disconnect the worker process, it is
586-
susceptible to waiting indefinitely for the disconnect to complete. For example,
587-
if the worker enters an infinite loop, a graceful disconnect will never occur.
588-
If the graceful disconnect behavior is not needed, use `worker.process.kill()`.
584+
The `kill()` function kills the worker process without waiting for a graceful
585+
disconnect, it has the same behavior as `worker.process.kill()`.
589586

590-
Causes `.exitedAfterDisconnect` to be set.
591-
592-
This method is aliased as `worker.destroy()` for backward compatibility.
587+
This method is aliased as `worker.destroy()` for backwards compatibility.
593588

594589
In a worker, `process.kill()` exists, but it is not this function;
595590
it is [`kill()`][].

lib/internal/cluster/primary.js

+2-9
Original file line numberDiff line numberDiff line change
@@ -374,14 +374,7 @@ Worker.prototype.disconnect = function() {
374374

375375
Worker.prototype.destroy = function(signo) {
376376
const proc = this.process;
377+
const signal = signo || 'SIGTERM';
377378

378-
signo = signo || 'SIGTERM';
379-
380-
if (this.isConnected()) {
381-
this.once('disconnect', () => proc.kill(signo));
382-
this.disconnect();
383-
return;
384-
}
385-
386-
proc.kill(signo);
379+
proc.kill(signal);
387380
};
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,49 @@
1+
'use strict';
2+
// test-cluster-worker-kill-signal.js
3+
// verifies that when we're killing a worker using Worker.prototype.kill
4+
// and the worker's process was killed with the given signal (SIGKILL)
5+
6+
7+
const common = require('../common');
8+
const assert = require('assert');
9+
const cluster = require('cluster');
10+
11+
if (cluster.isWorker) {
12+
// Make the worker run something
13+
const http = require('http');
14+
const server = http.Server(() => { });
15+
16+
server.once('listening', common.mustCall(() => { }));
17+
server.listen(0, '127.0.0.1');
18+
19+
} else if (cluster.isMaster) {
20+
const KILL_SIGNAL = 'SIGKILL';
21+
22+
// Start worker
23+
const worker = cluster.fork();
24+
25+
// When the worker is up and running, kill it
26+
worker.once('listening', common.mustCall(() => {
27+
worker.kill(KILL_SIGNAL);
28+
}));
29+
30+
// Check worker events and properties
31+
worker.on('disconnect', common.mustCall(() => {
32+
assert.strictEqual(worker.exitedAfterDisconnect, false);
33+
assert.strictEqual(worker.state, 'disconnected');
34+
}, 1));
35+
36+
// Check that the worker died
37+
worker.once('exit', common.mustCall((exitCode, signalCode) => {
38+
const isWorkerProcessStillAlive = common.isAlive(worker.process.pid);
39+
const numOfRunningWorkers = Object.keys(cluster.workers).length;
40+
41+
assert.strictEqual(exitCode, null);
42+
assert.strictEqual(signalCode, KILL_SIGNAL);
43+
assert.strictEqual(isWorkerProcessStillAlive, false);
44+
assert.strictEqual(numOfRunningWorkers, 0);
45+
}, 1));
46+
47+
// Check if the cluster was killed as well
48+
cluster.on('exit', common.mustCall(() => {}, 1));
49+
}

0 commit comments

Comments
 (0)