From: Pedro Alves <palves@redhat.com>
Date: Tue, 7 Apr 2015 17:19:30 +0000 (+0100)
Subject: testsuite: Don't use expect_background to reap gdbserver
X-Git-Tag: gdb-7.10-release~903
X-Git-Url: http://review.tizen.org/git/?a=commitdiff_plain;h=6423214fcb912a5d402ffcb84d6f61f2b900a900;p=external%2Fbinutils.git

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 694b1c8..eb084d2 100644
--- a/gdb/testsuite/ChangeLog
+++ b/gdb/testsuite/ChangeLog
@@ -1,5 +1,15 @@
 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,
 	append the substituted item, not the original item.
 
diff --git a/gdb/testsuite/lib/gdb.exp b/gdb/testsuite/lib/gdb.exp
index 203b8cf..1669f45 100644
--- 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 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.