From a6e227f7ec54d20422a7b4d7dee6b923aee65af3 Mon Sep 17 00:00:00 2001 From: Elad Nava Date: Thu, 20 Jan 2022 22:24:30 -0500 Subject: [PATCH 1/3] cluster: respect server.listen() backlog set by workers Co-authored-by: @oyyd --- doc/api/net.md | 4 ++ lib/internal/cluster/round_robin_handle.js | 7 ++-- lib/net.js | 1 + .../test-cluster-net-listen-backlog.js | 40 +++++++++++++++++++ 4 files changed, 49 insertions(+), 3 deletions(-) create mode 100644 test/parallel/test-cluster-net-listen-backlog.js diff --git a/doc/api/net.md b/doc/api/net.md index f8bcb8dfc9db2e..ffe18c233e4da4 100644 --- a/doc/api/net.md +++ b/doc/api/net.md @@ -464,6 +464,10 @@ server.listen({ }); ``` +When `exclusive` is `true` and the underlying handle is shared, it's +possible that several workers query a handle with different backlogs. +In this case, the first `backlog` passed to the master process will be used. + Starting an IPC server as root may cause the server path to be inaccessible for unprivileged users. Using `readableAll` and `writableAll` will make the server accessible for all users. diff --git a/lib/internal/cluster/round_robin_handle.js b/lib/internal/cluster/round_robin_handle.js index 2bf008939e7573..9d242cc60ad7c1 100644 --- a/lib/internal/cluster/round_robin_handle.js +++ b/lib/internal/cluster/round_robin_handle.js @@ -15,7 +15,7 @@ const { constants } = internalBinding('tcp_wrap'); module.exports = RoundRobinHandle; -function RoundRobinHandle(key, address, { port, fd, flags }) { +function RoundRobinHandle(key, address, { port, fd, flags, backlog }) { this.key = key; this.all = new SafeMap(); this.free = new SafeMap(); @@ -24,16 +24,17 @@ function RoundRobinHandle(key, address, { port, fd, flags }) { this.server = net.createServer(assert.fail); if (fd >= 0) - this.server.listen({ fd }); + this.server.listen({ fd, backlog }); else if (port >= 0) { this.server.listen({ port, host: address, // Currently, net module only supports `ipv6Only` option in `flags`. ipv6Only: Boolean(flags & constants.UV_TCP_IPV6ONLY), + backlog, }); } else - this.server.listen(address); // UNIX socket path. + this.server.listen(address, backlog); // UNIX socket path. this.server.once('listening', () => { this.handle = this.server._handle; diff --git a/lib/net.js b/lib/net.js index 3bbe96f1e04af0..2fa4a1ece188e3 100644 --- a/lib/net.js +++ b/lib/net.js @@ -1385,6 +1385,7 @@ function listenInCluster(server, address, port, addressType, addressType: addressType, fd: fd, flags, + backlog, }; // Get the primary's server handle, and listen on it diff --git a/test/parallel/test-cluster-net-listen-backlog.js b/test/parallel/test-cluster-net-listen-backlog.js new file mode 100644 index 00000000000000..eab734f3b6fe4f --- /dev/null +++ b/test/parallel/test-cluster-net-listen-backlog.js @@ -0,0 +1,40 @@ +'use strict'; + +const common = require('../common'); +const assert = require('assert'); +// Monkey-patch `net.Server.listen` +const net = require('net'); +const cluster = require('cluster'); + +// Ensures that the `backlog` is used to create a `net.Server`. +const kExpectedBacklog = 127; +if (cluster.isMaster) { + const listen = net.Server.prototype.listen; + + net.Server.prototype.listen = common.mustCall( + function(...args) { + const options = args[0]; + if (typeof options === 'object') { + assert(options.backlog, kExpectedBacklog); + } else { + assert(args[1], kExpectedBacklog); + } + return listen.call(this, ...args); + } + ); + + const worker = cluster.fork(); + worker.on('message', () => { + worker.disconnect(); + }); +} else { + const server = net.createServer(); + + server.listen({ + host: common.localhostIPv4, + port: 0, + backlog: kExpectedBacklog, + }, common.mustCall(() => { + process.send(true); + })); +} From ccded6357914109d8c51d76675db381d11e2c8b5 Mon Sep 17 00:00:00 2001 From: Elad Nava Date: Fri, 21 Jan 2022 12:39:46 -0500 Subject: [PATCH 2/3] cluster: respect server.listen() backlog set by workers (fix test) --- test/parallel/test-cluster-net-listen-backlog.js | 5 +++++ 1 file changed, 5 insertions(+) diff --git a/test/parallel/test-cluster-net-listen-backlog.js b/test/parallel/test-cluster-net-listen-backlog.js index eab734f3b6fe4f..090552fd1e1eeb 100644 --- a/test/parallel/test-cluster-net-listen-backlog.js +++ b/test/parallel/test-cluster-net-listen-backlog.js @@ -6,6 +6,11 @@ const assert = require('assert'); const net = require('net'); const cluster = require('cluster'); +// Force round-robin scheduling policy +// as Windows defaults to SCHED_NONE +// https://nodejs.org/docs/latest/api/cluster.html#clusterschedulingpolicy +cluster.schedulingPolicy = cluster.SCHED_RR; + // Ensures that the `backlog` is used to create a `net.Server`. const kExpectedBacklog = 127; if (cluster.isMaster) { From 7e083a3c388e75668bd9fd5f57debf3be97994a8 Mon Sep 17 00:00:00 2001 From: Elad Nava Date: Wed, 26 Jan 2022 12:37:29 -0500 Subject: [PATCH 3/3] cluster: respect server.listen() backlog parameter set by workers (fix typo) MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Co-authored-by: Tobias Nießen --- doc/api/net.md | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/doc/api/net.md b/doc/api/net.md index ffe18c233e4da4..48f19a74fd7086 100644 --- a/doc/api/net.md +++ b/doc/api/net.md @@ -464,7 +464,7 @@ server.listen({ }); ``` -When `exclusive` is `true` and the underlying handle is shared, it's +When `exclusive` is `true` and the underlying handle is shared, it is possible that several workers query a handle with different backlogs. In this case, the first `backlog` passed to the master process will be used.