cluster: fixing debug port logic for forking workers

lib/internal/cluster/master.js

@@ -12,6 +12,16 @@ const cluster = new EventEmitter();
 const intercom = new EventEmitter();
 const SCHED_NONE = 1;
 const SCHED_RR = 2;
+let debugPortOffset = 0;
+
+const debugArgRegex = /^(--inspect(?:-brk|-port)?|--debug-port)(?:=(.*))?$/;
+
+const hasDebugArg = process.execArgv.some((argv) => debugArgRegex.test(argv));
+
+if (hasDebugArg) {
+  // Increase debugPortOffset only if master was started with debug.
+  debugPortOffset = 1;
+}
 
 module.exports = cluster;
 
@@ -24,7 +34,6 @@ cluster.SCHED_NONE = SCHED_NONE;  // Leave it to the operating system.
 cluster.SCHED_RR = SCHED_RR;      // Master distributes connections.
 
 var ids = 0;
-var debugPortOffset = 1;
 var initialized = false;
 
 // XXX(bnoordhuis) Fold cluster.schedulingPolicy into cluster.settings?
@@ -70,6 +79,15 @@ cluster.setupMaster = function(options) {
   assert(schedulingPolicy === SCHED_NONE || schedulingPolicy === SCHED_RR,
          `Bad cluster.schedulingPolicy: ${schedulingPolicy}`);
 
+  const hasDebugArg = process.execArgv.some((argv) => {
+    return debugArgRegex.test(argv);
+  });
+
+  if (hasDebugArg) {
+    // Increase debugPortOffset only if master was started with debug
+    debugPortOffset = 1;
+  }
+
   process.nextTick(setupSettingsNT, settings);
 
   process.on('internalMessage', (message) => {
@@ -96,25 +114,73 @@ function setupSettingsNT(settings) {
 }
 
 function createWorkerProcess(id, env) {
-  var workerEnv = util._extend({}, process.env);
-  var execArgv = cluster.settings.execArgv.slice();
-  var debugPort = 0;
+  // If master process started with debug, workers should use ports with offset
+  // This code parses execArgv from cluster.settings
+  // and augments values of ports in following flags:
+  // --inspect, --inspect-brk, --inspect-port, --debug-port.
+
+  const workerEnv = util._extend({}, process.env);
+  const execArgv = cluster.settings.execArgv.slice();
+  const ipv6Regex = /^(\[[\d:]+\])(?::(.+))?$/;
 
   util._extend(workerEnv, env);
   workerEnv.NODE_UNIQUE_ID = '' + id;
 
-  for (var i = 0; i < execArgv.length; i++) {
-    const match = execArgv[i].match(
-      /^(--inspect|--inspect-(brk|port)|--debug|--debug-(brk|port))(=\d+)?$/
-    );
+  // Cycle is reversed on purpose:
+  // node currently selects debug port from last argument
+  // so we're changing last argument and breaking the cycle.
+  let i = execArgv.length;
+  while (i--) {
+    if (debugArgRegex.test(execArgv[i])) {
+      let debugPort = 0;
+      let debugHost = '';
+
+      const splitDebugArg = execArgv[i].split('=');
+
+      let resultArg = splitDebugArg[0] + '=';
+      const debugSocket = splitDebugArg[1];
+
+      if (debugSocket !== undefined) {
+        const ipv6Match = debugSocket.match(ipv6Regex);
+
+        if (ipv6Match) {
+          debugHost = ipv6Match[1];
+
+          if (ipv6Match[2]) {
+            debugPort = +ipv6Match[2];
+          }
+        } else {
+          const splitDebugSocket = debugSocket.split(':');
+
+          if (splitDebugSocket[1]) {
+            debugHost = splitDebugSocket[0];
+            debugPort = +splitDebugSocket[1];
+          } else {
+            if (/^\d+$/.test(debugSocket)) {
+              debugPort = +debugSocket;
+            } else {
+              debugHost = debugSocket;
+            }
+          }
+        }
+      }
 
-    if (match) {
       if (debugPort === 0) {
-        debugPort = process.debugPort + debugPortOffset;
-        ++debugPortOffset;
+        debugPort = process.debugPort;
       }
 
-      execArgv[i] = match[1] + '=' + debugPort;
+      debugPort += debugPortOffset;
+      ++debugPortOffset;
+
+      if (debugHost) {
+        resultArg += debugHost + ':';
+      }
+
+      resultArg += debugPort;
+
+      execArgv[i] = resultArg;
+
+      break;
     }
   }
 

test/sequential/test-cluster-inspect-port-manual.js

@@ -0,0 +1,56 @@
+'use strict';
+
+/**
+ * This test suite checks debug port numbers
+ * when node started without --inspect argument
+ * and debug initiated with cluster.settings.execArgv or similar
+ */
+
+const common = require('../common');
+const assert = require('assert');
+const cluster = require('cluster');
+
+if (cluster.isMaster) {
+  let message = 'If master is not debugging, ' +
+                'forked worker should not have --inspect argument';
+
+  cluster.fork({message}).on('exit', common.mustCall(checkExitCode));
+
+  message = 'When debugging port is set at cluster.settings, ' +
+            'forked worker should have debugPort without offset';
+  cluster.settings.execArgv = ['--inspect=' + common.PORT];
+  cluster.fork({
+    portSet: common.PORT,
+    message
+  }).on('exit', common.mustCall(checkExitCode));
+
+  message = 'When using custom port number, ' +
+            'forked worker should have debugPort incremented from that port';
+  cluster.fork({
+    portSet: common.PORT + 1,
+    message
+  }).on('exit', common.mustCall(checkExitCode));
+
+  cluster.fork({
+    portSet: common.PORT + 2,
+    message
+  }).on('exit', common.mustCall(checkExitCode));
+} else {
+  const hasDebugArg = process.execArgv.some(function(arg) {
+    return /--inspect/.test(arg);
+  });
+
+  assert.strictEqual(hasDebugArg, process.env.portSet !== undefined);
+  hasDebugArg && assert.strictEqual(
+    process.debugPort,
+    +process.env.portSet,
+    process.env.message +
+    `; expected port: ${+process.env.portSet}, actual: ${process.debugPort}`
+  );
+  process.exit();
+}
+
+function checkExitCode(code, signal) {
+  assert.strictEqual(code, 0);
+  assert.strictEqual(signal, null);
+}

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

@@ -0,0 +1,141 @@
+'use strict';
+// Flags: --inspect={PORT}
+const common = require('../common');
+
+common.skipIfInspectorDisabled();
+
+const assert = require('assert');
+const cluster = require('cluster');
+const debuggerPort = common.PORT;
+
+let offset = 0;
+
+function checkExitCode(code, signal) {
+  assert.strictEqual(code, 0);
+  assert.strictEqual(signal, null);
+}
+
+if (cluster.isMaster) {
+  assert.strictEqual(process.debugPort, debuggerPort);
+
+  let message = 'If master is debugging with default port, ' +
+                'forked worker should have debugPort, with offset = 1';
+
+  cluster.fork({
+    portSet: process.debugPort + (++offset),
+    message
+  }).on('exit', common.mustCall(checkExitCode));
+
+  message = 'Debug port offset should increment';
+  cluster.fork({
+    portSet: process.debugPort + (++offset),
+    message
+  }).on('exit', common.mustCall(checkExitCode));
+
+  message = 'If master is debugging on non-default port, ' +
+            'worker ports should be incremented from it';
+  cluster.setupMaster({
+    execArgv: ['--inspect=' + common.PORT]
+  });
+  cluster.fork({
+    portSet: common.PORT + (++offset),
+    message
+  }).on('exit', common.mustCall(checkExitCode));
+
+  message = '--inspect=hostname:port argument format ' +
+            'should be parsed correctly';
+  cluster.setupMaster({
+    execArgv: ['--inspect=localhost:' + (common.PORT + 10)]
+  });
+  cluster.fork({
+    portSet: common.PORT + 10 + (++offset),
+    message
+  }).on('exit', common.mustCall(checkExitCode));
+
+  message = '--inspect=ipv4addr:port argument format ' +
+            'should be parsed correctly';
+  cluster.setupMaster({
+    execArgv: ['--inspect=' + common.localhostIPv4 + ':' + (common.PORT + 20)]
+  });
+  cluster.fork({
+    portSet: common.PORT + 20 + (++offset),
+    message
+  }).on('exit', common.mustCall(checkExitCode));
+
+  message = '--inspect=[ipv6addr]:port argument format ' +
+            'should be parsed correctly';
+  if (common.hasIPv6) {
+    cluster.setupMaster({
+      execArgv: ['--inspect=[::1]:' + (common.PORT + 30)]
+    });
+    cluster.fork({
+      portSet: common.PORT + 30 + (++offset),
+      message
+    }).on('exit', common.mustCall(checkExitCode));
+  }
+
+  message = '--inspect=hostname:port argument format ' +
+            'should be parsed correctly';
+  cluster.setupMaster({
+    execArgv: ['--inspect=localhost'],
+  });
+  cluster.fork({
+    portSet: process.debugPort + (++offset),
+    message
+  }).on('exit', common.mustCall(checkExitCode));
+
+  message = '--inspect=ipv4addr argument format ' +
+            'should be parsed correctly';
+  cluster.setupMaster({
+    execArgv: ['--inspect=' + common.localhostIPv4]
+  });
+  cluster.fork({
+    portSet: process.debugPort + (++offset),
+    message
+  }).on('exit', common.mustCall(checkExitCode));
+
+  message = '--inspect=[ipv6addr] argument format ' +
+            'should be parsed correctly';
+  if (common.hasIPv6) {
+    cluster.setupMaster({
+      execArgv: ['--inspect=[::1]']
+    });
+    cluster.fork({
+      portSet: process.debugPort + (++offset),
+      message
+    }).on('exit', common.mustCall(checkExitCode));
+  }
+
+  message = '--inspect --debug-port={PORT} argument format ' +
+            'should be parsed correctly';
+  cluster.setupMaster({
+    execArgv: ['--inspect', '--debug-port=' + debuggerPort]
+  });
+  cluster.fork({
+    portSet: process.debugPort + (++offset),
+    message
+  }).on('exit', common.mustCall(checkExitCode));
+
+  message = '--inspect --inspect-port={PORT} argument format ' +
+            'should be parsed correctly';
+  cluster.setupMaster({
+    execArgv: ['--inspect', '--inspect-port=' + debuggerPort]
+  });
+  cluster.fork({
+    portSet: process.debugPort + (++offset),
+    message
+  }).on('exit', common.mustCall(checkExitCode));
+
+} else {
+  const hasDebugArg = process.execArgv.some(function(arg) {
+    return /inspect/.test(arg);
+  });
+
+  assert.strictEqual(hasDebugArg, true);
+  assert.strictEqual(
+    process.debugPort,
+    +process.env.portSet,
+    process.env.message
+  );
+  process.exit();
+}