It's documented as such but didn't actually behave that way.
Bug introduced in commit 66fc8ca ("cluster: emit 'message' event on
cluster master"), which is the commit that introduced the event.
Fixes: https://github.com/nodejs/node/issues/5126
PR-URL: https://github.com/nodejs/node/pull/5361
Reviewed-By: Ali Ijaz Sheikh <ofrobots@google.com>
Reviewed-By: Colin Ihrig <cjihrig@gmail.com>
Reviewed-By: Sakthipriyan Vairamani <thechargingvolcano@gmail.com>
PR-URL: https://github.com/nodejs/node/pull/5216
Reviewed-By: Colin Ihrig <cjihrig@gmail.com>
Reviewed-By: Brian White <mscdex@mscdex.net>
Reviewed-By: James M Snell <jasnell@gmail.com>
Reviewed-By: Rod Vagg <rod@vagg.org>
There is no guarantee that the `suicide` property of a worker in the
master process is going to be set when the `disconnect` and `exit`
events are emitted.
To fix it, wait for the ACK of the suicide message from the master
before disconnecting the worker. Also, there's no need to send the
suicide message from the worker if the disconnection has been
initiated in the master.
Add `test-cluster-disconnect-suicide-race` that forks a lot of workers
to consistently reproduce the issue this patch tries to solve.
Modify `test-regress-GH-3238` so it checks both the `kill` and
`disconnect` cases. Also take into account that the `disconnect` event
may be received after the `exit` event.
PR-URL: https://github.com/nodejs/node/pull/4349
Reviewed-By: Colin Ihrig <cjihrig@gmail.com>
Reviewed-By: Rich Trott <rtrott@gmail.com>
It avoids the creation of unnecessary handles. This issue is causing
intermitent failures in `test-cluster-disconnect-race` on `FreeBSD`
and `OS X`.
The problem is that the `worker2.disconnect` is being called on the
master before the `queryServer` is handled, causing the worker to
be deleted, then the Server handle is created afterwards. Later on,
when `removeWorker` is called from the `exit` handler, there are no
workers left, but one handle, thus the `AssertionError`.
Add a new `test/sequential/test-cluster-disconnect-leak` based on
`test-cluster-disconnect-race` that creates lots of workers and fails
consistently without this patch.
PR-URL: https://github.com/nodejs/node/pull/4465
Reviewed-By: James M Snell <jasnell@gmail.com>
Reviewed-By: Johan Bergström <bugs@bergstroem.nu>
Reviewed-By: Rich Trott <rtrott@gmail.com>
use `arrow functions` instead of `bind(this)` in order to improve
performance through optimizations.
PR-URL: https://github.com/nodejs/node/pull/3622
Reviewed-By: Jeremiah Senkpiel <fishrock123@rocketmail.com>
Reviewed-By: Trevor Norris <trev.norris@gmail.com>
Reviewed-By: James M Snell <jasnell@gmail.com>
Reviewed-By: Myles Borins <myles.borins@gmail.com>
Due to the race window between the master's "disconnect" message and the
worker's "handle received" message, connections sometimes got stuck in
the pending handles queue when calling `worker.disconnect()` in the
master process.
The observable effect from the client's perspective was a TCP or HTTP
connection that simply stalled. This commit fixes that by closing open
handles in the master when the "disconnect" message is sent.
Fixes: https://github.com/nodejs/node/issues/3551
PR-URL: https://github.com/nodejs/node/pull/3677
Reviewed-By: Colin Ihrig <cjihrig@gmail.com>
Reviewed-By: Fedor Indutny <fedor@indutny.com>
Reviewed-By: James M Snell <jasnell@gmail.com>
As per the `prefer-const` eslint rule, few instances of `let` have been
identified to be better with `const`. This patch updates all those
instances.
Refer: https://github.com/nodejs/node/issues/3118
PR-URL: https://github.com/nodejs/node/pull/3152
Reviewed-By: Colin Ihrig <cjihrig@gmail.com>
Reviewed-By: Сковорода Никита Андреевич <chalkerx@gmail.com>
Reviewed-By: Roman Reiss <me@silverwind.io>
Reviewed-By: Michaël Zasso <mic.besace@gmail.com>
The `events` module already exports `EventEmitter` constructor function
So, we don't have to use `events.EventEmitter` to access it.
Refer: https://github.com/nodejs/node/pull/2896
PR-URL: https://github.com/nodejs/node/pull/2921
Reviewed-By: Roman Reiss <me@silverwind.io>
Reviewed-By: Michaël Zasso <mic.besace@gmail.com>
Currently, each cluster worker is assigned an ever increasing
--debug-port argument. A long running cluster application that
does not use the debugger can run into errors related to the
port range. This commit mitigates the problem by only setting
the debug port if the master is started with debug arguments, or
the user explicitly defines debug arguments for the worker. This
commit also adds a new debug port offset counter that is only
incremented when a worker is created that utilizes debugging.
Fixes: https://github.com/joyent/node/issues/8159
Refs: https://github.com/nodejs/io.js/pull/1524
PR-URL: https://github.com/nodejs/io.js/pull/1949
Reviewed-By: Ben Noordhuis <info@bnoordhuis.nl>
Reviewed-By: Oleg Elifantiev <oleg@elifantiev.ru>
Add ref() and unref() stub methods to the faux handle in round-robin
mode. Fixes the following TypeError when calling `server.unref()` in
the worker:
net.js:1521
this._handle.unref();
^
TypeError: this._handle.unref is not a function
at Server.unref (net.js:1521:18)
No actual reference counting is implemented. It would effectively be
a no-op because the control channel would still keep the worker alive.
Fixes: https://github.com/nodejs/node/issues/73
PR-URL: https://github.com/nodejs/io.js/pull/2274
Reviewed-By: Evan Lucas <evanlucas@me.com>
Reviewed-By: Jeremiah Senkpiel <fishrock123@rocketmail.com>
For consistency with the worker 'exit', 'online', 'disconnect', and
'listening' events which are emitted on worker and cluster, also emit
'message' on cluster.
Reviewed-by: Sam Roberts <vieuxtech@gmail.com>
Reviewed-by: Christian Tellnes <christian@tellnes.no>
Reviewed-by: Stephen Belanger <admin@stephenbelanger.com>
PR-URL: https://github.com/nodejs/io.js/pull/861
Before this, cluster behaves not the way it is documented. When
disconnect is triggered, worker must wait for every server is closed
before doing disconnect actually.
Reviewed-By: Sam Roberts <vieuxtech@gmail.com>
Reviewed-By: Colin Ihrig <cjihrig@gmail.com>
PR-URL: https://github.com/nodejs/io.js/pull/1400
Fixes: https://github.com/nodejs/io.js/issues/1305
This commit better handles calls to process.binding() in lib/ by
no longer lazy loading the bindings (the load times themselves are
rather miniscule compared to the load time of V8) and never reloading
the bindings (which is 172 times slower than referencing a variable with
the same value).
PR-URL: https://github.com/iojs/io.js/pull/1367
Reviewed-By: Brian White <mscdex@mscdex.net>
Reviewed-By: Ben Noordhuis <info@bnoordhuis.nl>
Many of the util.is*() methods used to check data types
simply compare against a single value or the result of
typeof. This commit replaces calls to these methods with
equivalent checks. This commit does not touch calls to the
more complex methods (isRegExp(), isDate(), etc.).
Fixes: https://github.com/iojs/io.js/issues/607
PR-URL: https://github.com/iojs/io.js/pull/647
Reviewed-By: Ben Noordhuis <info@bnoordhuis.nl>
This commit replaces a number of var statements throughout
the lib code with const statements.
PR-URL: https://github.com/iojs/io.js/pull/541
Reviewed-By: Ben Noordhuis <info@bnoordhuis.nl>
Previously if a worker's state machine had already transitioned into the
'listening' state when it received the message enabling the debugger,
the worker would never enable its debugger.
Change the logic to allow the 'listening' as a valid state for enabling
the debugger.
Fixes: https://github.com/joyent/node/issues/6440
Original-PR-URL: https://github.com/joyent/node/pull/9037
Signed-off-by: Julien Gilli <julien.gilli@joyent.com>
Fixes: https://github.com/iojs/io.js/issues/340
PR-URL: https://github.com/iojs/io.js/pull/501
Reviewed-By: Ben Noordhuis <info@bnoordhuis.nl>
Reviewed-By: Bert Belder <bertbelder@gmail.com>
Reviewed-By: Rod Vagg <rod@vagg.org>
The copyright and license notice is already in the LICENSE file. There
is no justifiable reason to also require that it be included in every
file, since the individual files are not individually distributed except
as part of the entire package.
Make the cluster module intercept the `--debug-port=<port>` command line
switch and replace it with the debug port of the child process.
A happy coincidence of this change is that it finally makes it possible
to run the sequential/test-debug-signal-cluster in parallel, it now no
longer needs the default port numbers.
PR-URL: https://github.com/iojs/io.js/pull/306
Reviewed-By: Miroslav Bajtoš <miroslav@strongloop.com>
Workers that are already disconnected but not yet exited should not be
disconnected, trying to do so raises exceptions.
PR-URL: https://github.com/joyent/node/pull/8642
Reviewed-by: Bert Belder <bertbelder@gmail.com>
For shared handles that do not get connection close messages (UDP/dgram
is the only example of this), cluster must not assume that a port
listened on by one worker is listened on by all workers.
PR-URL: https://github.com/joyent/node/pull/8642
Reviewed-by: Bert Belder <bertbelder@gmail.com>
Turn on strict mode for the files in the lib/ directory. It helps
catch bugs and can have a positive effect on performance.
PR-URL: https://github.com/node-forward/node/pull/64
Reviewed-By: Colin Ihrig <cjihrig@gmail.com>
Reviewed-By: Fedor Indutny <fedor@indutny.com>
Do not send signal to children if they are already in debug mode.
Node.js on Windows does not register signal handler, and thus calling
`process._debugProcess()` will throw an error.
Reviewed-By: Trevor Norris <trevnorris@gmail.com>
PR-URL: https://github.com/joyent/node/pull/8476
Do not send signal to children if they are already in debug mode.
Node.js on Windows does not register signal handler, and thus calling
`process._debugProcess()` will throw an error.
Reviewed-By: Trevor Norris <trevnorris@gmail.com>
PR-URL: https://github.com/joyent/node/pull/8476
Currently, cluster workers can be removed from the workers list in three
different places:
- In the exit event handler for the worker process.
- In the disconnect event handler of the worker process.
- In the disconnect event handler of the cluster master.
However, handles for a given worker are cleaned up only in one of these
places: in the cluster master's disconnect event handler.
Because these events happen asynchronously, it is possible that the
workers list is empty before we even clean up one handle. This makes
the assert that makes sure that no handle is left when the workers
list is empty fail.
This commit removes the worker from the cluster.workers list only when
the worker is dead _and_ disconnected, at which point we're sure that
its associated handles are cleaned up.
Fixes#8191 and #8192.
Reviewed-By: Fedor Indutny <fedor@indutny.com>
Callbacks in node are usually asynchronous, and should never be
sometimes synchronous, and sometimes asynchronous.
Reviewed-by: Trevor Norris <trev.norris@gmail.com>
Between 0.11.1 and 0.11.2, the message and error events stopped
being usable via the cluster.worker object. This commit makes
them usable again. Closes#7998.
Signed-off-by: Fedor Indutny <fedor@indutny.com>
Emits on every call to cluster.setupMaster(), even if no new settings
are given. This is because calling cluster.setupMaster() without
arguments (or with an empty options object) results in the settings
being restored to their defaults.
Signed-off-by: Fedor Indutny <fedor@indutny.com>
Only attributes of 'cluster.settings' will be modified after the first
call, leaving all other cluster initialization alone. Each call that
includes a 'settings' argument triggers a 'setup' event to be emitted.
Instead of each call resetting all values to their defaults, use the
current settings (if any) as the default. This retains setupMaster's
support how cluster.fork() uses setupMaster() to ensure
cluster.settings has been populated.
Update example in docs to use current node coding style and include
an example of progressive configuration.
Signed-off-by: Fedor Indutny <fedor@indutny.com>
In v0.10.x, process.argv and process.execArgv would only be
evaluated and copied into cluster.settings on the first call to
cluster.setupMaster() (either directly or via cluster.fork()),
allowing them to be modified as needed before initializing the
settings.
In 41b75ca the behaviour was changed so that these values are
initialized at the time of the first require('cluster').
Fixes#7670.
Signed-off-by: Trevor Norris <trev.norris@gmail.com>
This is a problem present in both v0.10, and v0.11, where the 'setup'
event is synchronously emitted by `cluster.setupMaster()`, a mostly
harmless anti-pattern.
Fix inadvertent v0.11 changes to the definition of suicide, particularly
the relationship between suicide state, the disconnect event, and when
exit should occur.
In v0.10, workers don't forcibly exit on disconnect, it doesn't give
them time to do a graceful finish of open client connections, they exit
under normal node rules - when there is nothing left to do. But on
unexpected disconnect they do exit so the workers aren't left around
after the master.
Note that a test as-written was invalid, it failed against the v0.10
cluster API, demonstrating that it was an undocumented API change.
Fixes issue in 0.11 where callback doesn't occur if worker count is
currently zero. In 0.10 callback occurs after worker count is zero, and
occurs in next tick if worker count is currently zero.
Don't emit the 'disconnect' event until all workers have gone away.
Before this commit, the event was emitted when all open handles were
closed, which usually - but not always - amounts to the same thing.
Fixes#6346.
If `obj` given to `cluster._getServer` has `_setServerData` or
`_getServerData` methods, the data will be synchronized across workers
and stored in master.
Ben Noordhuis
Igor Klopov
Santiago Gimeno
Minwoo Jung
cjihrig
Sakthipriyan Vairamani
Rich Trott
Fedor Indutny
Sam Roberts
Oleg Elifantiev
Yosuke Furukawa
Trevor Norris
Brendan Ashworth
Timothy J Fontaine
isaacs
Julien Gilli
Ryan Graham
Fedor Indutny
Brian White