[v2] test-skeleton: Kill any child process's offspring
Commit Message
On Mon, 23 Jun 2014, Roland McGrath wrote:
> I don't know what you did to test that change. I can't see how it could
> possibly work as intended. You've changed the value of PID in the child,
> but it's only actually used in the parent (where it won't have changed).
> Am I missing something?
That my brain long needed the time off that I took last week perhaps,
sigh...
> Without more futzing than is worthwhile, there is no way that the child can
> communicate back to the parent whether setpgid succeeded or failed. The
> error message is nice to have, but I don't think the parent actually needs
> to care whether setpgid worked or not. If it didn't, there won't be any
> pgrp with pgid -pid.
That's sort-of why I argued there's no need to check for an error from
setpgid (0, 0) in the first place (of course with other sets of arguments
setpgid can indeed fail).
> So I think all you really need is to make the handler do:
>
> assert (pid > 1);
> /* Kill the whole process group. */
> kill (-pid, SIGKILL);
> /* In case, setpgid failed in the child, kill it individually too. */
> kill (pid, SIGKILL);
Thanks, here's the resulting implementation. I put it through a test run
so as to avoid any surprises. It produced one `Timed out: killed the
child process' message on the way and I saw no weird stuff.
OK to apply?
2014-06-30 Maciej W. Rozycki <macro@codesourcery.com>
Roland McGrath <roland@hack.frob.com>
* test-skeleton.c (signal_handler): Kill the whole process group
before killing the child individually.
(main): Report any failure on `setpgid'.
Maciej
glibc-test-skeleton-kill-pgid.diff
Comments
> + /* In case, setpgid failed in the child, kill it individually too. */
> kill (pid, SIGKILL);
Drop the first comma. Otherwise all looks right to me.
On Mon, 30 Jun 2014, Roland McGrath wrote:
> > + /* In case, setpgid failed in the child, kill it individually too. */
> > kill (pid, SIGKILL);
>
> Drop the first comma. Otherwise all looks right to me.
Applied. Thanks for your effort with the review.
Maciej
===================================================================
@@ -17,6 +17,7 @@
License along with the GNU C Library; if not, see
<http://www.gnu.org/licenses/>. */
+#include <assert.h>
#include <errno.h>
#include <fcntl.h>
#include <getopt.h>
@@ -138,7 +139,10 @@ signal_handler (int sig __attribute__ ((
int killed;
int status;
- /* Send signal. */
+ assert (pid > 1);
+ /* Kill the whole process group. */
+ kill (-pid, SIGKILL);
+ /* In case, setpgid failed in the child, kill it individually too. */
kill (pid, SIGKILL);
/* Wait for it to terminate. */
@@ -342,7 +346,8 @@ main (int argc, char *argv[])
/* We put the test process in its own pgrp so that if it bogusly
generates any job control signals, they won't hit the whole build. */
- setpgid (0, 0);
+ if (setpgid (0, 0) != 0)
+ printf ("Failed to set the process group ID: %m\n");
/* Execute the test function and exit with the return value. */
exit (TEST_FUNCTION);