testsuite: Don't use expect_background to reap gdbserver

I adjusted a test to do 'expect -i $server_spawn_id -re ...', and saw
really strange behavior.  Whether that expect would work, depended on
whether GDB would also send output and the same expect matched it too
(on $gdb_spawn_id).  I was perplexed until I noticed that
gdbserver_spawn spawns gdbserver and then uses expect_background to
reap gdbserver.  That expect_background conflicts/races with any
"expect -i $server_spawn_id" done anywhere else in parallel...

In order to make it possible for tests to read inferior I/O out of
$server_spawn_id, we to get rid of that expect_background.  This patch
makes us instead reap gdbserver's spawn id when GDB exits.  If GDB is
still around, this gives a chance for gdbserver to exit cleanly.  The
current code in gdb_finish uses "kill", but that doesn't work with
extended-remote (gdbserver doesn't exit).  We now use "monitor exit"
instead which works in both remote and extended-remote modes.

gdb/testsuite/ChangeLog:
2015-04-07  Pedro Alves  <palves@redhat.com>

	* lib/gdb.exp (gdb_finish): Delete persistent gdbserver handling.
	* lib/gdbserver-support.exp (gdbserver_start): Make
	$server_spawn_id global.
	(gdbserver_start): Don't wait for gdbserver's spawn id with
	expect_background.
	(close_gdbserver): New procedure.
	(gdb_exit): Rename the default version and reimplement.

1 files changed, 48 insertions, 13 deletions

diff --git a/gdb/testsuite/lib/gdbserver-support.exp b/gdb/testsuite/lib/gdbserver-support.exp
index 4a59154..f19b796 100644
--- a/gdb/testsuite/lib/gdbserver-support.exp
+++ b/gdb/testsuite/lib/gdbserver-support.exp
@@ -270,6 +270,7 @@ proc gdbserver_start { options arguments } {
 	    append gdbserver_command " $arguments"
 	}
 
+	global server_spawn_id
 	set server_spawn_id [remote_spawn target $gdbserver_command]
 
 	# Wait for the server to open its TCP socket, so that GDB can connect.
@@ -294,19 +295,6 @@ proc gdbserver_start { options arguments } {
 	break
     }
 
-    # We can't just call close, because if gdbserver is local then that means
-    # that it will get a SIGHUP.  Doing it this way could also allow us to
-    # get at the inferior's input or output if necessary, and means that we
-    # don't need to redirect output.
-    expect_background {
-	-i $server_spawn_id
-	full_buffer { }
-	eof {
-	    # The spawn ID is already closed now (but not yet waited for).
-	    wait -i $expect_out(spawn_id)
-	}
-    }
-
     return [list $protocol [$get_remote_address $debughost $portnum]]
 }
 
@@ -328,6 +316,53 @@ proc gdbserver_spawn { child_args } {
     return [gdbserver_start "" $arguments]
 }
 
+# Close the GDBserver connection.
+
+proc close_gdbserver {} {
+    global server_spawn_id
+
+    # We can't just call close, because if gdbserver is local then that means
+    # that it will get a SIGHUP.  Doing it this way could also allow us to
+    # get at the inferior's input or output if necessary, and means that we
+    # don't need to redirect output.
+
+    if {![info exists server_spawn_id]} {
+	return
+    }
+
+    verbose "Quitting GDBserver"
+
+    catch "close -i $server_spawn_id"
+    catch "wait -i $server_spawn_id"
+    unset server_spawn_id
+}
+
+# Hook into GDB exit, and close GDBserver.
+
+if { [info procs gdbserver_gdb_exit] == "" } {
+    rename gdb_exit gdbserver_orig_gdb_exit
+}
+proc gdb_exit {} {
+    global gdb_spawn_id server_spawn_id
+    global gdb_prompt
+
+    if {[info exists gdb_spawn_id] && [info exists server_spawn_id]} {
+	send_gdb "monitor exit\n";
+	gdb_expect {
+	    -re "$gdb_prompt $" {
+		exp_continue
+	    }
+	    -i "$server_spawn_id" eof {
+		wait -i $expect_out(spawn_id)
+		unset server_spawn_id
+	    }
+	}
+    }
+    close_gdbserver
+
+    gdbserver_orig_gdb_exit
+}
+
 # Start a gdbserver process running HOST_EXEC and pass CHILD_ARGS
 # to it.  Return 0 on success, or non-zero on failure: 2 if gdbserver
 # failed to start or 1 if we couldn't connect to it.