Skip to content

Commit 2777a7e

Browse files
mutantcornholiorefack
mutantcornholio
authored and
refack
committed
inspector,cluster: fix inspect port assignment
* Adding --inspect-port with debug port, instead of parsing `execArgv` * Export CLI debug options to `process.binding('config').debugOptions` (currently used only in tests) PR-URL: nodejs#13619 Refs: nodejs#9659 Reviewed-By: Refael Ackermann <refack@gmail.com> Reviewed-By: James M Snell <jasnell@gmail.com> Reviewed-By: Sam Roberts <vieuxtech@gmail.com> Reviewed-By: Matteo Collina <matteo.collina@gmail.com> Reviewed-By: Colin Ihrig <cjihrig@gmail.com>
1 parent f462ad1 commit 2777a7e

6 files changed

Lines changed: 166 additions & 58 deletions

File tree

‎lib/internal/cluster/master.js‎

Lines changed: 6 additions & 16 deletions
Original file line numberDiff line numberDiff line change
@@ -96,26 +96,16 @@ function setupSettingsNT(settings) {
9696
}
9797

9898
function createWorkerProcess(id, env) {
99-
var workerEnv = util._extend({}, process.env);
100-
var execArgv = cluster.settings.execArgv.slice();
101-
var debugPort = 0;
102-
var debugArgvRE =
103-
/^(--inspect|--inspect-(brk|port)|--debug|--debug-(brk|port))(=\d+)?$/;
99+
const workerEnv = util._extend({}, process.env);
100+
const execArgv = cluster.settings.execArgv.slice();
101+
const debugArgRegex = /--inspect(?:-brk|-port)?|--debug-port/;
104102

105103
util._extend(workerEnv, env);
106104
workerEnv.NODE_UNIQUE_ID = '' + id;
107105

108-
for (var i = 0; i < execArgv.length; i++) {
109-
const match = execArgv[i].match(debugArgvRE);
110-
111-
if (match) {
112-
if (debugPort === 0) {
113-
debugPort = process.debugPort + debugPortOffset;
114-
++debugPortOffset;
115-
}
116-
117-
execArgv[i] = match[1] + '=' + debugPort;
118-
}
106+
if (execArgv.some((arg) => arg.match(debugArgRegex))) {
107+
execArgv.push(`--inspect-port=${process.debugPort + debugPortOffset}`);
108+
debugPortOffset++;
119109
}
120110

121111
return fork(cluster.settings.exec, cluster.settings.args, {

‎src/node.cc‎

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -242,7 +242,7 @@ static double prog_start_time;
242242
static Mutex node_isolate_mutex;
243243
static v8::Isolate* node_isolate;
244244

245-
static node::DebugOptions debug_options;
245+
node::DebugOptions debug_options;
246246

247247
static struct {
248248
#if NODE_USE_V8_PLATFORM

‎src/node_config.cc‎

Lines changed: 24 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -4,11 +4,14 @@
44
#include "env-inl.h"
55
#include "util.h"
66
#include "util-inl.h"
7+
#include "node_debug_options.h"
78

89

910
namespace node {
1011

12+
using v8::Boolean;
1113
using v8::Context;
14+
using v8::Integer;
1215
using v8::Local;
1316
using v8::Object;
1417
using v8::ReadOnly;
@@ -62,6 +65,27 @@ static void InitConfig(Local<Object> target,
6265
target->DefineOwnProperty(env->context(), name, value).FromJust();
6366
}
6467

68+
Local<Object> debugOptions = Object::New(env->isolate());
69+
70+
target->DefineOwnProperty(env->context(),
71+
OneByteString(env->isolate(), "debugOptions"),
72+
debugOptions).FromJust();
73+
74+
debugOptions->DefineOwnProperty(env->context(),
75+
OneByteString(env->isolate(), "host"),
76+
String::NewFromUtf8(env->isolate(),
77+
debug_options.host_name().c_str())).FromJust();
78+
79+
debugOptions->DefineOwnProperty(env->context(),
80+
OneByteString(env->isolate(), "port"),
81+
Integer::New(env->isolate(),
82+
debug_options.port())).FromJust();
83+
84+
debugOptions->DefineOwnProperty(env->context(),
85+
OneByteString(env->isolate(), "inspectorEnabled"),
86+
Boolean::New(env->isolate(),
87+
debug_options.inspector_enabled())).FromJust();
88+
6589
if (config_expose_internals)
6690
READONLY_BOOLEAN_PROPERTY("exposeInternals");
6791
} // InitConfig

‎src/node_internals.h‎

Lines changed: 6 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -30,6 +30,7 @@
3030
#include "uv.h"
3131
#include "v8.h"
3232
#include "tracing/trace_event.h"
33+
#include "node_debug_options.h"
3334

3435
#include <stdint.h>
3536
#include <stdlib.h>
@@ -100,6 +101,11 @@ extern bool config_pending_deprecation;
100101
// Tells whether it is safe to call v8::Isolate::GetCurrent().
101102
extern bool v8_initialized;
102103

104+
// Contains initial debug options.
105+
// Set in node.cc.
106+
// Used in node_config.cc.
107+
extern node::DebugOptions debug_options;
108+
103109
// Forward declaration
104110
class Environment;
105111

‎test/inspector/test-inspector-port-cluster.js‎

Lines changed: 129 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,129 @@
1+
'use strict';
2+
3+
const common = require('../common');
4+
5+
common.skipIfInspectorDisabled();
6+
7+
const assert = require('assert');
8+
const cluster = require('cluster');
9+
10+
const debuggerPort = common.PORT;
11+
const childProcess = require('child_process');
12+
13+
let offset = 0;
14+
15+
/*
16+
* This test suite checks that inspector port in cluster is incremented
17+
* for different execArgv combinations
18+
*/
19+
20+
function testRunnerMain() {
21+
spawnMaster({
22+
execArgv: ['--inspect'],
23+
workers: [{expectedPort: 9230}]
24+
});
25+
26+
let port = debuggerPort + offset++ * 10;
27+
28+
spawnMaster({
29+
execArgv: [`--inspect=${port}`],
30+
workers: [
31+
{expectedPort: port + 1},
32+
{expectedPort: port + 2},
33+
{expectedPort: port + 3}
34+
]
35+
});
36+
37+
port = debuggerPort + offset++ * 10;
38+
39+
spawnMaster({
40+
execArgv: ['--inspect', `--inspect-port=${port}`],
41+
workers: [{expectedPort: port + 1}]
42+
});
43+
44+
port = debuggerPort + offset++ * 10;
45+
46+
spawnMaster({
47+
execArgv: ['--inspect', `--debug-port=${port}`],
48+
workers: [{expectedPort: port + 1}]
49+
});
50+
51+
port = debuggerPort + offset++ * 10;
52+
53+
spawnMaster({
54+
execArgv: [`--inspect=0.0.0.0:${port}`],
55+
workers: [{expectedPort: port + 1, expectedHost: '0.0.0.0'}]
56+
});
57+
58+
port = debuggerPort + offset++ * 10;
59+
60+
spawnMaster({
61+
execArgv: [`--inspect=127.0.0.1:${port}`],
62+
workers: [{expectedPort: port + 1, expectedHost: '127.0.0.1'}]
63+
});
64+
65+
if (common.hasIPv6) {
66+
port = debuggerPort + offset++ * 10;
67+
68+
spawnMaster({
69+
execArgv: [`--inspect=[::]:${port}`],
70+
workers: [{expectedPort: port + 1, expectedHost: '::'}]
71+
});
72+
73+
port = debuggerPort + offset++ * 10;
74+
75+
spawnMaster({
76+
execArgv: [`--inspect=[::1]:${port}`],
77+
workers: [{expectedPort: port + 1, expectedHost: '::1'}]
78+
});
79+
}
80+
}
81+
82+
function masterProcessMain() {
83+
const workers = JSON.parse(process.env.workers);
84+
85+
for (const worker of workers) {
86+
cluster.fork({
87+
expectedPort: worker.expectedPort,
88+
expectedHost: worker.expectedHost
89+
}).on('exit', common.mustCall(checkExitCode));
90+
}
91+
}
92+
93+
function workerProcessMain() {
94+
const {expectedPort, expectedHost} = process.env;
95+
96+
assert.strictEqual(process.debugPort, +expectedPort);
97+
98+
if (expectedHost !== 'undefined') {
99+
assert.strictEqual(
100+
process.binding('config').debugOptions.host,
101+
expectedHost
102+
);
103+
}
104+
105+
process.exit();
106+
}
107+
108+
function spawnMaster({execArgv, workers}) {
109+
childProcess.fork(__filename, {
110+
env: {
111+
workers: JSON.stringify(workers),
112+
testProcess: true
113+
},
114+
execArgv
115+
}).on('exit', common.mustCall(checkExitCode));
116+
}
117+
118+
function checkExitCode(code, signal) {
119+
assert.strictEqual(code, 0);
120+
assert.strictEqual(signal, null);
121+
}
122+
123+
if (!process.env.testProcess) {
124+
testRunnerMain();
125+
} else if (cluster.isMaster) {
126+
masterProcessMain();
127+
} else {
128+
workerProcessMain();
129+
}

‎test/parallel/test-cluster-inspector-debug-port.js‎

Lines changed: 0 additions & 41 deletions
This file was deleted.

0 commit comments

Comments
 (0)