[PATCH] Fix potential hang during gdbserver testing

[PATCH] Fix potential hang during gdbserver testing

[Kevin Buettner]

We're currently seeing testing of native-extended-gdbserver hang while
testing the x86_64 architecture on both Fedora 34 and Fedora Rawhide.
The test responsible for the hang is gdb.threads/fork-plus-threads.exp.

While there is clearly a problem/bug with this test on F34 and
Rawhide, it's also the case that testing should not hang.  This commit
prevents the hang by waiting with the "-nowait" flag in
close_gdbserver.

The -nowait flag is also used in the kill_wait_spawned_process proc in
gdb/testsuite/lib/gdb.exp, so there is precedent for doing this.

There are also 15 other uses of "wait -i" scattered throughout the
test suite.  While it's tempting to change these to also use the
-nowait flag, I think it might be safer to defer doing so until we
actually see a problem.

I've tested this patch on Fedora 32, 33, 34, and Rawhide.  Results are
comparable on Fedora 32 and 33.  On Fedora 34 and Rawhide, with this
commit in place, testing completes when the target_board is
native-extended-gdbserver.  On those OSes, when not using this commit,
testing usually hangs due to a problem with
gdb.threads/fork-plus-threads.exp.  I've also tested on all of the
mentioned OSes with target_board=native-gdbserver; for that testing,
I acheived comparable results over a number of runs.  (Unfortunately
results are rarely identical due to racy tests.)

gdb/testsuite/ChangeLog:

	* lib/gdbserver-support.exp (gdbserver_exit): Use the
	"-nowait" flag when waiting for gdbserver to exit.
---
 gdb/testsuite/lib/gdbserver-support.exp | 6 +++++-
 1 file changed, 5 insertions(+), 1 deletion(-)

diff --git a/gdb/testsuite/lib/gdbserver-support.exp b/gdb/testsuite/lib/gdbserver-support.exp
index 7b48b5a8399..acd611c23a6 100644
--- a/gdb/testsuite/lib/gdbserver-support.exp
+++ b/gdb/testsuite/lib/gdbserver-support.exp
@@ -421,7 +421,11 @@ proc close_gdbserver {} {
     verbose "Quitting GDBserver"
 
     catch "close -i $server_spawn_id"
-    catch "wait -i $server_spawn_id"
+
+    # If gdbserver misbehaves, and ignores the close, waiting for it
+    # without the -nowait flag will cause testing to hang. Passing -nowait
+    # makes expect tell Tcl to wait for the PID in the background.
+    catch "wait -nowait -i $server_spawn_id"
     unset server_spawn_id
 }
 
-- 
2.29.2

Re: [PATCH] Fix potential hang during gdbserver testing

[Pedro Alves]

On 19/03/21 18:45, Kevin Buettner via Gdb-patches wrote:

> I acheived comparable results over a number of runs.  (Unfortunately

typo "acheived" -> "achieved"

> results are rarely identical due to racy tests.)
> 
> gdb/testsuite/ChangeLog:
> 
> 	* lib/gdbserver-support.exp (gdbserver_exit): Use the
> 	"-nowait" flag when waiting for gdbserver to exit.

This is OK, thanks!

> +    # If gdbserver misbehaves, and ignores the close, waiting for it
> +    # without the -nowait flag will cause testing to hang. Passing -nowait

Double space after period.

> +    # makes expect tell Tcl to wait for the PID in the background.
> +    catch "wait -nowait -i $server_spawn_id"
>      unset server_spawn_id
>  }
>  
> 

Re: [PATCH] Fix potential hang during gdbserver testing

[Kevin Buettner]

On Fri, 19 Mar 2021 20:16:01 +0000
Pedro Alves <pedro@palves.net> wrote:

> On 19/03/21 18:45, Kevin Buettner via Gdb-patches wrote:
> 
> > I acheived comparable results over a number of runs.  (Unfortunately  
> 
> typo "acheived" -> "achieved"
> 
> > results are rarely identical due to racy tests.)
> > 
> > gdb/testsuite/ChangeLog:
> > 
> > 	* lib/gdbserver-support.exp (gdbserver_exit): Use the
> > 	"-nowait" flag when waiting for gdbserver to exit.  
> 
> This is OK, thanks!
> 
> > +    # If gdbserver misbehaves, and ignores the close, waiting for it
> > +    # without the -nowait flag will cause testing to hang. Passing -nowait  
> 
> Double space after period.
> 
> > +    # makes expect tell Tcl to wait for the PID in the background.
> > +    catch "wait -nowait -i $server_spawn_id"
> >      unset server_spawn_id
> >  }

Pushed with the above corrections.

Thanks for the review!

Kevin