Skip to content

Navigation Menu

Sign in
Appearance settings

Search code, repositories, users, issues, pull requests...

Provide feedback

We read every piece of feedback, and take your input very seriously.

Saved searches

Use saved searches to filter your results more quickly
Appearance settings

Commit 54bd4ab

Browse filesBrowse files
oyyddanielleadams
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
Copy full SHA for 54bd4ab

File tree

Expand file treeCollapse file tree

3 files changed

+98
-14
lines changed
Open diff view settings
Filter options
Expand file treeCollapse file tree

3 files changed

+98
-14
lines changed
Open diff view settings
Collapse file

‎lib/internal/cluster/child.js‎

Copy file name to clipboardExpand all lines: lib/internal/cluster/child.js
+27-14Lines 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

Collapse file

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

Copy file name to clipboard
+40Lines 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();
Collapse file

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

Copy file name to clipboard
+31Lines 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 (0)
Morty Proxy This is a proxified and sanitized view of the page, visit original site.