From 8e3f606779e38883d9022b67d93d10ad5cdc55a4 Mon Sep 17 00:00:00 2001 From: Ouyang Yadong Date: Mon, 4 Jan 2021 13:55:42 +0800 Subject: [PATCH] cluster: fix edge cases that throw ERR_INTERNAL_ASSERTION Some cases use both `cluster` and `net`/`cluser` will throw ERR_INTERNAL_ASSERTION when `listen`/`bind` to the port of `0`. This PR maitains a separate map of the index to fix the issue. See the new tests added for the detail cases. PR-URL: https://github.com/nodejs/node/pull/36764 Reviewed-By: Antoine du Hamel --- lib/internal/cluster/child.js | 39 ++++++++++++------ .../test-cluster-child-index-dgram.js | 40 +++++++++++++++++++ test/parallel/test-cluster-child-index-net.js | 31 ++++++++++++++ 3 files changed, 97 insertions(+), 13 deletions(-) create mode 100644 test/parallel/test-cluster-child-index-dgram.js create mode 100644 test/parallel/test-cluster-child-index-net.js diff --git a/lib/internal/cluster/child.js b/lib/internal/cluster/child.js index 5c884cc695077b..a6035d7a75b013 100644 --- a/lib/internal/cluster/child.js +++ b/lib/internal/cluster/child.js @@ -6,6 +6,7 @@ const { ObjectAssign, ReflectApply, SafeMap, + SafeSet, } = primordials; const assert = require('internal/assert'); @@ -74,14 +75,14 @@ cluster._getServer = function(obj, options, cb) { options.fd, ], ':'); - let index = indexes.get(indexesKey); + let indexSet = indexes.get(indexesKey); - if (index === undefined) - index = 0; - else - index++; - - indexes.set(indexesKey, index); + if (indexSet === undefined) { + indexSet = { nextIndex: 0, set: new SafeSet() }; + indexes.set(indexesKey, indexSet); + } + const index = indexSet.nextIndex++; + indexSet.set.add(index); const message = { act: 'queryServer', @@ -101,9 +102,9 @@ cluster._getServer = function(obj, options, cb) { obj._setServerData(reply.data); if (handle) - shared(reply, handle, indexesKey, cb); // Shared listen socket. + shared(reply, handle, indexesKey, index, cb); // Shared listen socket. else - rr(reply, indexesKey, cb); // Round-robin. + rr(reply, indexesKey, index, cb); // Round-robin. }); obj.once('listening', () => { @@ -115,8 +116,20 @@ cluster._getServer = function(obj, options, cb) { }); }; +function removeIndexesKey(indexesKey, index) { + const indexSet = indexes.get(indexesKey); + if (!indexSet) { + return; + } + + indexSet.set.delete(index); + if (indexSet.set.size === 0) { + indexes.delete(indexesKey); + } +} + // Shared listen socket. -function shared(message, handle, indexesKey, cb) { +function shared(message, handle, indexesKey, index, cb) { const key = message.key; // Monkey-patch the close() method so we can keep track of when it's // closed. Avoids resource leaks when the handle is short-lived. @@ -125,7 +138,7 @@ function shared(message, handle, indexesKey, cb) { handle.close = function() { send({ act: 'close', key }); handles.delete(key); - indexes.delete(indexesKey); + removeIndexesKey(indexesKey, index); return ReflectApply(close, handle, arguments); }; assert(handles.has(key) === false); @@ -134,7 +147,7 @@ function shared(message, handle, indexesKey, cb) { } // Round-robin. Primary distributes handles across workers. -function rr(message, indexesKey, cb) { +function rr(message, indexesKey, index, cb) { if (message.errno) return cb(message.errno, null); @@ -158,7 +171,7 @@ function rr(message, indexesKey, cb) { send({ act: 'close', key }); handles.delete(key); - indexes.delete(indexesKey); + removeIndexesKey(indexesKey, index); key = undefined; } diff --git a/test/parallel/test-cluster-child-index-dgram.js b/test/parallel/test-cluster-child-index-dgram.js new file mode 100644 index 00000000000000..0df7bc175b87f0 --- /dev/null +++ b/test/parallel/test-cluster-child-index-dgram.js @@ -0,0 +1,40 @@ +'use strict'; +const common = require('../common'); +const Countdown = require('../common/countdown'); +if (common.isWindows) + common.skip('dgram clustering is currently not supported on Windows.'); + +const cluster = require('cluster'); +const dgram = require('dgram'); + +// Test an edge case when using `cluster` and `dgram.Socket.bind()` +// the port of `0`. +const kPort = 0; + +function child() { + const kTime = 2; + const countdown = new Countdown(kTime * 2, () => { + process.exit(0); + }); + for (let i = 0; i < kTime; i += 1) { + const socket = new dgram.Socket('udp4'); + socket.bind(kPort, common.mustCall(() => { + // `process.nextTick()` or `socket2.close()` would throw + // ERR_SOCKET_DGRAM_NOT_RUNNING + process.nextTick(() => { + socket.close(countdown.dec()); + const socket2 = new dgram.Socket('udp4'); + socket2.bind(kPort, common.mustCall(() => { + process.nextTick(() => { + socket2.close(countdown.dec()); + }); + })); + }); + })); + } +} + +if (cluster.isMaster) + cluster.fork(__filename); +else + child(); diff --git a/test/parallel/test-cluster-child-index-net.js b/test/parallel/test-cluster-child-index-net.js new file mode 100644 index 00000000000000..d8c3166d1bae3c --- /dev/null +++ b/test/parallel/test-cluster-child-index-net.js @@ -0,0 +1,31 @@ +'use strict'; +const common = require('../common'); +const Countdown = require('../common/countdown'); +const cluster = require('cluster'); +const net = require('net'); + +// Test an edge case when using `cluster` and `net.Server.listen()` to +// the port of `0`. +const kPort = 0; + +function child() { + const kTime = 2; + const countdown = new Countdown(kTime * 2, () => { + process.exit(0); + }); + for (let i = 0; i < kTime; i += 1) { + const server = net.createServer(); + server.listen(kPort, common.mustCall(() => { + server.close(countdown.dec()); + const server2 = net.createServer(); + server2.listen(kPort, common.mustCall(() => { + server2.close(countdown.dec()); + })); + })); + } +} + +if (cluster.isMaster) + cluster.fork(__filename); +else + child();