From 4f619bde4c20fc46fa3e1b8671ab7174d29f340d Mon Sep 17 00:00:00 2001 From: Evan Lucas Date: Tue, 10 Nov 2015 13:14:34 -0600 Subject: [PATCH] cluster: migrate from worker.suicide MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Replace it with worker.exitedAfterDisconnect. Print deprecation message when getting or setting until it is removed. PR-URL: https://github.com/nodejs/node/pull/3743 Fixes: https://github.com/nodejs/node/issues/3721 Reviewed-By: James M Snell Reviewed-By: Сковорода Никита Андреевич Reviewed-By: Anna Henningsen Reviewed-By: Myles Borins Reviewed-By: Stephen Belanger --- doc/api/cluster.md | 46 ++++++++++++++---- lib/cluster.js | 48 ++++++++++++------- .../test-cluster-worker-disconnect.js | 10 ++-- test/parallel/test-cluster-worker-exit.js | 5 +- .../test-cluster-worker-forced-exit.js | 2 +- test/parallel/test-cluster-worker-kill.js | 7 +-- 6 files changed, 84 insertions(+), 34 deletions(-) diff --git a/doc/api/cluster.md b/doc/api/cluster.md index 8937aa91287dc0..dcf2f8a1e92f85 100644 --- a/doc/api/cluster.md +++ b/doc/api/cluster.md @@ -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 `.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, but connections may be accepted by any other listening worker. Existing @@ -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 * {Number} @@ -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 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. @@ -340,8 +361,8 @@ is stored. See: [Child Process module][] Note that workers will call `process.exit(0)` if the `'disconnect'` event occurs -on `process` and `.suicide` is not `true`. This protects against accidental -disconnection. +on `process` and `.exitedAfterDisconnect` is not `true`. This protects against +accidental disconnection. ### worker.send(message[, sendHandle][, callback]) @@ -374,17 +395,20 @@ if (cluster.isMaster) { ### 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 -exit, the master may choose not to respawn a worker based on this value. +Set by calling `.kill()` or `.disconnect()`. Until then, it is `undefined`. + +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 cluster.on('exit', (worker, code, signal) => { 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'). } }); @@ -392,6 +416,9 @@ cluster.on('exit', (worker, code, signal) => { worker.kill(); ``` +This API only exists for backwards compatibility and will be removed in the +future. + ## Event: 'disconnect' * `worker` {cluster.Worker} @@ -707,6 +734,7 @@ socket.on('data', (id) => { [`disconnect`]: child_process.html#child_process_child_disconnect [`kill`]: process.html#process_process_kill_pid_signal [`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 event: 'exit']: child_process.html#child_process_event_exit [child_process event: 'message']: child_process.html#child_process_event_message diff --git a/lib/cluster.js b/lib/cluster.js index b43d4a0a34fa7d..651b2f481de85f 100644 --- a/lib/cluster.js +++ b/lib/cluster.js @@ -27,7 +27,20 @@ function Worker(options) { if (options === null || typeof options !== 'object') 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.id = options.id | 0; @@ -355,7 +368,7 @@ function masterInit() { removeWorker(worker); } - worker.suicide = !!worker.suicide; + worker.exitedAfterDisconnect = !!worker.exitedAfterDisconnect; worker.state = 'dead'; worker.emit('exit', exitCode, signalCode); cluster.emit('exit', worker, exitCode, signalCode); @@ -376,7 +389,7 @@ function masterInit() { */ if (worker.isDead()) removeWorker(worker); - worker.suicide = !!worker.suicide; + worker.exitedAfterDisconnect = !!worker.exitedAfterDisconnect; worker.state = 'disconnected'; worker.emit('disconnect'); cluster.emit('disconnect', worker); @@ -407,7 +420,7 @@ function masterInit() { }; Worker.prototype.disconnect = function() { - this.suicide = true; + this.exitedAfterDisconnect = true; send(this, { act: 'disconnect' }); removeHandlesForWorker(this); removeWorker(this); @@ -432,8 +445,8 @@ function masterInit() { queryServer(worker, message); else if (message.act === 'listening') listening(worker, message); - else if (message.act === 'suicide') - suicide(worker, message); + else if (message.act === 'exitedAfterDisconnect') + exitedAfterDisconnect(worker, message); else if (message.act === 'close') close(worker, message); } @@ -444,14 +457,14 @@ function masterInit() { cluster.emit('online', worker); } - function suicide(worker, message) { - worker.suicide = true; + function exitedAfterDisconnect(worker, message) { + worker.exitedAfterDisconnect = true; send(worker, { ack: message.seq }); } function queryServer(worker, message) { // Stop processing if worker already disconnecting - if (worker.suicide) + if (worker.exitedAfterDisconnect) return; var args = [message.address, message.port, @@ -533,7 +546,7 @@ function workerInit() { cluster.worker = worker; process.once('disconnect', function() { worker.emit('disconnect'); - if (!worker.suicide) { + if (!worker.exitedAfterDisconnect) { // Unexpected disconnect, master exited, or some such nastiness, so // worker exits immediately. process.exit(0); @@ -670,10 +683,10 @@ function workerInit() { }; Worker.prototype.destroy = function() { - this.suicide = true; + this.exitedAfterDisconnect = true; if (!this.isConnected()) process.exit(0); var exit = process.exit.bind(null, 0); - send({ act: 'suicide' }, () => process.disconnect()); + send({ act: 'exitedAfterDisconnect' }, () => process.disconnect()); process.once('disconnect', exit); }; @@ -682,19 +695,20 @@ function workerInit() { } function _disconnect(masterInitiated) { - this.suicide = true; + this.exitedAfterDisconnect = true; let waitingCount = 1; function checkWaitingCount() { waitingCount--; if (waitingCount === 0) { - // If disconnect is worker initiated, wait for ack to be sure suicide - // is properly set in the master, otherwise, if it's master initiated - // there's no need to send the suicide message + // If disconnect is worker initiated, wait for ack to be sure + // exitedAfterDisconnect is properly set in the master, otherwise, if + // it's master initiated there's no need to send the + // exitedAfterDisconnect message if (masterInitiated) { process.disconnect(); } else { - send({ act: 'suicide' }, () => process.disconnect()); + send({ act: 'exitedAfterDisconnect' }, () => process.disconnect()); } } } diff --git a/test/parallel/test-cluster-worker-disconnect.js b/test/parallel/test-cluster-worker-disconnect.js index 4c9aaf1115d13f..6ee12795fd50d5 100644 --- a/test/parallel/test-cluster-worker-disconnect.js +++ b/test/parallel/test-cluster-worker-disconnect.js @@ -8,8 +8,12 @@ if (cluster.isWorker) { http.Server(function() { }).listen(common.PORT, '127.0.0.1'); + const worker = cluster.worker; + assert.strictEqual(worker.exitedAfterDisconnect, worker.suicide); cluster.worker.on('disconnect', function() { + assert.strictEqual(cluster.worker.exitedAfterDisconnect, + cluster.worker.suicide); process.exit(42); }); @@ -26,7 +30,7 @@ if (cluster.isWorker) { emitDisconnectInsideWorker: false, emitExit: false, state: false, - suicideMode: false, + voluntaryMode: false, died: false } }; @@ -60,7 +64,7 @@ if (cluster.isWorker) { // Check worker events and properties worker.once('disconnect', function() { checks.worker.emitDisconnect = true; - checks.worker.suicideMode = worker.suicide; + checks.worker.voluntaryMode = worker.exitedAfterDisconnect; checks.worker.state = worker.state; }); @@ -86,7 +90,7 @@ if (cluster.isWorker) { // flags 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 assert.ok(w.died, 'The worker did not die'); diff --git a/test/parallel/test-cluster-worker-exit.js b/test/parallel/test-cluster-worker-exit.js index 60c80ec938555d..ad1e30ddb01994 100644 --- a/test/parallel/test-cluster-worker-exit.js +++ b/test/parallel/test-cluster-worker-exit.js @@ -3,7 +3,7 @@ // 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 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 var common = require('../common'); @@ -32,6 +32,8 @@ if (cluster.isWorker) { worker_emitExit: [1, "the worker did not emit 'exit'"], worker_state: ['disconnected', 'the worker state 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_exitCode: [EXIT_CODE, 'the worker exited w/ incorrect exitCode'], worker_signalCode: [null, 'the worker exited w/ incorrect signalCode'] @@ -66,6 +68,7 @@ if (cluster.isWorker) { worker.on('disconnect', function() { results.worker_emitDisconnect += 1; results.worker_suicideMode = worker.suicide; + results.worker_exitedAfterDisconnect = worker.exitedAfterDisconnect; results.worker_state = worker.state; if (results.worker_emitExit > 0) { process.nextTick(function() { finish_test(); }); diff --git a/test/parallel/test-cluster-worker-forced-exit.js b/test/parallel/test-cluster-worker-forced-exit.js index ef9f7728bad3a2..4f42532b553059 100644 --- a/test/parallel/test-cluster-worker-forced-exit.js +++ b/test/parallel/test-cluster-worker-forced-exit.js @@ -6,7 +6,7 @@ var cluster = require('cluster'); var SENTINEL = 42; // workers forcibly exit when control channel is disconnected, if -// their .suicide flag isn't set +// their .exitedAfterDisconnect flag isn't set // // test this by: // diff --git a/test/parallel/test-cluster-worker-kill.js b/test/parallel/test-cluster-worker-kill.js index 7c2465f9a463d1..bce4af11b7bb1f 100644 --- a/test/parallel/test-cluster-worker-kill.js +++ b/test/parallel/test-cluster-worker-kill.js @@ -3,7 +3,7 @@ // verifies that, when a child process is killed (we use SIGKILL) // - the parent receives the proper events in the proper order, no duplicates // - 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 var common = require('../common'); @@ -29,7 +29,8 @@ if (cluster.isWorker) { 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_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_exitCode: [null, 'the worker exited w/ incorrect exitCode'], worker_signalCode: [KILL_SIGNAL, @@ -65,7 +66,7 @@ if (cluster.isWorker) { // Check worker events and properties worker.on('disconnect', function() { results.worker_emitDisconnect += 1; - results.worker_suicideMode = worker.suicide; + results.worker_exitedAfter = worker.exitedAfterDisconnect; results.worker_state = worker.state; });