Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

debugger: bind to random port with --debug-port=0 #5025

Closed
wants to merge 1 commit into from
Closed
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
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 @@ -3051,7 +3055,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 @@ -23,8 +23,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');