From a2ea16838ff7e5170387f6d08c85392eda843b29 Mon Sep 17 00:00:00 2001
From: Jackson Tian <puling.tyq@alibaba-inc.com>
Date: Fri, 27 Mar 2015 10:28:24 +0800
Subject: [PATCH] debugger: don't spawn child process in remote mode

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: https://github.com/iojs/io.js/issues/889
PR-URL: https://github.com/iojs/io.js/pull/1282
Reviewed-By: Ben Noordhuis <info@bnoordhuis.nl>
---
 lib/_debugger.js                      | 41 +++++++++++++--------------
 test/debugger/test-debugger-remote.js | 52 +++++++++++++++++++++++++++++++++++
 2 files changed, 73 insertions(+), 20 deletions(-)
 create mode 100644 test/debugger/test-debugger-remote.js

diff --git a/lib/_debugger.js b/lib/_debugger.js
index 7881d72..0517ed0 100644
--- a/lib/_debugger.js
+++ b/lib/_debugger.js
@@ -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);
   }
 
@@ -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+)$/);
 
@@ -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) {
@@ -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();
 
@@ -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);
   }
@@ -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);
     });
-  });
+  }
 };
diff --git a/test/debugger/test-debugger-remote.js b/test/debugger/test-debugger-remote.js
new file mode 100644
index 0000000..641199a
--- /dev/null
+++ b/test/debugger/test-debugger-remote.js
@@ -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);
-- 
2.7.4