Skip to content

Commit

Permalink
cluster: migrate from worker.suicide
Browse files Browse the repository at this point in the history 
Replace it with worker.exitedAfterDisconnect. Print deprecation
message when getting or setting until it is removed.

PR-URL: #3743
Fixes: #3721
Reviewed-By: James M Snell <jasnell@gmail.com>
Reviewed-By: Π‘ΠΊΠΎΠ²ΠΎΡ€ΠΎΠ΄Π° Никита АндрССвич <chalkerx@gmail.com>
Reviewed-By: Anna Henningsen <anna@addaleax.net>
Reviewed-By: Myles Borins <myles.borins@gmail.com>
Reviewed-By: Stephen Belanger <admin@stephenbelanger.com>
  • Loading branch information
@evanlucas
evanlucas committed Apr 26, 2016
1 parent 9bb5a5e commit 4f619bd
Show file tree
Hide file tree
Showing 6 changed files with 84 additions and 34 deletions.
46 changes: 37 additions & 9 deletions doc/api/cluster.md
View file
Expand Up @@ -239,7 +239,7 @@ those servers, and then disconnect the IPC channel.
In the master, an internal message is sent to the worker causing it to call In the master, an internal message is sent to the worker causing it to call
`.disconnect()` on itself. `.disconnect()` on itself.


Causes `.suicide` to be set. Causes `.exitedAfterDisconnect` to be set.


Note that after a server is closed, it will no longer accept new connections, Note that after a server is closed, it will no longer accept new connections,
but connections may be accepted by any other listening worker. Existing but connections may be accepted by any other listening worker. Existing
Expand Down Expand Up @@ -292,6 +292,27 @@ if (cluster.isMaster) {
} }
``` ```


### worker.exitedAfterDisconnect

* {Boolean}

Set by calling `.kill()` or `.disconnect()`. Until then, it is `undefined`.

The boolean `worker.exitedAfterDisconnect` lets you distinguish between voluntary
and accidental exit, the master may choose not to respawn a worker based on
this value.

```js
cluster.on('exit', (worker, code, signal) => {
if (worker.exitedAfterDisconnect === true) {
console.log('Oh, it was just voluntary\' – no need to worry').
}
});

// kill worker
worker.kill();
```

### worker.id ### worker.id


* {Number} * {Number}
Expand Down Expand Up @@ -322,7 +343,7 @@ 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 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 disconnecting the channel, and then exiting with code `0`.


Causes `.suicide` to be set. Causes `.exitedAfterDisconnect` to be set.


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


Expand All @@ -340,8 +361,8 @@ is stored.
See: [Child Process module][] See: [Child Process module][]


Note that workers will call `process.exit(0)` if the `'disconnect'` event occurs Note that workers will call `process.exit(0)` if the `'disconnect'` event occurs
on `process` and `.suicide` is not `true`. This protects against accidental on `process` and `.exitedAfterDisconnect` is not `true`. This protects against
disconnection. accidental disconnection.


### worker.send(message[, sendHandle][, callback]) ### worker.send(message[, sendHandle][, callback])


Expand Down Expand Up @@ -374,24 +395,30 @@ if (cluster.isMaster) {


### worker.suicide ### worker.suicide


* {Boolean} Stability: 0 - Deprecated: Use [`worker.exitedAfterDisconnect`][] instead.


Set by calling `.kill()` or `.disconnect()`, until then it is `undefined`. An alias to [`worker.exitedAfterDisconnect`][].


The boolean `worker.suicide` lets you distinguish between voluntary and accidental Set by calling `.kill()` or `.disconnect()`. Until then, it is `undefined`.
exit, the master may choose not to respawn a worker based on this value.
The boolean `worker.suicide` lets you distinguish between voluntary
and accidental exit, the master may choose not to respawn a worker based on
this value.


```js ```js
cluster.on('exit', (worker, code, signal) => { cluster.on('exit', (worker, code, signal) => {
if (worker.suicide === true) { if (worker.suicide === true) {
console.log('Oh, it was just suicide\' – no need to worry'). console.log('Oh, it was just voluntary\' – no need to worry').
} }
}); });


// kill worker // kill worker
worker.kill(); worker.kill();
``` ```


This API only exists for backwards compatibility and will be removed in the
future.

## Event: 'disconnect' ## Event: 'disconnect'


* `worker` {cluster.Worker} * `worker` {cluster.Worker}
Expand Down Expand Up @@ -707,6 +734,7 @@ socket.on('data', (id) => {
[`disconnect`]: child_process.html#child_process_child_disconnect [`disconnect`]: child_process.html#child_process_child_disconnect
[`kill`]: process.html#process_process_kill_pid_signal [`kill`]: process.html#process_process_kill_pid_signal
[`server.close()`]: net.html#net_event_close [`server.close()`]: net.html#net_event_close
[`worker.exitedAfterDisconnect`]: #cluster_worker_exitedafterdisconnect
[Child Process module]: child_process.html#child_process_child_process_fork_modulepath_args_options [Child Process module]: child_process.html#child_process_child_process_fork_modulepath_args_options
[child_process event: 'exit']: child_process.html#child_process_event_exit [child_process event: 'exit']: child_process.html#child_process_event_exit
[child_process event: 'message']: child_process.html#child_process_event_message [child_process event: 'message']: child_process.html#child_process_event_message
48 changes: 31 additions & 17 deletions lib/cluster.js
View file
Expand Up @@ -27,7 +27,20 @@ function Worker(options) {
if (options === null || typeof options !== 'object') if (options === null || typeof options !== 'object')
options = {}; options = {};


this.suicide = undefined; this.exitedAfterDisconnect = undefined;

Object.defineProperty(this, 'suicide', {
get: function() {
// TODO: Print deprecation message.
return this.exitedAfterDisconnect;
},
set: function(val) {
// TODO: Print deprecation message.
this.exitedAfterDisconnect = val;
},
enumerable: true
});

this.state = options.state || 'none'; this.state = options.state || 'none';
this.id = options.id | 0; this.id = options.id | 0;


Expand Down Expand Up @@ -355,7 +368,7 @@ function masterInit() {
removeWorker(worker); removeWorker(worker);
} }


worker.suicide = !!worker.suicide; worker.exitedAfterDisconnect = !!worker.exitedAfterDisconnect;
worker.state = 'dead'; worker.state = 'dead';
worker.emit('exit', exitCode, signalCode); worker.emit('exit', exitCode, signalCode);
cluster.emit('exit', worker, exitCode, signalCode); cluster.emit('exit', worker, exitCode, signalCode);
Expand All @@ -376,7 +389,7 @@ function masterInit() {
*/ */
if (worker.isDead()) removeWorker(worker); if (worker.isDead()) removeWorker(worker);


worker.suicide = !!worker.suicide; worker.exitedAfterDisconnect = !!worker.exitedAfterDisconnect;
worker.state = 'disconnected'; worker.state = 'disconnected';
worker.emit('disconnect'); worker.emit('disconnect');
cluster.emit('disconnect', worker); cluster.emit('disconnect', worker);
Expand Down Expand Up @@ -407,7 +420,7 @@ function masterInit() {
}; };


Worker.prototype.disconnect = function() { Worker.prototype.disconnect = function() {
this.suicide = true; this.exitedAfterDisconnect = true;
send(this, { act: 'disconnect' }); send(this, { act: 'disconnect' });
removeHandlesForWorker(this); removeHandlesForWorker(this);
removeWorker(this); removeWorker(this);
Expand All @@ -432,8 +445,8 @@ function masterInit() {
queryServer(worker, message); queryServer(worker, message);
else if (message.act === 'listening') else if (message.act === 'listening')
listening(worker, message); listening(worker, message);
else if (message.act === 'suicide') else if (message.act === 'exitedAfterDisconnect')
suicide(worker, message); exitedAfterDisconnect(worker, message);
else if (message.act === 'close') else if (message.act === 'close')
close(worker, message); close(worker, message);
} }
Expand All @@ -444,14 +457,14 @@ function masterInit() {
cluster.emit('online', worker); cluster.emit('online', worker);
} }


function suicide(worker, message) { function exitedAfterDisconnect(worker, message) {
worker.suicide = true; worker.exitedAfterDisconnect = true;
send(worker, { ack: message.seq }); send(worker, { ack: message.seq });
} }


function queryServer(worker, message) { function queryServer(worker, message) {
// Stop processing if worker already disconnecting // Stop processing if worker already disconnecting
if (worker.suicide) if (worker.exitedAfterDisconnect)
return; return;
var args = [message.address, var args = [message.address,
message.port, message.port,
Expand Down Expand Up @@ -533,7 +546,7 @@ function workerInit() {
cluster.worker = worker; cluster.worker = worker;
process.once('disconnect', function() { process.once('disconnect', function() {
worker.emit('disconnect'); worker.emit('disconnect');
if (!worker.suicide) { if (!worker.exitedAfterDisconnect) {
// Unexpected disconnect, master exited, or some such nastiness, so // Unexpected disconnect, master exited, or some such nastiness, so
// worker exits immediately. // worker exits immediately.
process.exit(0); process.exit(0);
Expand Down Expand Up @@ -670,10 +683,10 @@ function workerInit() {
}; };


Worker.prototype.destroy = function() { Worker.prototype.destroy = function() {
this.suicide = true; this.exitedAfterDisconnect = true;
if (!this.isConnected()) process.exit(0); if (!this.isConnected()) process.exit(0);
var exit = process.exit.bind(null, 0); var exit = process.exit.bind(null, 0);
send({ act: 'suicide' }, () => process.disconnect()); send({ act: 'exitedAfterDisconnect' }, () => process.disconnect());
process.once('disconnect', exit); process.once('disconnect', exit);
}; };


Expand All @@ -682,19 +695,20 @@ function workerInit() {
} }


function _disconnect(masterInitiated) { function _disconnect(masterInitiated) {
this.suicide = true; this.exitedAfterDisconnect = true;
let waitingCount = 1; let waitingCount = 1;


function checkWaitingCount() { function checkWaitingCount() {
waitingCount--; waitingCount--;
if (waitingCount === 0) { if (waitingCount === 0) {
// If disconnect is worker initiated, wait for ack to be sure suicide // If disconnect is worker initiated, wait for ack to be sure
// is properly set in the master, otherwise, if it's master initiated // exitedAfterDisconnect is properly set in the master, otherwise, if
// there's no need to send the suicide message // it's master initiated there's no need to send the
// exitedAfterDisconnect message
if (masterInitiated) { if (masterInitiated) {
process.disconnect(); process.disconnect();
} else { } else {
send({ act: 'suicide' }, () => process.disconnect()); send({ act: 'exitedAfterDisconnect' }, () => process.disconnect());
} }
} }
} }
Expand Down
10 changes: 7 additions & 3 deletions test/parallel/test-cluster-worker-disconnect.js
View file
Expand Up @@ -8,8 +8,12 @@ if (cluster.isWorker) {
http.Server(function() { http.Server(function() {


}).listen(common.PORT, '127.0.0.1'); }).listen(common.PORT, '127.0.0.1');
const worker = cluster.worker;
assert.strictEqual(worker.exitedAfterDisconnect, worker.suicide);


cluster.worker.on('disconnect', function() { cluster.worker.on('disconnect', function() {
assert.strictEqual(cluster.worker.exitedAfterDisconnect,
cluster.worker.suicide);
process.exit(42); process.exit(42);
}); });


Expand All @@ -26,7 +30,7 @@ if (cluster.isWorker) {
emitDisconnectInsideWorker: false, emitDisconnectInsideWorker: false,
emitExit: false, emitExit: false,
state: false, state: false,
suicideMode: false, voluntaryMode: false,
died: false died: false
} }
}; };
Expand Down Expand Up @@ -60,7 +64,7 @@ if (cluster.isWorker) {
// Check worker events and properties // Check worker events and properties
worker.once('disconnect', function() { worker.once('disconnect', function() {
checks.worker.emitDisconnect = true; checks.worker.emitDisconnect = true;
checks.worker.suicideMode = worker.suicide; checks.worker.voluntaryMode = worker.exitedAfterDisconnect;
checks.worker.state = worker.state; checks.worker.state = worker.state;
}); });


Expand All @@ -86,7 +90,7 @@ if (cluster.isWorker) {


// flags // flags
assert.equal(w.state, 'disconnected', 'The state property was not set'); assert.equal(w.state, 'disconnected', 'The state property was not set');
assert.equal(w.suicideMode, true, 'Suicide mode was not set'); assert.equal(w.voluntaryMode, true, 'Voluntary exit mode was not set');


// is process alive // is process alive
assert.ok(w.died, 'The worker did not die'); assert.ok(w.died, 'The worker did not die');
Expand Down
5 changes: 4 additions & 1 deletion test/parallel/test-cluster-worker-exit.js
View file
Expand Up @@ -3,7 +3,7 @@
// verifies that, when a child process exits (by calling `process.exit(code)`) // verifies that, when a child process exits (by calling `process.exit(code)`)
// - the parent receives the proper events in the proper order, no duplicates // - the parent receives the proper events in the proper order, no duplicates
// - the exitCode and signalCode are correct in the 'exit' event // - the exitCode and signalCode are correct in the 'exit' event
// - the worker.suicide flag, and worker.state are correct // - the worker.exitedAfterDisconnect flag, and worker.state are correct
// - the worker process actually goes away // - the worker process actually goes away


var common = require('../common'); var common = require('../common');
Expand Down Expand Up @@ -32,6 +32,8 @@ if (cluster.isWorker) {
worker_emitExit: [1, "the worker did not emit 'exit'"], worker_emitExit: [1, "the worker did not emit 'exit'"],
worker_state: ['disconnected', 'the worker state is incorrect'], worker_state: ['disconnected', 'the worker state is incorrect'],
worker_suicideMode: [false, 'the worker.suicide flag is incorrect'], worker_suicideMode: [false, 'the worker.suicide flag is incorrect'],
worker_exitedAfterDisconnect: [false,
'the .exitedAfterDisconnect flag is incorrect'],
worker_died: [true, 'the worker is still running'], worker_died: [true, 'the worker is still running'],
worker_exitCode: [EXIT_CODE, 'the worker exited w/ incorrect exitCode'], worker_exitCode: [EXIT_CODE, 'the worker exited w/ incorrect exitCode'],
worker_signalCode: [null, 'the worker exited w/ incorrect signalCode'] worker_signalCode: [null, 'the worker exited w/ incorrect signalCode']
Expand Down Expand Up @@ -66,6 +68,7 @@ if (cluster.isWorker) {
worker.on('disconnect', function() { worker.on('disconnect', function() {
results.worker_emitDisconnect += 1; results.worker_emitDisconnect += 1;
results.worker_suicideMode = worker.suicide; results.worker_suicideMode = worker.suicide;
results.worker_exitedAfterDisconnect = worker.exitedAfterDisconnect;
results.worker_state = worker.state; results.worker_state = worker.state;
if (results.worker_emitExit > 0) { if (results.worker_emitExit > 0) {
process.nextTick(function() { finish_test(); }); process.nextTick(function() { finish_test(); });
Expand Down
2 changes: 1 addition & 1 deletion test/parallel/test-cluster-worker-forced-exit.js
View file
Expand Up @@ -6,7 +6,7 @@ var cluster = require('cluster');
var SENTINEL = 42; var SENTINEL = 42;


// workers forcibly exit when control channel is disconnected, if // workers forcibly exit when control channel is disconnected, if
// their .suicide flag isn't set // their .exitedAfterDisconnect flag isn't set
// //
// test this by: // test this by:
// //
Expand Down
7 changes: 4 additions & 3 deletions test/parallel/test-cluster-worker-kill.js
View file
Expand Up @@ -3,7 +3,7 @@
// verifies that, when a child process is killed (we use SIGKILL) // verifies that, when a child process is killed (we use SIGKILL)
// - the parent receives the proper events in the proper order, no duplicates // - the parent receives the proper events in the proper order, no duplicates
// - the exitCode and signalCode are correct in the 'exit' event // - the exitCode and signalCode are correct in the 'exit' event
// - the worker.suicide flag, and worker.state are correct // - the worker.exitedAfterDisconnect flag, and worker.state are correct
// - the worker process actually goes away // - the worker process actually goes away


var common = require('../common'); var common = require('../common');
Expand All @@ -29,7 +29,8 @@ if (cluster.isWorker) {
worker_emitDisconnect: [1, "the worker did not emit 'disconnect'"], worker_emitDisconnect: [1, "the worker did not emit 'disconnect'"],
worker_emitExit: [1, "the worker did not emit 'exit'"], worker_emitExit: [1, "the worker did not emit 'exit'"],
worker_state: ['disconnected', 'the worker state is incorrect'], worker_state: ['disconnected', 'the worker state is incorrect'],
worker_suicideMode: [false, 'the worker.suicide flag is incorrect'], worker_exitedAfter: [false,
'the .exitedAfterDisconnect flag is incorrect'],
worker_died: [true, 'the worker is still running'], worker_died: [true, 'the worker is still running'],
worker_exitCode: [null, 'the worker exited w/ incorrect exitCode'], worker_exitCode: [null, 'the worker exited w/ incorrect exitCode'],
worker_signalCode: [KILL_SIGNAL, worker_signalCode: [KILL_SIGNAL,
Expand Down Expand Up @@ -65,7 +66,7 @@ if (cluster.isWorker) {
// Check worker events and properties // Check worker events and properties
worker.on('disconnect', function() { worker.on('disconnect', function() {
results.worker_emitDisconnect += 1; results.worker_emitDisconnect += 1;
results.worker_suicideMode = worker.suicide; results.worker_exitedAfter = worker.exitedAfterDisconnect;
results.worker_state = worker.state; results.worker_state = worker.state;
}); });


Expand Down

2 comments on commit 4f619bd

@Kreijstal
Copy link

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I think that's way more verbose.

@addaleax
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@rainbowdangerdash That is not necessarily a bad thing (and I don’t know if you are implying that it might be); the new name is arguably better at reflecting what is actually represented by the property.

Please sign in to comment.