This is the mail archive of the
libc-alpha@sourceware.org
mailing list for the glibc project.
Re: [PATCH v2] test-skeleton: Kill any child process's offspring
- From: "Maciej W. Rozycki" <macro at codesourcery dot com>
- To: Roland McGrath <roland at hack dot frob dot com>
- Cc: <libc-alpha at sourceware dot org>, Ondřej Bílka <neleai at seznam dot cz>
- Date: Mon, 30 Jun 2014 10:15:18 +0100
- Subject: Re: [PATCH v2] test-skeleton: Kill any child process's offspring
- Authentication-results: sourceware.org; auth=none
- References: <alpine dot DEB dot 1 dot 10 dot 1309231748080 dot 4379 at tp dot orcam dot me dot uk> <20131011163332 dot GA20086 at domone dot podge> <20140210171709 dot GA27843 at domone> <alpine dot DEB dot 1 dot 10 dot 1406230100010 dot 25395 at tp dot orcam dot me dot uk> <20140623173427 dot 7D80F2C3A0C at topped-with-meat dot com>
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
Index: glibc-fsf-trunk-quilt/test-skeleton.c
===================================================================
--- glibc-fsf-trunk-quilt.orig/test-skeleton.c 2014-06-23 02:07:41.000000000 +0100
+++ glibc-fsf-trunk-quilt/test-skeleton.c 2014-06-23 21:59:15.801805581 +0100
@@ -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);