node: enforce maxConnections correctly

src/js/node/net.ts

@@ -93,7 +93,6 @@ const bunSocketServerOptions = Symbol.for("::bunnetserveroptions::");
 const owner_symbol = Symbol("owner_symbol");
 
 const kServerSocket = Symbol("kServerSocket");
-const kServerCounted = Symbol("kServerCounted");
 const kBytesWritten = Symbol("kBytesWritten");
 const bunTLSConnectOptions = Symbol.for("::buntlsconnectoptions::");
 const kReinitializeHandle = Symbol("kReinitializeHandle");
@@ -343,10 +342,7 @@ const ServerHandlers: SocketHandler = {
     const data = this.data;
     if (!data) return;
 
-    if (data[kServerCounted]) {
-      data.server[bunSocketServerConnections]--;
-      data[kServerCounted] = false;
-    }
+    data.server[bunSocketServerConnections]--;
     {
       if (!data[kclosed]) {
         data[kclosed] = true;
@@ -392,7 +388,7 @@ const ServerHandlers: SocketHandler = {
         return;
       }
     }
-    if (self.maxConnections && self[bunSocketServerConnections] >= self.maxConnections) {
+    if (self.maxConnections != null && self[bunSocketServerConnections] >= self.maxConnections) {
       const data = {
         localAddress: _socket.localAddress,
         localPort: _socket.localPort || this.localPort,
@@ -412,7 +408,6 @@ const ServerHandlers: SocketHandler = {
     const isTLS = typeof bunTLS === "function";
 
     self[bunSocketServerConnections]++;
-    _socket[kServerCounted] = true;
 
     if (typeof connectionListener === "function") {
       this.pauseOnConnect = pauseOnConnect;

test/js/node/test/parallel/test-net-server-drop-connections.js

@@ -0,0 +1,49 @@
+'use strict';
+const common = require('../common');
+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);
+    assert.strictEqual(!!data.remoteAddress, true);
+    assert.strictEqual(!!data.remotePort, true);
+    assert.strictEqual(!!data.remoteFamily, true);
+    firstSocket.destroy();
+    server.close();
+  }),
+);
+
+dormantServer.listen(0, () => {
+  net.createConnection(dormantServer.address().port);
+});
+
+server.listen(0, () => {
+  net.createConnection(server.address().port);
+  net.createConnection(server.address().port);
+});

test/js/node/test/parallel/test-net-server-max-connections.js

@@ -1,95 +0,0 @@
-'use strict';
-const common = require('../common');
-const assert = require('assert');
-const net = require('net');
-
-// This test creates 20 connections to a server and sets the server's
-// maxConnections property to 10. The first 10 connections make it through
-// and the last 10 connections are rejected.
-
-const N = 20;
-let closes = 0;
-const waits = [];
-
-const server = net.createServer(
-  common.mustCall(function (connection) {
-    connection.write('hello');
-    waits.push(function () {
-      connection.end();
-    });
-  }, N / 2),
-);
-
-server.listen(0, function () {
-  makeConnection(0);
-});
-
-server.maxConnections = N / 2;
-
-function makeConnection(index) {
-  const c = net.createConnection(server.address().port);
-  let gotData = false;
-
-  c.on('connect', function () {
-    if (index + 1 < N) {
-      makeConnection(index + 1);
-    }
-
-    c.on('close', function () {
-      console.error(`closed ${index}`);
-      closes++;
-
-      if (closes < N / 2) {
-        assert.ok(
-          server.maxConnections <= index,
-          `${index} should not have been one of the first closed connections`,
-        );
-      }
-
-      if (closes === N / 2) {
-        let cb;
-        console.error('calling wait callback.');
-        while ((cb = waits.shift()) !== undefined) {
-          cb();
-        }
-        server.close();
-      }
-
-      if (index < server.maxConnections) {
-        assert.strictEqual(
-          gotData,
-          true,
-          `${index} didn't get data, but should have`,
-        );
-      } else {
-        assert.strictEqual(
-          gotData,
-          false,
-          `${index} got data, but shouldn't have`,
-        );
-      }
-    });
-  });
-
-  c.on('end', function () {
-    c.end();
-  });
-
-  c.on('data', function (b) {
-    gotData = true;
-    assert.ok(b.length > 0);
-  });
-
-  c.on('error', function (e) {
-    // Retry if SmartOS and ECONNREFUSED. See
-    // https://github.com/nodejs/node/issues/2663.
-    if (common.isSunOS && e.code === 'ECONNREFUSED') {
-      c.connect(server.address().port);
-    }
-    console.error(`error ${index}: ${e}`);
-  });
-}
-
-process.on('exit', function () {
-  assert.strictEqual(closes, N);
-});