diff --git a/lib/cluster.js b/lib/cluster.js index 1f8f7f3e0f436f..a9ae012ce4292d 100644 --- a/lib/cluster.js +++ b/lib/cluster.js @@ -138,7 +138,11 @@ RoundRobinHandle.prototype.add = function(worker, send) { // Still busy binding. this.server.once('listening', done); this.server.once('error', function(err) { - send(err.errno, null); + // Hack: translate 'EADDRINUSE' error string back to numeric error code. + // It works but ideally we'd have some backchannel between the net and + // cluster modules for stuff like this. + var errno = process.binding('uv')['UV_' + err.errno]; + send(errno, null); }); }; @@ -388,14 +392,12 @@ function masterInit() { // Set custom server data handle.add(worker, function(errno, reply, handle) { reply = util._extend({ - ack: message.seq, + errno: errno, key: key, + ack: message.seq, data: handles[key].data }, reply); - if (errno) { - reply.errno = errno; - delete handles[key]; // Gives other workers a chance to retry. - } + if (errno) delete handles[key]; // Gives other workers a chance to retry. send(worker, reply, handle); }); } @@ -487,63 +489,52 @@ function workerInit() { }; assert(IS_UNDEFINED(handles[key])); handles[key] = handle; - cb(handle); + cb(message.errno, handle); } // Round-robin. Master distributes handles across workers. function rr(message, cb) { if (message.errno) - onerror(message, cb); - else - onsuccess(message, cb); + return cb(message.errno, null); - function onerror(message, cb) { - function listen(backlog) { - // Translate 'EADDRINUSE' error back to numeric value. This function - // is called as sock._handle.listen(). - return process.binding('uv')['UV_' + message.errno]; - } - function close() { - } - cb({ close: close, listen: listen }); + var key = message.key; + function listen(backlog) { + // TODO(bnoordhuis) Send a message to the master that tells it to + // update the backlog size. The actual backlog should probably be + // the largest requested size by any worker. + return 0; } - function onsuccess(message, cb) { - var key = message.key; - function listen(backlog) { - // TODO(bnoordhuis) Send a message to the master that tells it to - // update the backlog size. The actual backlog should probably be - // the largest requested size by any worker. - return 0; - } - function close() { - // lib/net.js treats server._handle.close() as effectively synchronous. - // That means there is a time window between the call to close() and - // the ack by the master process in which we can still receive handles. - // onconnection() below handles that by sending those handles back to - // the master. - if (IS_UNDEFINED(key)) return; - send({ act: 'close', key: key }); - delete handles[key]; - key = undefined; - } - function getsockname(out) { - if (key) util._extend(out, message.sockname); - } - // Faux handle. Mimics a TCPWrap with just enough fidelity to get away - // with it. Fools net.Server into thinking that it's backed by a real - // handle. - var handle = { - close: close, - listen: listen - }; - if (message.sockname) { - handle.getsockname = getsockname; // TCP handles only. - } - assert(IS_UNDEFINED(handles[key])); - handles[key] = handle; - cb(handle); + function close() { + // lib/net.js treats server._handle.close() as effectively synchronous. + // That means there is a time window between the call to close() and + // the ack by the master process in which we can still receive handles. + // onconnection() below handles that by sending those handles back to + // the master. + if (IS_UNDEFINED(key)) return; + send({ act: 'close', key: key }); + delete handles[key]; + key = undefined; } + + function getsockname(out) { + if (key) util._extend(out, message.sockname); + return 0; + } + + // Faux handle. Mimics a TCPWrap with just enough fidelity to get away + // with it. Fools net.Server into thinking that it's backed by a real + // handle. + var handle = { + close: close, + listen: listen + }; + if (message.sockname) { + handle.getsockname = getsockname; // TCP handles only. + } + assert(IS_UNDEFINED(handles[key])); + handles[key] = handle; + cb(0, handle); } // Round-robin connection. diff --git a/lib/dgram.js b/lib/dgram.js index 1ac5c27c387856..de0675fb007731 100644 --- a/lib/dgram.js +++ b/lib/dgram.js @@ -191,7 +191,13 @@ Socket.prototype.bind = function(/*port, address, callback*/) { cluster = require('cluster'); if (cluster.isWorker) { - cluster._getServer(self, ip, port, self.type, -1, function(handle) { + cluster._getServer(self, ip, port, self.type, -1, function(err, handle) { + if (err) { + self.emit('error', errnoException(err, 'bind')); + self._bindState = BIND_STATE_UNBOUND; + return; + } + if (!self._handle) // handle has been closed in the mean time. return handle.close(); diff --git a/lib/net.js b/lib/net.js index ddce74b375eb67..6bd86e1896441e 100644 --- a/lib/net.js +++ b/lib/net.js @@ -1094,27 +1094,30 @@ function listen(self, address, port, addressType, backlog, fd) { return; } - cluster._getServer(self, address, port, addressType, fd, function(handle) { - // Some operating systems (notably OS X and Solaris) don't report EADDRINUSE - // errors right away. libuv mimics that behavior for the sake of platform - // consistency but that means we have have a socket on our hands that is - // not actually bound. That's why we check if the actual port matches what - // we requested and if not, raise an error. The exception is when port == 0 - // because that means "any random port". - if (port && handle.getsockname) { + cluster._getServer(self, address, port, addressType, fd, cb); + + function cb(err, handle) { + // EADDRINUSE may not be reported until we call listen(). To complicate + // matters, a failed bind() followed by listen() will implicitly bind to + // a random port. Ergo, check that the socket is bound to the expected + // port before calling listen(). + // + // FIXME(bnoordhuis) Doesn't work for pipe handles, they don't have a + // getsockname() method. Non-issue for now, the cluster module doesn't + // really support pipes anyway. + if (err === 0 && port > 0 && handle.getsockname) { var out = {}; - var err = handle.getsockname(out); - if (err === 0 && port !== out.port) { + err = handle.getsockname(out); + if (err === 0 && port !== out.port) err = uv.UV_EADDRINUSE; - } - if (err) { - return self.emit('error', errnoException(err, 'bind')); - } } + if (err) + return self.emit('error', errnoException(err, 'bind')); + self._handle = handle; self._listen2(address, port, addressType, backlog, fd); - }); + } } diff --git a/test/simple/test-cluster-shared-handle-bind-error.js b/test/simple/test-cluster-shared-handle-bind-error.js new file mode 100644 index 00000000000000..ceccd9146d742b --- /dev/null +++ b/test/simple/test-cluster-shared-handle-bind-error.js @@ -0,0 +1,47 @@ +// 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. + +var common = require('../common'); +var assert = require('assert'); +var cluster = require('cluster'); +var net = require('net'); + +if (cluster.isMaster) { + // Master opens and binds the socket and shares it with the worker. + cluster.schedulingPolicy = cluster.SCHED_NONE; + // Hog the TCP port so that when the worker tries to bind, it'll fail. + net.createServer(assert.fail).listen(common.PORT, function() { + var server = this; + var worker = cluster.fork(); + worker.on('exit', common.mustCall(function(exitCode) { + assert.equal(exitCode, 0); + server.close(); + })); + }); +} +else { + var s = net.createServer(assert.fail); + s.listen(common.PORT, assert.fail.bind(null, 'listen should have failed')); + s.on('error', common.mustCall(function(err) { + assert.equal(err.code, 'EADDRINUSE'); + process.disconnect(); + })); +}