Skip to content

Commit

Permalink
cluster: make kill to be just process.kill
Browse files Browse the repository at this point in the history
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>
  • Loading branch information
baradm100 authored Feb 4, 2022
1 parent 6c0eb94 commit 38007df
Show file tree
Hide file tree
Showing 3 changed files with 59 additions and 22 deletions.
21 changes: 8 additions & 13 deletions doc/api/cluster.md
Original file line number Diff line number Diff line change
Expand Up @@ -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
Expand Down Expand Up @@ -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()`][].
Expand Down
11 changes: 2 additions & 9 deletions lib/internal/cluster/primary.js
Original file line number Diff line number Diff line change
Expand Up @@ -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);
};
49 changes: 49 additions & 0 deletions test/parallel/test-cluster-worker-kill-signal.js
Original file line number Diff line number Diff line change
@@ -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));
}

0 comments on commit 38007df

Please sign in to comment.