From patchwork Mon Jun 30 09:15:18 2014 Content-Type: text/plain; charset="utf-8" MIME-Version: 1.0 Content-Transfer-Encoding: 7bit X-Patchwork-Submitter: "Maciej W. Rozycki" X-Patchwork-Id: 1815 Received: (qmail 7796 invoked by alias); 30 Jun 2014 09:15:40 -0000 Mailing-List: contact libc-alpha-help@sourceware.org; run by ezmlm Precedence: bulk List-Id: List-Unsubscribe: List-Subscribe: List-Archive: List-Post: List-Help: , Sender: libc-alpha-owner@sourceware.org Delivered-To: mailing list libc-alpha@sourceware.org Received: (qmail 7711 invoked by uid 89); 30 Jun 2014 09:15:39 -0000 Authentication-Results: sourceware.org; auth=none X-Virus-Found: No X-Spam-SWARE-Status: No, score=-1.0 required=5.0 tests=AWL, BAYES_00 autolearn=ham version=3.3.2 X-HELO: relay1.mentorg.com Date: Mon, 30 Jun 2014 10:15:18 +0100 From: "Maciej W. Rozycki" To: Roland McGrath CC: , =?ISO-8859-2?Q?Ond=F8ej_B=EDlka?= Subject: Re: [PATCH v2] test-skeleton: Kill any child process's offspring In-Reply-To: <20140623173427.7D80F2C3A0C@topped-with-meat.com> Message-ID: References: <20131011163332.GA20086@domone.podge> <20140210171709.GA27843@domone> <20140623173427.7D80F2C3A0C@topped-with-meat.com> User-Agent: Alpine 1.10 (DEB 962 2008-03-14) MIME-Version: 1.0 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 Roland McGrath * 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 . */ +#include #include #include #include @@ -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);