Re: [PATCH v2 01/47] Make default_gdb_exit resilient to failed closes

[Andrew Burgess <aburgess@redhat.com>]

Pedro Alves <pedro@palves.net> writes:

> For some reason, when testing GDB on Cygwin, I get:
>
>  child process exited abnormally
>      while executing
>  "exec sh -c "exec > /dev/null 2>&1 && (kill -2 -$spid || kill -2 $spid)""
>      (procedure "close_wait_program" line 20)
>      invoked from within
>  "close_wait_program $shell_id $pid"
>      (procedure "standard_close" line 23)
>      invoked from within
>  "standard_close "Windows-ROCm""
>      ("eval" body line 1)
>      invoked from within
>  "eval ${try}_${proc} \"$dest\" $args"
>      (procedure "call_remote" line 42)
>      invoked from within
>  "call_remote "" close $host"
>      (procedure "remote_close" line 3)
>      invoked from within
>  "remote_close host"
>      (procedure "log_and_exit" line 30)
>      invoked from within
>  "log_and_exit"
>
> When that happens from within clean_restart, clean_restart doesn't
> clear the gdb_spawn_id variable, and then when clean_restart starts up
> a new GDB, that sees that gdb_spawn_id is already set, so it doesn't
> actually spawn a new GDB, and so clean_restart happens to reuse the
> same GDB (!).  Many tests happen to actually work OK with this, but
> some don't, and the failure modes can be head-scratching.
>
> Of course, the failure to close GDB should be fixed, but when it
> happens, I think it's good to not end up with the current weird state.
> Connecting the "child process exit abnormally" errors at the end of a
> testcase run with weird FAILs in other testcases took me a while (as
> in, weeks!), it wasn't obvious to me immediately.
>
> Thus, this patch makes default_gdb_exit more resilient to failed
> closes, so that gdb_spawn_id is unset even is closing GDB fails, and
> we move on to start a new GDB.
>
> Change-Id: I9ec95aa61872a40095775534743525e0ad2097d2

Makes sense to me.

Approved-By: Andrew Burgess <aburgess@redhat.com>

Thanks,
Andrew


> ---
>  gdb/testsuite/lib/gdb.exp | 4 +++-
>  1 file changed, 3 insertions(+), 1 deletion(-)
>
> diff --git a/gdb/testsuite/lib/gdb.exp b/gdb/testsuite/lib/gdb.exp
> index 49ef0d54654..c51cea86a9d 100644
> --- a/gdb/testsuite/lib/gdb.exp
> +++ b/gdb/testsuite/lib/gdb.exp
> @@ -2315,7 +2315,9 @@ proc default_gdb_exit {} {
>      }
>  
>      if ![is_remote host] {
> -	remote_close host
> +	if {[catch { remote_close host } message]} {
> +	    warning "closing gdb failed with: $message"
> +	}
>      }
>      unset gdb_spawn_id
>      unset ::gdb_tty_name
> -- 
> 2.49.0