Re: close_wait_program, process groups and kill /bin/sh builtin

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  <[email protected]>

	* 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"