Skip to content

Commit

Permalink
inspector,cluster: fix inspect port assignment
Browse files Browse the repository at this point in the history
* 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>
  • Loading branch information
mutantcornholio authored and refack committed Jun 16, 2017
1 parent f462ad1 commit 2777a7e
Show file tree
Hide file tree
Showing 6 changed files with 166 additions and 58 deletions.
22 changes: 6 additions & 16 deletions lib/internal/cluster/master.js
Original file line number Diff line number Diff line change
Expand Up @@ -96,26 +96,16 @@ function setupSettingsNT(settings) {
}

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

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

for (var i = 0; i < execArgv.length; i++) {
const match = execArgv[i].match(debugArgvRE);

if (match) {
if (debugPort === 0) {
debugPort = process.debugPort + debugPortOffset;
++debugPortOffset;
}

execArgv[i] = match[1] + '=' + debugPort;
}
if (execArgv.some((arg) => arg.match(debugArgRegex))) {
execArgv.push(`--inspect-port=${process.debugPort + debugPortOffset}`);
debugPortOffset++;
}

return fork(cluster.settings.exec, cluster.settings.args, {
Expand Down
2 changes: 1 addition & 1 deletion src/node.cc
Original file line number Diff line number Diff line change
Expand Up @@ -242,7 +242,7 @@ static double prog_start_time;
static Mutex node_isolate_mutex;
static v8::Isolate* node_isolate;

static node::DebugOptions debug_options;
node::DebugOptions debug_options;

static struct {
#if NODE_USE_V8_PLATFORM
Expand Down
24 changes: 24 additions & 0 deletions src/node_config.cc
Original file line number Diff line number Diff line change
Expand Up @@ -4,11 +4,14 @@
#include "env-inl.h"
#include "util.h"
#include "util-inl.h"
#include "node_debug_options.h"


namespace node {

using v8::Boolean;
using v8::Context;
using v8::Integer;
using v8::Local;
using v8::Object;
using v8::ReadOnly;
Expand Down Expand Up @@ -62,6 +65,27 @@ static void InitConfig(Local<Object> target,
target->DefineOwnProperty(env->context(), name, value).FromJust();
}

Local<Object> debugOptions = Object::New(env->isolate());

target->DefineOwnProperty(env->context(),
OneByteString(env->isolate(), "debugOptions"),
debugOptions).FromJust();

debugOptions->DefineOwnProperty(env->context(),
OneByteString(env->isolate(), "host"),
String::NewFromUtf8(env->isolate(),
debug_options.host_name().c_str())).FromJust();

debugOptions->DefineOwnProperty(env->context(),
OneByteString(env->isolate(), "port"),
Integer::New(env->isolate(),
debug_options.port())).FromJust();

debugOptions->DefineOwnProperty(env->context(),
OneByteString(env->isolate(), "inspectorEnabled"),
Boolean::New(env->isolate(),
debug_options.inspector_enabled())).FromJust();

if (config_expose_internals)
READONLY_BOOLEAN_PROPERTY("exposeInternals");
} // InitConfig
Expand Down
6 changes: 6 additions & 0 deletions src/node_internals.h
Original file line number Diff line number Diff line change
Expand Up @@ -30,6 +30,7 @@
#include "uv.h"
#include "v8.h"
#include "tracing/trace_event.h"
#include "node_debug_options.h"

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

// Contains initial debug options.
// Set in node.cc.
// Used in node_config.cc.
extern node::DebugOptions debug_options;

// Forward declaration
class Environment;

Expand Down
129 changes: 129 additions & 0 deletions test/inspector/test-inspector-port-cluster.js
Original file line number Diff line number Diff line change
@@ -0,0 +1,129 @@
'use strict';

const common = require('../common');

common.skipIfInspectorDisabled();

const assert = require('assert');
const cluster = require('cluster');

const debuggerPort = common.PORT;
const childProcess = require('child_process');

let offset = 0;

/*
* This test suite checks that inspector port in cluster is incremented
* for different execArgv combinations
*/

function testRunnerMain() {
spawnMaster({
execArgv: ['--inspect'],
workers: [{expectedPort: 9230}]
});

let port = debuggerPort + offset++ * 10;

spawnMaster({
execArgv: [`--inspect=${port}`],
workers: [
{expectedPort: port + 1},
{expectedPort: port + 2},
{expectedPort: port + 3}
]
});

port = debuggerPort + offset++ * 10;

spawnMaster({
execArgv: ['--inspect', `--inspect-port=${port}`],
workers: [{expectedPort: port + 1}]
});

port = debuggerPort + offset++ * 10;

spawnMaster({
execArgv: ['--inspect', `--debug-port=${port}`],
workers: [{expectedPort: port + 1}]
});

port = debuggerPort + offset++ * 10;

spawnMaster({
execArgv: [`--inspect=0.0.0.0:${port}`],
workers: [{expectedPort: port + 1, expectedHost: '0.0.0.0'}]
});

port = debuggerPort + offset++ * 10;

spawnMaster({
execArgv: [`--inspect=127.0.0.1:${port}`],
workers: [{expectedPort: port + 1, expectedHost: '127.0.0.1'}]
});

if (common.hasIPv6) {
port = debuggerPort + offset++ * 10;

spawnMaster({
execArgv: [`--inspect=[::]:${port}`],
workers: [{expectedPort: port + 1, expectedHost: '::'}]
});

port = debuggerPort + offset++ * 10;

spawnMaster({
execArgv: [`--inspect=[::1]:${port}`],
workers: [{expectedPort: port + 1, expectedHost: '::1'}]
});
}
}

function masterProcessMain() {
const workers = JSON.parse(process.env.workers);

for (const worker of workers) {
cluster.fork({
expectedPort: worker.expectedPort,
expectedHost: worker.expectedHost
}).on('exit', common.mustCall(checkExitCode));
}
}

function workerProcessMain() {
const {expectedPort, expectedHost} = process.env;

assert.strictEqual(process.debugPort, +expectedPort);

if (expectedHost !== 'undefined') {
assert.strictEqual(
process.binding('config').debugOptions.host,
expectedHost
);
}

process.exit();
}

function spawnMaster({execArgv, workers}) {
childProcess.fork(__filename, {
env: {
workers: JSON.stringify(workers),
testProcess: true
},
execArgv
}).on('exit', common.mustCall(checkExitCode));
}

function checkExitCode(code, signal) {
assert.strictEqual(code, 0);
assert.strictEqual(signal, null);
}

if (!process.env.testProcess) {
testRunnerMain();
} else if (cluster.isMaster) {
masterProcessMain();
} else {
workerProcessMain();
}
41 changes: 0 additions & 41 deletions test/parallel/test-cluster-inspector-debug-port.js

This file was deleted.

0 comments on commit 2777a7e

Please sign in to comment.