From 5e339ec8e330334b09ca45ea8fa0f3ca274ff2c6 Mon Sep 17 00:00:00 2001 From: Ouyang Yadong Date: Wed, 10 Jun 2020 14:00:04 +0800 Subject: [PATCH 1/2] fix(cluster): respect the backlog from workers Currently, the master process would ignore `backlog` from worker processes and use the default value instead. This commit will respect the first `backlog` passed to the master process for a specific handle. Refs: https://github.com/nodejs/node/pull/4056 --- doc/api/net.md | 4 ++ lib/internal/cluster/round_robin_handle.js | 7 ++-- lib/net.js | 1 + .../test-cluster-net-listen-backlog.js | 42 +++++++++++++++++++ 4 files changed, 51 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 72324760f88fc5..e8e9ea6aaa7bf9 100644 --- a/doc/api/net.md +++ b/doc/api/net.md @@ -367,6 +367,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 492fd725c82f1d..c80ad6e58c27dd 100644 --- a/lib/internal/cluster/round_robin_handle.js +++ b/lib/internal/cluster/round_robin_handle.js @@ -13,7 +13,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 Map(); this.free = new Map(); @@ -22,16 +22,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 0b32646a43a43e..cebb7b0603daca 100644 --- a/lib/net.js +++ b/lib/net.js @@ -1343,6 +1343,7 @@ function listenInCluster(server, address, port, addressType, addressType: addressType, fd: fd, flags, + backlog, }; // Get the master'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..d4f84e2a0680b0 --- /dev/null +++ b/test/parallel/test-cluster-net-listen-backlog.js @@ -0,0 +1,42 @@ +'use strict'; + +const common = require('../common'); +const Countdown = require('../common/countdown'); +const assert = require('assert'); +// Monkey-patch `net.Server._listen2` +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) { + let worker + + const nativeListen = net.Server.prototype._listen2; + const countdown = new Countdown(2, () => { + worker.disconnect(); + }); + + net.Server.prototype._listen2 = common.mustCall( + function(address, port, addressType, backlog, fd, flags) { + assert(backlog, kExpectedBacklog); + countdown.dec(); + return nativeListen.call(this, address, port, addressType, backlog, fd, flags); + } + ); + + worker = cluster.fork(); + worker.on('message', () => { + countdown.dec(); + }); +} else { + const server = net.createServer() + + server.listen({ + host: common.localhostIPv4, + port: 0, + backlog: kExpectedBacklog, + }, common.mustCall(() => { + process.send(true); + })); +} From 4dcefb44bf31d6bb317cbbed53a7ea07dd692742 Mon Sep 17 00:00:00 2001 From: Ouyang Yadong Date: Tue, 18 Aug 2020 19:19:40 +0800 Subject: [PATCH 2/2] fixup! fix(cluster): respect the backlog from workers --- .../test-cluster-net-listen-backlog.js | 32 +++++++++---------- 1 file changed, 15 insertions(+), 17 deletions(-) diff --git a/test/parallel/test-cluster-net-listen-backlog.js b/test/parallel/test-cluster-net-listen-backlog.js index d4f84e2a0680b0..eab734f3b6fe4f 100644 --- a/test/parallel/test-cluster-net-listen-backlog.js +++ b/test/parallel/test-cluster-net-listen-backlog.js @@ -1,37 +1,35 @@ 'use strict'; const common = require('../common'); -const Countdown = require('../common/countdown'); const assert = require('assert'); -// Monkey-patch `net.Server._listen2` +// 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) { - let worker + const listen = net.Server.prototype.listen; - const nativeListen = net.Server.prototype._listen2; - const countdown = new Countdown(2, () => { - worker.disconnect(); - }); - - net.Server.prototype._listen2 = common.mustCall( - function(address, port, addressType, backlog, fd, flags) { - assert(backlog, kExpectedBacklog); - countdown.dec(); - return nativeListen.call(this, address, port, addressType, backlog, fd, flags); + 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); } ); - worker = cluster.fork(); + const worker = cluster.fork(); worker.on('message', () => { - countdown.dec(); + worker.disconnect(); }); } else { - const server = net.createServer() - + const server = net.createServer(); + server.listen({ host: common.localhostIPv4, port: 0,