Re: [PATCH] Avoid gdb.base/fork-running-state.exp lingering processes (Re: [gdb/testsuite] Fix hang in fork-running-state.c)

[Tom de Vries <tdevries@suse.de>]

On 06/14/2018 02:59 PM, Pedro Alves wrote:
> On 06/13/2018 01:10 PM, Pedro Alves wrote:
> 
>> Your patch is definitely a good idea.  Please push.
>>
>> However, I think that still leaves an unnecessary delay until
>> the detached child/parent terminate.  They used to exit themselves,
>> but that caused a race, so they no longer do, see here:
>>
>>  https://sourceware.org/ml/gdb-patches/2018-03/msg00588.html
>>
>> Sounds like the best would be to restore the self-killing,
>> but make it controlled by a variable that gdb sets, depending
>> on whether gdb is staying attached to the child/parent.
> 
> Like so?
> 

I tried it out, and it works for me.

The approach looks ok to me.

I guess the alarms (180) calls are no longer necessary?

Thanks,
- Tom

> From 1d7047ff526a4bbd6e2f4336c046b3438048808f Mon Sep 17 00:00:00 2001
> From: Pedro Alves <palves@redhat.com>
> Date: Thu, 14 Jun 2018 13:00:32 +0100
> Subject: [PATCH] Avoid gdb.base/fork-running-state.exp lingering processes
> 
> Currently, the gdb.base/fork-running-state.exp testcase leaves a few
> processes lingering until a 3 minutes alarm kills them:
> 
>  pedro    28308     1  0 13:55 ?        00:00:00 /home/pedro/gdb/binutils-gdb/build/gdb/testsuite/outputs/gdb.base/fork-running-state/fork-running-state
>  pedro    28340     1  0 13:55 ?        00:00:00 /home/pedro/gdb/binutils-gdb/build/gdb/testsuite/outputs/gdb.base/fork-running-state/fork-running-state
>  pedro    28372     1  0 13:55 ?        00:00:00 /home/pedro/gdb/binutils-gdb/build/gdb/testsuite/outputs/gdb.base/fork-running-state/fork-running-state
>  pedro    28400     1  0 13:55 ?        00:00:00 /home/pedro/gdb/binutils-gdb/build/gdb/testsuite/outputs/gdb.base/fork-running-state/fork-running-state
>  pedro    28431     1  0 13:55 ?        00:00:00 /home/pedro/gdb/binutils-gdb/build/gdb/testsuite/outputs/gdb.base/fork-running-state/fork-running-state
>  pedro    28463     1  0 13:55 ?        00:00:00 /home/pedro/gdb/binutils-gdb/build/gdb/testsuite/outputs/gdb.base/fork-running-state/fork-running-state
> 
> Those processes used to kill themselves, but that was changed by
> commit f50d8a2eaea0 ("Fix gdb.base/fork-running-state.exp race").
> 
> This commit restores the self-killing, but only in the cases gdb won't
> try killing the processes, thus avoiding the old race.
> 
> (The restored code in fork_parent isn't exactly the same as it was.
> In this version, we're exiting immediately when 'wait' returns
> success, while in the old version we'd loop again and end up in the
> perror call.  The output from that perror call is not expected by the
> "kill inferior" tests, and would result in a test FAIL.)
> 
> gdb/testsuite/ChangeLog:
> 2018-06-14  Pedro Alves  <palves@redhat.com>
> 
> 	* gdb.base/fork-running-state.c: Include <errno.h>.
> 	(exit_if_relative_exits): New.
> 	(fork_child): If 'exit_if_relative_exits' is true, exit if the parent
> 	exits.
> 	(fork_parent): If 'exit_if_relative_exits' is true, exit if the
> 	child exits.
> ---
>  gdb/testsuite/gdb.base/fork-running-state.c   | 39 +++++++++++++++++++++++++--
>  gdb/testsuite/gdb.base/fork-running-state.exp |  7 +++++
>  2 files changed, 44 insertions(+), 2 deletions(-)
> 
> diff --git a/gdb/testsuite/gdb.base/fork-running-state.c b/gdb/testsuite/gdb.base/fork-running-state.c
> index 65ca942ea02..0037cb5a5e1 100644
> --- a/gdb/testsuite/gdb.base/fork-running-state.c
> +++ b/gdb/testsuite/gdb.base/fork-running-state.c
> @@ -19,9 +19,15 @@
>  #include <stdio.h>
>  #include <sys/types.h>
>  #include <sys/wait.h>
> +#include <errno.h>
>  
>  int save_parent;
>  
> +/* Variable set by GDB.  If true, then a fork child (or parent) exits
> +   if its parent (or child) exits.  Otherwise the process waits
> +   forever until either GDB or the alarm kills it.  */
> +volatile int exit_if_relative_exits = 0;
> +
>  /* The fork child.  Just runs forever.  */
>  
>  static int
> @@ -31,7 +37,20 @@ fork_child (void)
>    alarm (180);
>  
>    while (1)
> -    pause ();
> +    {
> +      if (exit_if_relative_exits)
> +	{
> +	  sleep (1);
> +
> +	  /* Exit if GDB kills the parent.  */
> +	  if (getppid () != save_parent)
> +	    break;
> +	  if (kill (getppid (), 0) != 0)
> +	    break;
> +	}
> +      else
> +	pause ();
> +    }
>  
>    return 0;
>  }
> @@ -45,7 +64,23 @@ fork_parent (void)
>    alarm (180);
>  
>    while (1)
> -    pause ();
> +    {
> +      if (exit_if_relative_exits)
> +	{
> +	  int res = wait (NULL);
> +	  if (res == -1 && errno == EINTR)
> +	    continue;
> +	  else if (res == -1)
> +	    {
> +	      perror ("wait");
> +	      return 1;
> +	    }
> +	  else
> +	    return 0;
> +	}
> +      else
> +	pause ();
> +    }
>  
>    return 0;
>  }
> diff --git a/gdb/testsuite/gdb.base/fork-running-state.exp b/gdb/testsuite/gdb.base/fork-running-state.exp
> index 27ed8a43e93..c15bc53f7a0 100644
> --- a/gdb/testsuite/gdb.base/fork-running-state.exp
> +++ b/gdb/testsuite/gdb.base/fork-running-state.exp
> @@ -70,6 +70,13 @@ proc do_test { detach_on_fork follow_fork non_stop schedule_multiple } {
>  	gdb_test_no_output "set schedule-multiple $schedule_multiple"
>      }
>  
> +    # If we're detaching from the parent (or child), then tell it to
> +    # exit itself when its child (or parent) exits.  If we stay
> +    # attached, we take care of killing it.
> +    if {$detach_on_fork == "on"} {
> +	gdb_test "print exit_if_relative_exits = 1" " = 1"
> +    }
> +
>      set test "continue &"
>      gdb_test_multiple $test $test {
>  	-re "$gdb_prompt " {
>