FazBrowse GitHub Viewer | Trending |
URL:
| Home
Tools: [Download Repo ZIP]   [Original HTTPS Page]

cluster: fix edge cases that throw ERR_INTERNAL_ASSERTION · nodejs/node@54bd4ab · GitHub

/ node Public

Commit 54bd4ab

Browse files
oyyd
authored and
danielleadams
committed
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: #36764 Reviewed-By: Antoine du Hamel <duhamelantoine1995@gmail.com>
1 parent ff5bd04 commit 54bd4ab

3 files changed

Lines changed: 98 additions & 14 deletions

File tree

‎lib/internal/cluster/child.js‎

Lines changed: 27 additions & 14 deletions
Original file line numberDiff line numberDiff line change
@@ -6,6 +6,7 @@ const {
66
ObjectAssign,
77
ReflectApply,
88
SafeMap,
9+
SafeSet,
910
} = primordials;
1011

1112
const assert = require('internal/assert');
@@ -73,14 +74,14 @@ cluster._getServer = function(obj, options, cb) {
7374
options.fd,
7475
], ':');
7576

76-
let index = indexes.get(indexesKey);
77+
let indexSet = indexes.get(indexesKey);
7778

78-
if (index === undefined)
79-
index = 0;
80-
else
81-
index++;
82-
83-
indexes.set(indexesKey, index);
79+
if (indexSet === undefined) {
80+
indexSet = { nextIndex: 0, set: new SafeSet() };
81+
indexes.set(indexesKey, indexSet);
82+
}
83+
const index = indexSet.nextIndex++;
84+
indexSet.set.add(index);
8485

8586
const message = {
8687
act: 'queryServer',
@@ -100,9 +101,9 @@ cluster._getServer = function(obj, options, cb) {
100101
obj._setServerData(reply.data);
101102

102103
if (handle)
103-
shared(reply, handle, indexesKey, cb); // Shared listen socket.
104+
shared(reply, handle, indexesKey, index, cb); // Shared listen socket.
104105
else
105-
rr(reply, indexesKey, cb); // Round-robin.
106+
rr(reply, indexesKey, index, cb); // Round-robin.
106107
});
107108

108109
obj.once('listening', () => {
@@ -114,8 +115,20 @@ cluster._getServer = function(obj, options, cb) {
114115
});
115116
};
116117

118+
function removeIndexesKey(indexesKey, index) {
119+
const indexSet = indexes.get(indexesKey);
120+
if (!indexSet) {
121+
return;
122+
}
123+
124+
indexSet.set.delete(index);
125+
if (indexSet.set.size === 0) {
126+
indexes.delete(indexesKey);
127+
}
128+
}
129+
117130
// Shared listen socket.
118-
function shared(message, handle, indexesKey, cb) {
131+
function shared(message, handle, indexesKey, index, cb) {
119132
const key = message.key;
120133
// Monkey-patch the close() method so we can keep track of when it's
121134
// closed. Avoids resource leaks when the handle is short-lived.
@@ -124,16 +137,16 @@ function shared(message, handle, indexesKey, cb) {
124137
handle.close = function() {
125138
send({ act: 'close', key });
126139
handles.delete(key);
127-
indexes.delete(indexesKey);
140+
removeIndexesKey(indexesKey, index);
128141
return ReflectApply(close, handle, arguments);
129142
};
130143
assert(handles.has(key) === false);
131144
handles.set(key, handle);
132145
cb(message.errno, handle);
133146
}
134147

135-
// Round-robin. Master distributes handles across workers.
136-
function rr(message, indexesKey, cb) {
148+
// Round-robin. Primary distributes handles across workers.
149+
function rr(message, indexesKey, index, cb) {
137150
if (message.errno)
138151
return cb(message.errno, null);
139152

@@ -157,7 +170,7 @@ function rr(message, indexesKey, cb) {
157170

158171
send({ act: 'close', key });
159172
handles.delete(key);
160-
indexes.delete(indexesKey);
173+
removeIndexesKey(indexesKey, index);
161174
key = undefined;
162175
}
163176

‎test/parallel/test-cluster-child-index-dgram.js‎

Lines changed: 40 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,40 @@
1+
'use strict';
2+
const common = require('../common');
3+
const Countdown = require('../common/countdown');
4+
if (common.isWindows)
5+
common.skip('dgram clustering is currently not supported on Windows.');
6+
7+
const cluster = require('cluster');
8+
const dgram = require('dgram');
9+
10+
// Test an edge case when using `cluster` and `dgram.Socket.bind()`
11+
// the port of `0`.
12+
const kPort = 0;
13+
14+
function child() {
15+
const kTime = 2;
16+
const countdown = new Countdown(kTime * 2, () => {
17+
process.exit(0);
18+
});
19+
for (let i = 0; i < kTime; i += 1) {
20+
const socket = new dgram.Socket('udp4');
21+
socket.bind(kPort, common.mustCall(() => {
22+
// `process.nextTick()` or `socket2.close()` would throw
23+
// ERR_SOCKET_DGRAM_NOT_RUNNING
24+
process.nextTick(() => {
25+
socket.close(countdown.dec());
26+
const socket2 = new dgram.Socket('udp4');
27+
socket2.bind(kPort, common.mustCall(() => {
28+
process.nextTick(() => {
29+
socket2.close(countdown.dec());
30+
});
31+
}));
32+
});
33+
}));
34+
}
35+
}
36+
37+
if (cluster.isMaster)
38+
cluster.fork(__filename);
39+
else
40+
child();

‎test/parallel/test-cluster-child-index-net.js‎

Lines changed: 31 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,31 @@
1+
'use strict';
2+
const common = require('../common');
3+
const Countdown = require('../common/countdown');
4+
const cluster = require('cluster');
5+
const net = require('net');
6+
7+
// Test an edge case when using `cluster` and `net.Server.listen()` to
8+
// the port of `0`.
9+
const kPort = 0;
10+
11+
function child() {
12+
const kTime = 2;
13+
const countdown = new Countdown(kTime * 2, () => {
14+
process.exit(0);
15+
});
16+
for (let i = 0; i < kTime; i += 1) {
17+
const server = net.createServer();
18+
server.listen(kPort, common.mustCall(() => {
19+
server.close(countdown.dec());
20+
const server2 = net.createServer();
21+
server2.listen(kPort, common.mustCall(() => {
22+
server2.close(countdown.dec());
23+
}));
24+
}));
25+
}
26+
}
27+
28+
if (cluster.isMaster)
29+
cluster.fork(__filename);
30+
else
31+
child();

0 commit comments

Comments
 (0)
Back | FazBrowse Home | New Git URL