From 30e462e91937ced3847af3fe9c393ebd32294b68 Mon Sep 17 00:00:00 2001 From: Ben Noordhuis Date: Fri, 10 Feb 2012 20:26:56 +0100 Subject: [PATCH] cluster: propagate bind errors This commit fixes a bug where the cluster module failed to propagate EADDRINUSE errors. When a worker starts a (net, http) server, it requests the listen socket from its master who then creates and binds the socket. Now, OS X and Windows don't always signal EADDRINUSE from bind() but instead defer the error until a later syscall. libuv mimics this behaviour to provide consistent behaviour across platforms but that means the worker could end up with a socket that is not actually bound to the requested addresss. That's why the worker now checks if the socket is bound, raising EADDRINUSE if that's not the case. Fixes #2721. --- lib/net.js | 24 +++- ...twice.js => test-cluster-bind-twice-v1.js} | 0 test/simple/test-cluster-bind-twice-v2.js | 115 ++++++++++++++++++ 3 files changed, 133 insertions(+), 6 deletions(-) rename test/simple/{test-cluster-bind-twice.js => test-cluster-bind-twice-v1.js} (100%) create mode 100644 test/simple/test-cluster-bind-twice-v2.js diff --git a/lib/net.js b/lib/net.js index e41e96ae1d..a797ed6fed 100644 --- a/lib/net.js +++ b/lib/net.js @@ -756,14 +756,26 @@ Server.prototype._listen2 = function(address, port, addressType) { function listen(self, address, port, addressType) { - if (process.env.NODE_WORKER_ID) { - require('cluster')._getServer(address, port, addressType, function(handle) { - self._handle = handle; - self._listen2(address, port, addressType); - }); - } else { + if (!process.env.NODE_WORKER_ID) { self._listen2(address, port, addressType); + return; } + + require('cluster')._getServer(address, port, addressType, function(handle) { + // OS X doesn't necessarily signal EADDRINUSE from bind(), it may defer + // the error until later. libuv mimics this behaviour to provide + // consistent behaviour across platforms but that means we could very + // well have a socket that is not actually bound... that's why we do + // this ghetto port check and raise EADDRINUSE if the requested and the + // actual port differ except if port == 0 because that means "any port". + if (port && port != handle.getsockname().port) { + self.emit('error', errnoException('EADDRINUSE', 'bind')); + return; + } + + self._handle = handle; + self._listen2(address, port, addressType); + }); } diff --git a/test/simple/test-cluster-bind-twice.js b/test/simple/test-cluster-bind-twice-v1.js similarity index 100% rename from test/simple/test-cluster-bind-twice.js rename to test/simple/test-cluster-bind-twice-v1.js diff --git a/test/simple/test-cluster-bind-twice-v2.js b/test/simple/test-cluster-bind-twice-v2.js new file mode 100644 index 0000000000..1842cce260 --- /dev/null +++ b/test/simple/test-cluster-bind-twice-v2.js @@ -0,0 +1,115 @@ +// 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. + +// This test starts two clustered HTTP servers on the same port. It expects the +// first cluster to succeed and the second cluster to fail with EADDRINUSE. +// +// The test may seem complex but most of it is plumbing that routes messages +// from the child processes back to the "super" master. As a tree it looks +// something like this: +// +// +// / \ +// +// / \ +// +// +// The first worker starts a server on a fixed port and fires a ready message +// that is routed to the second worker. When it tries to bind, it expects to +// see an EADDRINUSE error. +// +// See https://github.com/joyent/node/issues/2721 for more details. + +var common = require('../common'); +var assert = require('assert'); +var cluster = require('cluster'); +var fork = require('child_process').fork; +var http = require('http'); + +var id = process.argv[2]; + +if (!id) { + var a = fork(__filename, ['one']); + var b = fork(__filename, ['two']); + + a.on('message', function(m) { + if (typeof m === 'object') return; + assert.equal(m, 'READY'); + b.send('START'); + }); + + var ok = false; + + b.on('message', function(m) { + if (typeof m === 'object') return; // ignore system messages + assert.equal(m, 'EADDRINUSE'); + a.kill(); + b.kill(); + ok = true; + }); + + process.on('exit', function() { + a.kill(); + b.kill(); + assert(ok); + }); +} +else if (id === 'one') { + if (cluster.isMaster) return startWorker(); + + http.createServer(assert.fail).listen(common.PORT, function() { + process.send('READY'); + }); +} +else if (id === 'two') { + if (cluster.isMaster) return startWorker(); + + var ok = false; + process.on('SIGTERM', process.exit); + process.on('exit', function() { + assert(ok); + }); + + process.on('message', function(m) { + if (typeof m === 'object') return; // ignore system messages + assert.equal(m, 'START'); + var server = http.createServer(assert.fail); + server.listen(common.PORT, assert.fail); + server.on('error', function(e) { + assert.equal(e.code, 'EADDRINUSE'); + process.send(e.code); + ok = true; + }); + }); +} +else { + assert(0); // bad command line argument +} + +function startWorker() { + var worker = cluster.fork(); + worker.on('message', process.send); + process.on('message', worker.send.bind(worker)); + process.on('SIGTERM', function() { + worker.kill(); + process.exit(); + }); +}