From bfd7fc06c7fe87bf2c8097c6ad6fc5a18def3ddb Mon Sep 17 00:00:00 2001 From: Meghan Denny Date: Tue, 3 Jun 2025 21:50:08 -0800 Subject: [PATCH] fix test-net-server-max-connections.js (#20034) Co-authored-by: Jarred Sumner Co-authored-by: nektro <5464072+nektro@users.noreply.github.com> Co-authored-by: Ciro Spaciari --- src/js/node/net.ts | 38 ++++--- src/js/private.d.ts | 7 ++ ...-connections-close-makes-more-available.js | 85 ++++++++++++++ .../test-net-server-max-connections.js | 107 ++++++++++++++++++ 4 files changed, 221 insertions(+), 16 deletions(-) create mode 100644 test/js/node/test/parallel/test-net-server-max-connections-close-makes-more-available.js create mode 100644 test/js/node/test/parallel/test-net-server-max-connections.js diff --git a/src/js/node/net.ts b/src/js/node/net.ts index 749af39f41..0f599f0d35 100644 --- a/src/js/node/net.ts +++ b/src/js/node/net.ts @@ -28,7 +28,8 @@ let dns: typeof import("node:dns"); const normalizedArgsSymbol = Symbol("normalizedArgs"); const { ExceptionWithHostPort } = require("internal/shared"); import type { Socket, SocketHandler, SocketListener } from "bun"; -import type { ServerOpts } from "node:net"; +import type { Server as NetServer, Socket as NetSocket, ServerOpts } from "node:net"; +import type { TLSSocket } from "node:tls"; const { kTimeout, getTimerDuration } = require("internal/timers"); const { validateFunction, validateNumber, validateAbortSignal, validatePort, validateBoolean, validateInt32 } = require("internal/validators"); // prettier-ignore const { NodeAggregateError, ErrnoException } = require("internal/shared"); @@ -325,7 +326,7 @@ const SocketEmitEndNT = (self, _err?) => { // } }; -const ServerHandlers: SocketHandler = { +const ServerHandlers: SocketHandler = { data(socket, buffer) { const { data: self } = socket; if (!self) return; @@ -336,10 +337,10 @@ const ServerHandlers: SocketHandler = { } }, close(socket, err) { + $debug("Bun.Server close"); const data = this.data; if (!data) return; - data.server._connections--; { if (!data[kclosed]) { data[kclosed] = true; @@ -350,20 +351,18 @@ const ServerHandlers: SocketHandler = { socket[owner_symbol] = null; } } - - data.server._emitCloseIfDrained(); }, end(socket) { SocketHandlers.end(socket); }, open(socket) { - const self = this.data; + $debug("Bun.Server open"); + const self = socket.data as any as NetServer; socket[kServerSocket] = self._handle; const options = self[bunSocketServerOptions]; const { pauseOnConnect, connectionListener, [kSocketClass]: SClass, requestCert, rejectUnauthorized } = options; - const _socket = new SClass({}); + const _socket = new SClass({}) as NetSocket | TLSSocket; _socket.isServer = true; - _socket.server = self; _socket._requestCert = requestCert; _socket._rejectUnauthorized = rejectUnauthorized; @@ -396,7 +395,6 @@ const ServerHandlers: SocketHandler = { }; socket.end(); - self.emit("drop", data); return; } @@ -405,6 +403,7 @@ const ServerHandlers: SocketHandler = { const isTLS = typeof bunTLS === "function"; self._connections++; + _socket.server = self; if (pauseOnConnect) { _socket.pause(); @@ -423,7 +422,7 @@ const ServerHandlers: SocketHandler = { } }, handshake(socket, success, verifyError) { - const { data: self } = socket; + const self = socket.data; if (!success && verifyError?.code === "ECONNRESET") { const err = new ConnResetException("socket hang up"); self.emit("_tlsError", err); @@ -437,7 +436,7 @@ const ServerHandlers: SocketHandler = { self.secureConnecting = false; self._secureEstablished = !!success; self.servername = socket.getServername(); - const server = self.server; + const server = self.server!; self.alpnProtocol = socket.alpnProtocol; if (self._requestCert || self._rejectUnauthorized) { if (verifyError) { @@ -510,9 +509,8 @@ const ServerHandlers: SocketHandler = { binaryType: "buffer", } as const; -type SocketHandleData = NonNullable["data"]; // TODO: SocketHandlers2 is a bad name but its temporary. reworking the Server in a followup PR -const SocketHandlers2: SocketHandler = { +const SocketHandlers2: SocketHandler["data"]> = { open(socket) { $debug("Bun.Socket open"); let { self, req } = socket.data; @@ -1165,6 +1163,14 @@ Socket.prototype._destroy = function _destroy(err, callback) { callback(err); process.nextTick(emitCloseNT, this, false); } + + if (this.server) { + $debug("has server"); + this.server._connections--; + if (this.server._emitCloseIfDrained) { + this.server._emitCloseIfDrained(); + } + } }; Socket.prototype._final = function _final(callback) { @@ -2378,6 +2384,7 @@ Server.prototype[kRealListen] = function ( ipv6Only: ipv6Only || this[bunSocketServerOptions]?.ipv6Only || false, exclusive: exclusive || this[bunSocketServerOptions]?.exclusive || false, socket: ServerHandlers, + data: this, }); } else if (fd != null) { this._handle = Bun.listen({ @@ -2389,6 +2396,7 @@ Server.prototype[kRealListen] = function ( ipv6Only: ipv6Only || this[bunSocketServerOptions]?.ipv6Only || false, exclusive: exclusive || this[bunSocketServerOptions]?.exclusive || false, socket: ServerHandlers, + data: this, }); } else { this._handle = Bun.listen({ @@ -2400,12 +2408,10 @@ Server.prototype[kRealListen] = function ( ipv6Only: ipv6Only || this[bunSocketServerOptions]?.ipv6Only || false, exclusive: exclusive || this[bunSocketServerOptions]?.exclusive || false, socket: ServerHandlers, + data: this, }); } - //make this instance available on handlers - this._handle.data = this; - const addr = this.address(); if (addr && typeof addr === "object") { const familyLast = String(addr.family).slice(-1); diff --git a/src/js/private.d.ts b/src/js/private.d.ts index 5efb3388c9..9159a0afa0 100644 --- a/src/js/private.d.ts +++ b/src/js/private.d.ts @@ -224,10 +224,17 @@ declare function $newZigFunction any>( declare function $bindgenFn any>(filename: string, symbol: string): T; // NOTE: $debug, $assert, and $isPromiseFulfilled omitted +import "node:net"; declare module "node:net" { export function _normalizeArgs(args: any[]): unknown[]; interface Socket { _handle: Bun.Socket<{ self: Socket; req?: object }> | null; + server: Server | null; + } + + interface Server { + _handle: Bun.SocketListener | null; + _connections: number; } } diff --git a/test/js/node/test/parallel/test-net-server-max-connections-close-makes-more-available.js b/test/js/node/test/parallel/test-net-server-max-connections-close-makes-more-available.js new file mode 100644 index 0000000000..f607f28c10 --- /dev/null +++ b/test/js/node/test/parallel/test-net-server-max-connections-close-makes-more-available.js @@ -0,0 +1,85 @@ +'use strict'; +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']); +}); diff --git a/test/js/node/test/parallel/test-net-server-max-connections.js b/test/js/node/test/parallel/test-net-server-max-connections.js new file mode 100644 index 0000000000..f0cbea1bc5 --- /dev/null +++ b/test/js/node/test/parallel/test-net-server-max-connections.js @@ -0,0 +1,107 @@ +// 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. + +'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); +});