Skip to content

Commit

Permalink
net: do not treat server.maxConnections=0 as Infinity
Browse files Browse the repository at this point in the history
Setting the `maxConnections` to 0 should result in no connection.
Instead, it was treated as if the option was not there.

PR-URL: #48276
Reviewed-By: Matteo Collina <matteo.collina@gmail.com>
Reviewed-By: Antoine du Hamel <duhamelantoine1995@gmail.com>
Reviewed-By: Luigi Pinca <luigipinca@gmail.com>
Reviewed-By: Paolo Insogna <paolo@cowtech.it>
  • Loading branch information
ignoramous committed Jun 26, 2023
1 parent b38bc9f commit 7202859
Show file tree
Hide file tree
Showing 5 changed files with 36 additions and 7 deletions.
5 changes: 5 additions & 0 deletions doc/api/net.md
Expand Up @@ -565,6 +565,11 @@ added: v5.7.0

<!-- YAML
added: v0.2.0
changes:
- version: REPLACEME
pr-url: https://github.com/nodejs/node/pull/48276
description: Setting `maxConnections` to `0` drops all the incoming
connections. Previously, it was interpreted as `Infinity`.
-->

* {integer}
Expand Down
2 changes: 1 addition & 1 deletion lib/internal/cluster/child.js
Expand Up @@ -233,7 +233,7 @@ function onconnection(message, handle) {

if (accepted && server[owner_symbol]) {
const self = server[owner_symbol];
if (self.maxConnections && self._connections >= self.maxConnections) {
if (self.maxConnections != null && self._connections >= self.maxConnections) {
accepted = false;
}
}
Expand Down
2 changes: 1 addition & 1 deletion lib/net.js
Expand Up @@ -2062,7 +2062,7 @@ function onconnection(err, clientHandle) {
return;
}

if (self.maxConnections && self._connections >= self.maxConnections) {
if (self.maxConnections != null && self._connections >= self.maxConnections) {
if (clientHandle.getsockname || clientHandle.getpeername) {
const data = { __proto__: null };
if (clientHandle.getsockname) {
Expand Down
19 changes: 14 additions & 5 deletions test/parallel/test-cluster-net-server-drop-connection.js
Expand Up @@ -10,25 +10,29 @@ const tmpdir = require('../common/tmpdir');
if (common.isWindows)
common.skip('no setSimultaneousAccepts on pipe handle');

let connectionCount = 0;
let listenCount = 0;
const totalConns = 10;
const totalWorkers = 3;
let worker0;
let worker1;
let worker2;
let connectionCount = 0;
let listenCount = 0;

function request(path) {
for (let i = 0; i < 10; i++) {
for (let i = 0; i < totalConns; i++) {
net.connect(path);
}
}

function handleMessage(message) {
assert.match(message.action, /listen|connection/);
if (message.action === 'listen') {
if (++listenCount === 2) {
if (++listenCount === totalWorkers) {
request(common.PIPE);
}
} else if (message.action === 'connection') {
if (++connectionCount === 10) {
if (++connectionCount === totalConns) {
worker0.send({ action: 'disconnect' });
worker1.send({ action: 'disconnect' });
worker2.send({ action: 'disconnect' });
}
Expand All @@ -38,8 +42,13 @@ function handleMessage(message) {
if (cluster.isPrimary) {
cluster.schedulingPolicy = cluster.SCHED_RR;
tmpdir.refresh();
worker0 = cluster.fork({ maxConnections: 0, pipePath: common.PIPE });
worker1 = cluster.fork({ maxConnections: 1, pipePath: common.PIPE });
worker2 = cluster.fork({ maxConnections: 9, pipePath: common.PIPE });
// expected = { action: 'listen' } + maxConnections * { action: 'connection' }
worker0.on('message', common.mustCall((message) => {
handleMessage(message);
}, 1));
worker1.on('message', common.mustCall((message) => {
handleMessage(message);
}, 2));
Expand Down
15 changes: 15 additions & 0 deletions test/parallel/test-net-server-drop-connections.js
Expand Up @@ -4,12 +4,23 @@ const assert = require('assert');
const net = require('net');

let firstSocket;
const dormantServer = net.createServer(common.mustNotCall());
const server = net.createServer(common.mustCall((socket) => {
firstSocket = socket;
}));

dormantServer.maxConnections = 0;
server.maxConnections = 1;

dormantServer.on('drop', common.mustCall((data) => {
assert.strictEqual(!!data.localAddress, true);
assert.strictEqual(!!data.localPort, true);
assert.strictEqual(!!data.remoteAddress, true);
assert.strictEqual(!!data.remotePort, true);
assert.strictEqual(!!data.remoteFamily, true);
dormantServer.close();
}));

server.on('drop', common.mustCall((data) => {
assert.strictEqual(!!data.localAddress, true);
assert.strictEqual(!!data.localPort, true);
Expand All @@ -20,6 +31,10 @@ server.on('drop', common.mustCall((data) => {
server.close();
}));

dormantServer.listen(0, () => {
net.createConnection(dormantServer.address().port);
});

server.listen(0, () => {
net.createConnection(server.address().port);
net.createConnection(server.address().port);
Expand Down

0 comments on commit 7202859

Please sign in to comment.