Skip to content

Commit

Permalink
inspector: bind to random port with --inspect=0
Browse files Browse the repository at this point in the history
Allow binding to a randomly assigned port number with `--inspect=0`
or `--inspect-brk=0`.

PR-URL: nodejs#5025
Refs: nodejs#4419
Reviewed-By: Colin Ihrig <[email protected]>
Reviewed-By: Refael Ackermann <[email protected]>
Reviewed-By: Sam Roberts <[email protected]>
  • Loading branch information
bnoordhuis committed May 30, 2017
1 parent c420cd1 commit 399cb25
Show file tree
Hide file tree
Showing 6 changed files with 99 additions and 4 deletions.
3 changes: 2 additions & 1 deletion src/inspector_io.cc
Original file line number Diff line number Diff line change
Expand Up @@ -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));
Expand Down Expand Up @@ -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_);
}
Expand Down
3 changes: 3 additions & 0 deletions src/inspector_io.h
Original file line number Diff line number Diff line change
Expand Up @@ -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 =
Expand Down Expand Up @@ -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;
Expand Down
14 changes: 13 additions & 1 deletion src/node.cc
Original file line number Diff line number Diff line change
Expand Up @@ -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
Expand Down Expand Up @@ -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);
}


Expand Down
5 changes: 3 additions & 2 deletions src/node_debug_options.cc
Original file line number Diff line number Diff line change
Expand Up @@ -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);
Expand Down
32 changes: 32 additions & 0 deletions test/inspector/test-inspector-port-zero-cluster.js
Original file line number Diff line number Diff line change
@@ -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 });
});
}
46 changes: 46 additions & 0 deletions test/inspector/test-inspector-port-zero.js
Original file line number Diff line number Diff line change
@@ -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');

0 comments on commit 399cb25

Please sign in to comment.