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: #13619
Refs: #9659
Reviewed-By: Refael Ackermann <[email protected]>
Reviewed-By: James M Snell <[email protected]>
Reviewed-By: Sam Roberts <[email protected]>
Reviewed-By: Matteo Collina <[email protected]>
Reviewed-By: Colin Ihrig <[email protected]>

Author: mutantcornholio

lib/internal/cluster/master.js

@@ -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, {

src/node.cc

@@ -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

src/node_config.cc

@@ -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;
@@ -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

src/node_internals.h

@@ -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>
@@ -83,6 +84,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;
 

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

@@ -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();
+}