Fixed debugger_test to wait until target process is dead

tests/standalone/debugger/debug_lib.dart

Index: tests/standalone/debugger/debug_lib.dart
diff --git a/tests/standalone/debugger/debug_lib.dart b/tests/standalone/debugger/debug_lib.dart
index ff51f9ba5c2ae5bfedb7324459bb7af176d7b503..6658fa4fab36dafe36152004e7494a9b8bd92fa9 100644
--- a/tests/standalone/debugger/debug_lib.dart
+++ b/tests/standalone/debugger/debug_lib.dart
@@ -488,16 +488,30 @@ class Debugger {
   }
 
   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);
   }
 }
 
@@ -508,7 +522,7 @@ bool RunScript(List script) {
     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);
@@ -536,11 +550,11 @@ bool RunScript(List 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);