inspector: bind to random port with --inspect=0

bnoordhuis · bnoordhuis · commit 399cb25f6266 · 2017-05-30T12:22:02.000+02:00
Allow binding to a randomly assigned port number with `--inspect=0` or `--inspect-brk=0`. PR-URL: #5025 Refs: #4419 Reviewed-By: Colin Ihrig <cjihrig@gmail.com> Reviewed-By: Refael Ackermann <refack@gmail.com> Reviewed-By: Sam Roberts <vieuxtech@gmail.com>
diff --git a/src/inspector_io.cc b/src/inspector_io.cc
@@ -175,7 +175,7 @@ InspectorIo::InspectorIo(Environment* env, v8::Platform* platform,
                            io_thread_req_(), platform_(platform),
                            dispatching_messages_(false), session_id_(0),
                            script_name_(path),
-                           wait_for_connect_(wait_for_connect) {
+                           wait_for_connect_(wait_for_connect), port_(-1) {
   main_thread_req_ = new AsyncAndAgent({uv_async_t(), env->inspector_agent()});
   CHECK_EQ(0, uv_async_init(env->event_loop(), &main_thread_req_->first,
                             InspectorIo::MainThreadAsyncCb));
@@ -298,6 +298,7 @@ void InspectorIo::WorkerRunIO() {
     uv_sem_post(&start_sem_);
     return;
   }
+  port_ = server.port();  // Safe, main thread is waiting on semaphore.
   if (!wait_for_connect_) {
     uv_sem_post(&start_sem_);
   }
diff --git a/src/inspector_io.h b/src/inspector_io.h
@@ -68,6 +68,8 @@ class InspectorIo {
     uv_close(reinterpret_cast<uv_handle_t*>(&io_thread_req_), nullptr);
   }
 
+  int port() const { return port_; }
+
  private:
   template <typename Action>
   using MessageQueue =
@@ -129,6 +131,7 @@ class InspectorIo {
   std::string script_path_;
   const std::string id_;
   const bool wait_for_connect_;
+  int port_;
 
   friend class DispatchMessagesTask;
   friend class IoSessionDelegate;
diff --git a/src/node.cc b/src/node.cc
@@ -40,6 +40,10 @@
 #include "node_i18n.h"
 #endif
 
+#if HAVE_INSPECTOR
+#include "inspector_io.h"
+#endif
+
 #if defined HAVE_DTRACE || defined HAVE_ETW
 #include "node_dtrace.h"
 #endif
@@ -3048,7 +3052,15 @@ static Local<Object> GetFeatures(Environment* env) {
 
 static void DebugPortGetter(Local<Name> property,
                             const PropertyCallbackInfo<Value>& info) {
-  info.GetReturnValue().Set(debug_options.port());
+  int port = debug_options.port();
+#if HAVE_INSPECTOR
+  if (port == 0) {
+    Environment* env = Environment::GetCurrent(info);
+    if (env->inspector_agent()->IsStarted())
+      port = env->inspector_agent()->io()->port();
+  }
+#endif  // HAVE_INSPECTOR
+  info.GetReturnValue().Set(port);
 }
 
 
diff --git a/src/node_debug_options.cc b/src/node_debug_options.cc
@@ -21,8 +21,9 @@ int parse_and_validate_port(const std::string& port) {
   char* endptr;
   errno = 0;
   const long result = strtol(port.c_str(), &endptr, 10);  // NOLINT(runtime/int)
-  if (errno != 0 || *endptr != '\0'|| result < 1024 || result > 65535) {
-    fprintf(stderr, "Debug port must be in range 1024 to 65535.\n");
+  if (errno != 0 || *endptr != '\0'||
+      (result != 0 && result < 1024) || result > 65535) {
+    fprintf(stderr, "Debug port must be 0 or in range 1024 to 65535.\n");
     exit(12);
   }
   return static_cast<int>(result);
diff --git a/test/inspector/test-inspector-port-zero-cluster.js b/test/inspector/test-inspector-port-zero-cluster.js
@@ -0,0 +1,32 @@
+// Flags: --inspect=0
+'use strict';
+
+const common = require('../common');
+const assert = require('assert');
+const cluster = require('cluster');
+
+if (cluster.isMaster) {
+  const ports = [];
+  for (const worker of [cluster.fork(),
+                        cluster.fork(),
+                        cluster.fork()]) {
+    worker.on('message', common.mustCall((message) => {
+      ports.push(message.debugPort);
+      worker.kill();
+    }));
+    worker.send('debugPort');
+  }
+  process.on('exit', () => {
+    ports.sort();
+    assert.strictEqual(ports.length, 3);
+    assert(ports.every((port) => port > 0));
+    assert(ports.every((port) => port < 65536));
+    assert.strictEqual(ports[0] + 1, ports[1]);  // Ports should be consecutive.
+    assert.strictEqual(ports[1] + 1, ports[2]);
+  });
+} else {
+  process.on('message', (message) => {
+    if (message === 'debugPort')
+      process.send({ debugPort: process.debugPort });
+  });
+}
diff --git a/test/inspector/test-inspector-port-zero.js b/test/inspector/test-inspector-port-zero.js
@@ -0,0 +1,46 @@
+'use strict';
+
+const { mustCall } = require('../common');
+const assert = require('assert');
+const { URL } = require('url');
+const { spawn } = require('child_process');
+
+function test(arg) {
+  const args = [arg, '-p', 'process.debugPort'];
+  const proc = spawn(process.execPath, args);
+  proc.stdout.setEncoding('utf8');
+  proc.stderr.setEncoding('utf8');
+  let stdout = '';
+  let stderr = '';
+  proc.stdout.on('data', (data) => stdout += data);
+  proc.stderr.on('data', (data) => stderr += data);
+  proc.stdout.on('close', assert.ifError);
+  proc.stderr.on('close', assert.ifError);
+  let port = '';
+  proc.stderr.on('data', () => {
+    if (!stderr.includes('\n')) return;
+    assert(/Debugger listening on (.+)/.test(stderr));
+    port = new URL(RegExp.$1).port;
+    assert(+port > 0);
+  });
+  if (/inspect-brk/.test(arg)) {
+    proc.stderr.on('data', () => {
+      if (stderr.includes('\n') && !proc.killed) proc.kill();
+    });
+  } else {
+    let onclose = () => {
+      onclose = () => assert.strictEqual(port, stdout.trim());
+    };
+    proc.stdout.on('close', mustCall(() => onclose()));
+    proc.stderr.on('close', mustCall(() => onclose()));
+    proc.on('exit', mustCall((exitCode) => assert.strictEqual(exitCode, 0)));
+  }
+}
+
+test('--inspect=0');
+test('--inspect=127.0.0.1:0');
+test('--inspect=localhost:0');
+
+test('--inspect-brk=0');
+test('--inspect-brk=127.0.0.1:0');
+test('--inspect-brk=localhost:0');
