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: nodejs#33715
  • Loading branch information
Bar Admoni committed Jul 11, 2020
1 parent c23d2fd commit e6878a6
Show file tree
Hide file tree
Showing 4 changed files with 103 additions and 16 deletions.
2 changes: 2 additions & 0 deletions README.md
Original file line number Diff line number Diff line change
Expand Up @@ -437,6 +437,8 @@ For information about the governance of the Node.js project, see
**Yosuke Furukawa** <[email protected]>
* [ZYSzys](https://github.com/ZYSzys) -
**Yongsheng Zhang** <[email protected]> (he/him)
* [baradm100](https://github.com/baradm100) -
**Bar Admoni** <[email protected]> (he/him)

### Collaborator Emeriti

Expand Down
10 changes: 3 additions & 7 deletions doc/api/cluster.md
Original file line number Diff line number Diff line change
Expand Up @@ -427,14 +427,10 @@ added: v0.9.12

This function will kill the worker. In the master, 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`.
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()`.

Causes `.exitedAfterDisconnect` to be set.
The `kill()` function kills the worker process without waiting for a graceful
disconnect, it have the same behavior of `worker.process.kill()`.

This method is aliased as `worker.destroy()` for backwards compatibility.

Expand Down
11 changes: 2 additions & 9 deletions lib/internal/cluster/master.js
Original file line number Diff line number Diff line change
Expand Up @@ -360,14 +360,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);
};
96 changes: 96 additions & 0 deletions test/parallel/test-cluster-worker-kill-signal.js
Original file line number Diff line number Diff line change
@@ -0,0 +1,96 @@
// Copyright Joyent, Inc. and other Node contributors.
//
// Permission is hereby granted, free of charge, to any person obtaining a
// copy of this software and associated documentation files (the
// "Software"), to deal in the Software without restriction, including
// without limitation the rights to use, copy, modify, merge, publish,
// distribute, sublicense, and/or sell copies of the Software, and to permit
// persons to whom the Software is furnished to do so, subject to the
// following conditions:
//
// The above copyright notice and this permission notice shall be included
// in all copies or substantial portions of the Software.
//
// THE SOFTWARE IS PROVIDED "AS IS", WITHOUT WARRANTY OF ANY KIND, EXPRESS
// OR IMPLIED, INCLUDING BUT NOT LIMITED TO THE WARRANTIES OF
// MERCHANTABILITY, FITNESS FOR A PARTICULAR PURPOSE AND NONINFRINGEMENT. IN
// NO EVENT SHALL THE AUTHORS OR COPYRIGHT HOLDERS BE LIABLE FOR ANY CLAIM,
// DAMAGES OR OTHER LIABILITY, WHETHER IN AN ACTION OF CONTRACT, TORT OR
// OTHERWISE, ARISING FROM, OUT OF OR IN CONNECTION WITH THE SOFTWARE OR THE
// USE OR OTHER DEALINGS IN THE SOFTWARE.

'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) {
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';
const expected_results = {
worker_emitDisconnect: [1, "the worker did not emit 'disconnect'"],
worker_emitExit: [1, "the worker did not emit 'exit'"],
worker_state: ['disconnected', 'the worker state is incorrect'],
worker_exitedAfter: [false, 'the .exitedAfterDisconnect flag is incorrect'],
worker_died: [true, 'the worker is still running'],
worker_exitCode: [null, 'the worker exited w/ incorrect exitCode'],
worker_signalCode: [KILL_SIGNAL,
'the worker exited w/ incorrect signalCode']
};
const results = {
worker_emitDisconnect: 0,
worker_emitExit: 0
};


// 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(() => {
results.worker_emitDisconnect += 1;
results.worker_exitedAfter = worker.exitedAfterDisconnect;
results.worker_state = worker.state;
}));

// Check that the worker died
worker.once('exit', common.mustCall((exitCode, signalCode) => {
results.worker_exitCode = exitCode;
results.worker_signalCode = signalCode;
results.worker_emitExit += 1;
results.worker_died = !common.isAlive(worker.process.pid);
}));

process.on('exit', () => {
checkResults(expected_results, results);
});
}

// Some helper functions ...

function checkResults(expected_results, results) {
for (const k in expected_results) {
const actual = results[k];
const expected = expected_results[k];

assert.strictEqual(
actual, expected && expected.length ? expected[0] : expected,
`${expected[1] || ''} [expected: ${expected[0]} / actual: ${actual}]`);
}
}

0 comments on commit e6878a6

Please sign in to comment.