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.

diff --git a/gdb/testsuite/ChangeLog b/gdb/testsuite/ChangeLog
index 694b1c80f30d28b81013d06ac92d52cc18b8b200..eb084d2e5ad84e43968a5ca84927e6a3f1a7255c 100644 (file)
--- a/gdb/testsuite/ChangeLog
+++ b/gdb/testsuite/ChangeLog
@@ -1,3 +1,13 @@
+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.
+
 2015-04-07  Pedro Alves  <palves@redhat.com>
 
         * lib/gdb.exp (gdb_test_multiple): When processing an argument,

diff --git a/gdb/testsuite/lib/gdb.exp b/gdb/testsuite/lib/gdb.exp
index 203b8cf0800bf00455a10c594668e1ba7ce56f48..1669f456611e161880fe808e1ff7726e35cc20ba 100644 (file)
--- a/gdb/testsuite/lib/gdb.exp
+++ b/gdb/testsuite/lib/gdb.exp
@@ -4000,20 +4000,6 @@ proc gdb_finish { } {
     global gdb_prompt
     global cleanfiles
 
-    # Give persistent gdbserver a chance to terminate before GDB is killed.
-    if {[info exists gdbserver_reconnect_p] && $gdbserver_reconnect_p
-       && [info exists gdb_spawn_id]} {
-       send_gdb "kill\n";
-       gdb_expect 10 {
-           -re "y or n" {
-               send_gdb "y\n";
-               exp_continue;
-           }
-           -re "$gdb_prompt $" {
-           }
-       }
-    }
-
     # Exit first, so that the files are no longer in use.
     gdb_exit
 

diff --git a/gdb/testsuite/lib/gdbserver-support.exp b/gdb/testsuite/lib/gdbserver-support.exp
index 4a591541cd55ac57a3f695841db43bbb54058fd7..f19b7961689ddc1820ce2c61033990876ba34190 100644 (file)
--- 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.