From 4593c049592d1dda6f61279887474bac132bb5ab Mon Sep 17 00:00:00 2001 From: Ryan Dahl Date: Sun, 15 Aug 2010 14:01:44 -0700 Subject: [PATCH] Implement net.Server.maxConnections Simplify EMFILE behavior. --- doc/api.markdown | 9 +++ lib/net.js | 47 ++++------- test/simple/test-net-pingpong.js | 1 + .../simple/test-net-server-max-connections.js | 79 +++++++++++++++++++ 4 files changed, 106 insertions(+), 30 deletions(-) create mode 100644 test/simple/test-net-server-max-connections.js diff --git a/doc/api.markdown b/doc/api.markdown index 677af4f90de..f80b8fa4f6e 100644 --- a/doc/api.markdown +++ b/doc/api.markdown @@ -2196,6 +2196,15 @@ Stops the server from accepting new connections. This function is asynchronous, the server is finally closed when the server emits a `'close'` event. +### server.maxConnections + +Set this property to reject connections when the server's connection count gets high. + +### server.connections + +The number of concurrent connections on the server. + + ## net.Stream diff --git a/lib/net.js b/lib/net.js index 3469e826065..2349bc19094 100644 --- a/lib/net.js +++ b/lib/net.js @@ -220,29 +220,6 @@ var ioWatchers = new FreeList("iowatcher", 100, function () { }); -// waitingForFDs stores servers which have experienced EMFILE. -// When a file descriptor becomes available through closeFD() -// a server from waitingForFDs is started. - -var waitingForFDs = []; - -function closeFD(fd) { - close(fd); - - // Try to recover from EMFILE - - var server, serverFD; - while (server = waitingForFDs.shift()) { - serverFD = parseInt(server.fd); - if (serverFD && serverFD > 0) { - server.watcher.set(serverFD, true, false); - server.watcher.start(); - return; - } - } -} - - // Allocated on demand. var pool = null; function allocNewPool () { @@ -997,9 +974,13 @@ Stream.prototype.destroy = function (exception) { this.secureStream.close(); } + if (this.server) { + this.server.connections--; + } + // FIXME Bug when this.fd == 0 if (typeof this.fd == 'number') { - closeFD(this.fd); + close(this.fd); this.fd = null; process.nextTick(function () { if (exception) self.emit('error', exception); @@ -1058,6 +1039,8 @@ function Server (listener) { self.addListener('connection', listener); } + self.connections = 0; + self.watcher = new IOWatcher(); self.watcher.host = self; self.watcher.callback = function () { @@ -1065,15 +1048,19 @@ function Server (listener) { try { var peerInfo = accept(self.fd); } catch (e) { - if (e.errno == EMFILE) { - waitingForFDs.push(self); - self.watcher.stop(); - return; - } + if (e.errno == EMFILE) return; throw e; } if (!peerInfo) return; + if (self.maxConnections && self.connections >= self.maxConnections) { + // Accept and close the connection. + close(peerInfo.fd); + return; + } + + self.connections++; + var s = new Stream(peerInfo.fd, self.type); s.remoteAddress = peerInfo.address; s.remotePort = peerInfo.port; @@ -1209,7 +1196,7 @@ Server.prototype.close = function () { self.watcher.stop(); - closeFD(self.fd); + close(self.fd); self.fd = null; if (self.type === "unix") { diff --git a/test/simple/test-net-pingpong.js b/test/simple/test-net-pingpong.js index f00d6528102..5db5e127e3f 100644 --- a/test/simple/test-net-pingpong.js +++ b/test/simple/test-net-pingpong.js @@ -13,6 +13,7 @@ function pingPongTest (port, host) { var server = net.createServer(function (socket) { console.log("connection: " + socket.remoteAddress); assert.equal(server, socket.server); + assert.equal(1, server.connections); socket.setNoDelay(); socket.timeout = 0; diff --git a/test/simple/test-net-server-max-connections.js b/test/simple/test-net-server-max-connections.js new file mode 100644 index 00000000000..daf516d567d --- /dev/null +++ b/test/simple/test-net-server-max-connections.js @@ -0,0 +1,79 @@ +common = require("../common"); +assert = common.assert; +net = require('net'); + +// This test creates 200 connections to a server and sets the server's +// maxConnections property to 100. The first 100 connections make it through +// and the last 100 connections are rejected. +// TODO: test that the server can accept more connections after it reaches +// its maximum and some are closed. + +N = 200; +count = 0; +closes = 0; +waits = []; + +server = net.createServer(function (connection) { + console.error("connect %d", count++); + connection.write("hello"); + waits.push(function () { connection.end(); }); +}); + +server.listen(common.PORT, function () { + for (var i = 0; i < N; i++) { + makeConnection(i); + } +}); + +server.maxConnections = N/2; + +console.error("server.maxConnections = %d", server.maxConnections); + + +function makeConnection (index) { + setTimeout(function () { + var c = net.createConnection(common.PORT); + var gotData = false; + + c.on('end', function () { c.end(); }); + + c.on('data', function (b) { + gotData = true; + assert.ok(0 < b.length); + }); + + c.on('error', function (e) { + console.error("error %d: %s", index, e); + }); + + c.on('close', function () { + console.error("closed %d", index); + closes++; + + if (closes < N/2) { + assert.ok(server.maxConnections <= index, + index + " was one of the first closed connections but shouldnt have been"); + } + + if (closes === N/2) { + var cb; + console.error("calling wait callback."); + while (cb = waits.shift()) { + cb(); + } + server.close(); + } + + if (index < server.maxConnections) { + assert.equal(true, gotData, index + " didn't get data, but should have"); + } else { + assert.equal(false, gotData, index + " got data, but shouldn't have"); + } + }); + }, index); +} + + +process.on('exit', function () { + assert.equal(N, closes); +});