Re: [Bug-dejagnu] close_wait_program, process groups and kill /bin/sh builtin

[Richard Biener]

On Fri, 6 Jul 2018, Ben Elliston wrote:

> Hi Richard,
> 
> Long time, no talk!
> 
> On Thu, Jul 05, 2018 at 12:24:32PM +0200, Richard Biener wrote:
> 
> > So my suggestion is to first fix the pgid computation (which would
> > probably solve my other issue as well), then to either avoid using
> > shell builtins by doing && (env kill -15 ... or to not rely on the
> > particular semantics of multiple process kills and somehow iterate
> > over $pid/$pgid.
> 
> I agree with your approach. Would you care to work up a patch? :-)

Andreas Schwab rightfully points out that the test

    if { $pid > 0 } {

looks odd as well thus the whole close_wait_program function doesn't
seem to expect multiple PIDs.  The above is probably to catch

    # If we didn't get EOF, we have to kill the poor defenseless program.
    if { $got_eof } {
        set pid -1
    }

so it might work depending on what 12 234 > 0 semantically means
for TCL...  That said, without touching the above test the following
approach makes sure to not rely on any particular behavior for
multi-process kills.

I've verified it works for my case and I ran make check with the
patch on the git head where all tests passed.

So please apply.

Thanks,
Richard.


2018-07-06  Richard Biener  <address@hidden>

        * lib/remote.exp (close_wait_program): Use separate kill
        command for each pid.

diff --git a/lib/remote.exp b/lib/remote.exp
index 4dd6a8a..3d5d176 100644
--- a/lib/remote.exp
+++ b/lib/remote.exp
@@ -1,4 +1,4 @@
-# Copyright (C) 1992-2016 Free Software Foundation, Inc.
+# Copyright (C) 1992-2018 Free Software Foundation, Inc.
 #
 # This file is part of DejaGnu.
 #
@@ -70,13 +70,16 @@ proc close_wait_program { program_id pid {wres_varname ""} 
} {
        # Tcl has no kill primitive, so we have to execute an external
        # command in order to kill the process.
        verbose "doing kill, pid is $pid"
-       # Prepend "-" to generate the "process group ID" needed by
-       # kill.
-       set pgid "-$pid"
        # Send SIGINT to give the program a better chance to interrupt
        # whatever it might be doing and react to stdin closing.
        # eg, in case of GDB, this should get it back to the prompt.
-       exec sh -c "exec > /dev/null 2>&1 && (kill -2 $pgid || kill -2 $pid)"
+       # Do so separately for each PID in the list to avoid differences
+       # in return value behavior for kill between shells
+       foreach spid $pid {
+          # Prepend "-" to generate the "process group ID" needed by
+          # kill.
+         exec sh -c "exec > /dev/null 2>&1 && (kill -2 -$spid || kill -2 
$spid)"
+       }
 
        # If the program doesn't exit gracefully when stdin closes,
        # we'll need to kill it.  But only do this after 'wait'ing a
@@ -86,9 +89,15 @@ proc close_wait_program { program_id pid {wres_varname ""} } 
{
        # PID reuse race.
        set secs 5
        set sh_cmd "exec > /dev/null 2>&1"
-       append sh_cmd " && sleep $secs && (kill -15 $pgid || kill -15 $pid)"
-       append sh_cmd " && sleep $secs && (kill -9 $pgid || kill -9 $pid)"
-       append sh_cmd " && sleep $secs"
+       append sh_cmd " && sleep $secs && ("
+       foreach spid $pid {
+         append sh_cmd "(kill -15 -$spid || kill -15 $spid);"
+       }
+       append sh_cmd ") && sleep $secs && ("
+       foreach spid $pid {
+         append sh_cmd "(kill -9 -$spid || kill -9 $spid);"
+       }
+       append sh_cmd ") && sleep $secs"
        set exec_pid [exec sh -c "$sh_cmd" &]
     }
     verbose "pid is $pid"