node: enforce maxConnections correctly

src/js/node/net.ts

@@ -89,7 +89,6 @@ function isIP(s): 0 | 4 | 6 {
 
 const bunTlsSymbol = Symbol.for("::buntls::");
 const bunSocketServerConnections = Symbol.for("::bunnetserverconnections::");
-const bunSocketServerDropped = Symbol.for("::bunnetserverdropped::");
 const bunSocketServerOptions = Symbol.for("::bunnetserveroptions::");
 const owner_symbol = Symbol("owner_symbol");
 
@@ -343,17 +342,16 @@ const ServerHandlers: SocketHandler = {
     const data = this.data;
     if (!data) return;
 
-    if (!data[bunSocketServerDropped]) {
-      data.server[bunSocketServerConnections]--;
-    }
-
-    if (!data[kclosed]) {
-      data[kclosed] = true;
-      //socket cannot be used after close
-      detachSocket(data);
-      SocketEmitEndNT(data, err);
-      data.data = null;
-      socket[owner_symbol] = null;
+    data.server[bunSocketServerConnections]--;
+    {
+      if (!data[kclosed]) {
+        data[kclosed] = true;
+        //socket cannot be used after close
+        detachSocket(data);
+        SocketEmitEndNT(data, err);
+        data.data = null;
+        socket[owner_symbol] = null;
+      }
     }
 
     data.server._emitCloseIfDrained();
@@ -385,13 +383,12 @@ const ServerHandlers: SocketHandler = {
           remotePort: _socket.remotePort,
           remoteFamily: _socket.remoteFamily || "IPv4",
         };
-        _socket[bunSocketServerDropped] = true;
         socket.end();
         self.emit("drop", data);
         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,
@@ -401,7 +398,6 @@ const ServerHandlers: SocketHandler = {
         remoteFamily: _socket.remoteFamily || "IPv4",
       };
 
-      _socket[bunSocketServerDropped] = true;
       socket.end();
 
       self.emit("drop", data);

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-close-makes-more-available.js

@@ -1,84 +0,0 @@
-require("../common");
-const assert = require("assert");
-
-const net = require("net");
-
-// Sets the server's maxConnections property to 1.
-// Open 2 connections (connection 0 and connection 1).
-// Connection 0  should be accepted.
-// Connection 1 should be rejected.
-// Closes connection 0.
-// Open 2 more connections (connection 2 and 3).
-// Connection 2 should be accepted.
-// Connection 3 should be rejected.
-
-const connections = [];
-const received = [];
-const sent = [];
-
-function createConnection(index) {
-  console.error(`creating connection ${index}`);
-
-  return new Promise(function (resolve, reject) {
-    const connection = net.createConnection(server.address().port, function () {
-      const msg = String(index);
-      console.error(`sending message: ${msg}`);
-      this.write(msg);
-      sent.push(msg);
-    });
-
-    connection.on("error", function (err) {
-      assert.strictEqual(err.code, "ECONNRESET");
-      resolve();
-    });
-
-    connection.on("data", function (e) {
-      console.error(`connection ${index} received response`);
-      resolve();
-    });
-
-    connection.on("end", function () {
-      console.error(`ending ${index}`);
-      resolve();
-    });
-
-    connections[index] = connection;
-  });
-}
-
-function closeConnection(index) {
-  console.error(`closing connection ${index}`);
-  return new Promise(function (resolve, reject) {
-    connections[index].on("end", function () {
-      resolve();
-    });
-    connections[index].end();
-  });
-}
-
-const server = net.createServer(function (socket) {
-  socket.on("data", function (data) {
-    console.error(`received message: ${data}`);
-    received.push(String(data));
-    socket.write("acknowledged");
-  });
-});
-
-server.maxConnections = 1;
-
-server.listen(0, function () {
-  createConnection(0)
-    .then(createConnection.bind(null, 1))
-    .then(closeConnection.bind(null, 0))
-    .then(createConnection.bind(null, 2))
-    .then(createConnection.bind(null, 3))
-    .then(server.close.bind(server))
-    .then(closeConnection.bind(null, 2));
-});
-
-process.on("exit", function () {
-  // Confirm that all connections tried to send data...
-  assert.deepStrictEqual(sent, ["0", "1", "2", "3"]);
-  // ...but that only connections 0 and 2 were successful.
-  assert.deepStrictEqual(received, ["0", "2"]);
-});