Skip to content

Commit

Permalink
debugger: don't spawn child process in remote mode
Browse files Browse the repository at this point in the history
When debug in remote mode with host:port or pid, the interface
spawn child process also. If the debugger agent is running, will
get following output:

```
< Error: listen EADDRINUSE :::5858
<     at Object.exports._errnoException (util.js:734:11)
<     at exports._exceptionWithHostPort (util.js:757:20)
<     at Agent.Server._listen2 (net.js:1155:14)
<     at listen (net.js:1181:10)
<     at Agent.Server.listen (net.js:1268:5)
<     at Object.start (_debug_agent.js:21:9)
<     at startup (node.js:68:9)
<     at node.js:799:3
```

This fix won't spawn child process and no more error message was
shown.

When use `iojs debug`, the tip information just like this:

```
Usage: iojs debug script.js
```

This fix will display the advance usage also:

```
Usage: iojs debug script.js
       iojs debug <host>:<port>
       iojs debug -p <pid>
```

Fixes: #889
PR-URL: #1282
Reviewed-By: Ben Noordhuis <info@bnoordhuis.nl>
  • Loading branch information
JacksonTian authored and bnoordhuis committed Mar 27, 2015
1 parent 955c150 commit a2ea168
Show file tree
Hide file tree
Showing 2 changed files with 73 additions and 20 deletions.
41 changes: 21 additions & 20 deletions lib/_debugger.js
Original file line number Diff line number Diff line change
Expand Up @@ -15,6 +15,8 @@ exports.start = function(argv, stdin, stdout) {

if (argv.length < 1) {
console.error('Usage: iojs debug script.js');
console.error(' iojs debug <host>:<port>');
console.error(' iojs debug -p <pid>');
process.exit(1);
}

Expand Down Expand Up @@ -1626,6 +1628,7 @@ Interface.prototype.trySpawn = function(cb) {
this.killChild();
assert(!this.child);

var isRemote = false;
if (this.args.length === 2) {
var match = this.args[1].match(/^([^:]+):(\d+)$/);

Expand All @@ -1634,21 +1637,13 @@ Interface.prototype.trySpawn = function(cb) {
// `node debug localhost:5858`
host = match[1];
port = parseInt(match[2], 10);
this.child = {
kill: function() {
// TODO Do we really need to handle it?
}
};
isRemote = true;
}
} else if (this.args.length === 3) {
// `node debug -p pid`
if (this.args[1] === '-p' && /^\d+$/.test(this.args[2])) {
this.child = {
kill: function() {
// TODO Do we really need to handle it?
}
};
process._debugProcess(parseInt(this.args[2], 10));
isRemote = true;
} else {
var match = this.args[1].match(/^--port=(\d+)$/);
if (match) {
Expand All @@ -1660,10 +1655,13 @@ Interface.prototype.trySpawn = function(cb) {
}
}

this.child = spawn(process.execPath, childArgs);
if (!isRemote) {
// pipe stream into debugger
this.child = spawn(process.execPath, childArgs);

this.child.stdout.on('data', this.childPrint.bind(this));
this.child.stderr.on('data', this.childPrint.bind(this));
this.child.stdout.on('data', this.childPrint.bind(this));
this.child.stderr.on('data', this.childPrint.bind(this));
}

this.pause();

Expand Down Expand Up @@ -1707,9 +1705,10 @@ Interface.prototype.trySpawn = function(cb) {

client.on('error', connectError);
function connectError() {
// If it's failed to connect 4 times then don't catch the next error
// If it's failed to connect 10 times then print failed message
if (connectionAttempts >= 10) {
client.removeListener('error', connectError);
self.stdout.write(' failed, please retry\n');
return;
}
setTimeout(attemptConnect, 500);
}
Expand All @@ -1720,10 +1719,12 @@ Interface.prototype.trySpawn = function(cb) {
client.connect(port, host);
}

this.child.stderr.once('data', function() {
setImmediate(function() {
self.print('connecting to port ' + port + '..', true);
attemptConnect();
self.print('connecting to ' + host + ':' + port + ' ..', true);
if (isRemote) {
attemptConnect();
} else {
this.child.stderr.once('data', function() {
setImmediate(attemptConnect);
});
});
}
};
52 changes: 52 additions & 0 deletions test/debugger/test-debugger-remote.js
Original file line number Diff line number Diff line change
@@ -0,0 +1,52 @@
var common = require('../common');
var assert = require('assert');
var spawn = require('child_process').spawn;

var port = common.PORT + 1337;
var buffer = '';
var expected = [];
var scriptToDebug = common.fixturesDir + '/empty.js';

function fail() {
assert(0); // `iojs --debug-brk script.js` should not quit
}

// running with debug agent
var child = spawn(process.execPath, ['--debug-brk=5959', scriptToDebug]);

console.error(process.execPath, '--debug-brk=5959', scriptToDebug);

// connect to debug agent
var interfacer = spawn(process.execPath, ['debug', 'localhost:5959']);

console.error(process.execPath, 'debug', 'localhost:5959');
interfacer.stdout.setEncoding('utf-8');
interfacer.stdout.on('data', function(data) {
data = (buffer + data).split('\n');
buffer = data.pop();
data.forEach(function(line) {
interfacer.emit('line', line);
});
});

interfacer.on('line', function(line) {
line = line.replace(/^(debug> *)+/, '');
console.log(line);
var expected = '\bconnecting to localhost:5959 ... ok';
assert.ok(expected == line, 'Got unexpected line: ' + line);
});

// give iojs time to start up the debugger
setTimeout(function() {
child.removeListener('exit', fail);
child.kill();
interfacer.removeListener('exit', fail);
interfacer.kill();
}, 2000);

process.on('exit', function() {
assert(child.killed);
assert(interfacer.killed);
});

interfacer.stderr.pipe(process.stderr);

0 comments on commit a2ea168

Please sign in to comment.