Fixed debugger_test to wait until target process is dead

tests/standalone/debugger/debug_lib.dart

 // Copyright (c) 2013, the Dart project authors.  Please see the AUTHORS file
 // for details. All rights reserved. Use of this source code is governed by a
 // BSD-style license that can be found in the LICENSE file.
 
 // Library used by debugger wire protocol tests (standalone VM debugging).
 
 library DartDebugger;
 
 import "dart:async";
 import "dart:io";
@@ skipping 470 matching lines @@
       onError: (e) {
         String msg = "Error while connecting to debugee: $e";
         var trace = getAttachedStackTrace(e);
         if (trace != null) msg += "\nStackTrace: $trace";
         error(msg);
         close(killDebugee: true);
       });
   }
 
   void close({killDebugee: false}) {
+    void exit() {
+      if (errorsDetected) throw "Errors detected";
+      exit(errors.length);
+    }
     if (errorsDetected) {
       for (int i = 0; i < errors.length; i++) print(errors[i]);
     }
-    if (socket != null) socket.close();
+    if (socket != null) {
+      socket.close().catchError((error) {
+        print("Error occured while closing socket: $error");
+      };
+    }
     if (killDebugee) {
-      targetProcess.kill();
-      print("Target process killed");
+      if (!targetProcess.kill()) {
+        print("Could not send kill signal to target process.");

[hausner, 2013/05/15 15:43:36]

After printing this, you still wait for the target process to exit? Could this
not lead to hangs if the target process hangs?

[ricow1, 2013/05/16 07:19:55]

From the api docs:
Returns true if the process is successfully killed (the signal is successfully
sent). Returns false if the process could not be killed (the signal could not be
sent). Usually, a false return value from kill means that the process is already
dead.

We use this extensively in infrastructure code and I have not seen any cases
where this happens and the process is not already dead. The future below will
then return immediately.

Additionally, there is not really any difference between having just the child
hang and to have this hang as well. This process will be killed by the testing
script if it hangs.

[kustermann, 2013/05/16 08:06:08]

We wait for the exitCode and if the target process is already dead, we will get
the exitCode immediately.

Maybe this example helps you understand why this works:
main() {
  var c = new Completer();
  c.complete(true);

  Timer.run(() {
    c.future.then((_) => print("done"));
  }); 
}
In this example we make a completer and complete it immediately (this is the
same as the process already exited). We schedule then a new callback (which is
inserted into the event loop). This callback registers on the future and gets
executed immediately (because the completer was already completed).

[hausner, 2013/05/16 15:38:32]

Thanks for the explanation Rico and Martin. Thinking about this some more, it
might make sense to simplify close() and always make sure the target process
exits. I will make that change later.

+      } else {
+        print("Successfully sent kill signal to target process.");
+      }
+      targetProcess.exitCode.then((exitCode) {
+        exit();
+      });
+    } else {
+      exit();
     }
-    if (errorsDetected) throw "Errors detected";
-    exit(errors.length);
   }
 }
 
 
 bool RunScript(List script) {
   var options = new Options();
   if (options.arguments.contains("--debuggee")) {
     return false;
   }
   verboseWire = options.arguments.contains("--wire");
-  
+
   // Pick a port in the upper half of the port number range.
   var seed = new DateTime.now().millisecondsSinceEpoch;
   Random random = new Random(seed);
   var debugPort = random.nextInt(32000) + 32000;
   print('using debug port $debugPort ...');
   ServerSocket.bind('127.0.0.1', debugPort).then((ServerSocket s) {
       s.close();
       var targetOpts = [ "--debug:$debugPort" ];
       // --verbose_debug is necessary so the test knows when the debuggee
       // is initialized.
       targetOpts.add("--verbose_debug");
       targetOpts.add(options.script);
       targetOpts.add("--debuggee");
       print('args: ${targetOpts.join(" ")}');
 
       Process.start(options.executable, targetOpts).then((Process process) {
         print("Debug target process started");
         process.stdin.close();
         process.exitCode.then((int exitCode) {
           if (exitCode != 0) throw "bad exit code: $exitCode";
           print("Debug target process exited with exit code $exitCode");
         });
         var debugger =
             new Debugger(process, debugPort, new DebugScript(script));
       });
     },
     onError: (e) {
-      if (++retries >= 3) { 
+      if (++retries >= 3) {
         print('unable to find unused port: $e');
         var trace = getAttachedStackTrace(e);
         if (trace != null) print("StackTrace: $trace");
-        return -1; 
+        return -1;
       } else {
         // Retry with another random port.
         RunScript(script);
       }
     });
   return true;
 }