From owner-freebsd-arch@FreeBSD.ORG Fri Mar 9 22:54:35 2012 Return-Path: Delivered-To: freebsd-arch@freebsd.org Received: from mx1.freebsd.org (mx1.freebsd.org [IPv6:2001:4f8:fff6::34]) by hub.freebsd.org (Postfix) with ESMTP id 91A94106566B for ; Fri, 9 Mar 2012 22:54:35 +0000 (UTC) (envelope-from jilles@stack.nl) Received: from mx1.stack.nl (relay04.stack.nl [IPv6:2001:610:1108:5010::107]) by mx1.freebsd.org (Postfix) with ESMTP id 0F8618FC1F for ; Fri, 9 Mar 2012 22:54:35 +0000 (UTC) Received: from snail.stack.nl (snail.stack.nl [IPv6:2001:610:1108:5010::131]) by mx1.stack.nl (Postfix) with ESMTP id BA03E1DD5BC; Fri, 9 Mar 2012 23:54:33 +0100 (CET) Received: by snail.stack.nl (Postfix, from userid 1677) id 8F5A828470; Fri, 9 Mar 2012 23:54:33 +0100 (CET) Date: Fri, 9 Mar 2012 23:54:33 +0100 From: Jilles Tjoelker To: Matthew Story Message-ID: <20120309225433.GA32725@stack.nl> References: MIME-Version: 1.0 Content-Type: text/plain; charset=us-ascii Content-Disposition: inline In-Reply-To: User-Agent: Mutt/1.5.21 (2010-09-15) Cc: freebsd-arch@freebsd.org Subject: Re: Change to xargs to avoid orphaning of utility processes on signal|exit 255 from child X-BeenThere: freebsd-arch@freebsd.org X-Mailman-Version: 2.1.5 Precedence: list List-Id: Discussion related to FreeBSD architecture List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , X-List-Received-Date: Fri, 09 Mar 2012 22:54:35 -0000 On Tue, Feb 21, 2012 at 12:50:19PM -0500, Matthew Story wrote: > On Thu, Feb 16, 2012 at 7:09 PM, Matthew Story wrote: > > Apologies if this is the wrong list, I would like to submit a patch that > > changes the behavior of xargs(1) on signal to child utility process or > > child utility process exiting 255. The patch(es) is|are available here: > > http://axe0.blackskyresearch.net/patches/matt/xargs.no_orphan.patch.txt-- this version will apply to current xargs, and adds diagnostic > > information for exit 255|signal to utility, as required by POSIX (see > > PR165155). > > > > http://axe0.blackskyresearch.net/patches/matt/xargs.no_orphan.PR165155.patch.txt-- this version will apply on top of the patch in PR165155, as the errx > > calls in that patch need to be modified to warnx calls. > I have updated these 2 patches above to branch correctly (the change from > errx to warnx requires else'ing ... may have to purge browser catch to pick > up the change), wondering if this patch should be expanded to include all > of the cases mentioned in the ``Consequences of Errors'' section of the > POSIX specification: > > [... snip] > > If a command line meeting the specified requirements cannot be assembled, > > the utility cannot be invoked, an invocation of the utility is terminated > > by a signal, or an invocation of the utility exits with exit status 255, > > the *xargs* utility shall write a diagnostic message and exit without > > processing any remaining input. > This would cause xargs to wait on children if the command line cannot be > assembled, or utility cannot be invoked, in addition to the 2 cases covered > by the patch. This should leave termination via signal to the xargs > process itself as the only outstanding case where xargs orphans utilities, > which is congruent with sh(1) behavior, and still allows for signaling all > processes via signal to the process group, if you actually desire to signal > all utility processes, along with xargs itself. Thoughts? I think that makes sense. I updated the patch to the recent changes in -current and changed a local from short to int: Index: xargs.1 =================================================================== --- xargs.1 (revision 232399) +++ xargs.1 (working copy) @@ -297,14 +297,18 @@ The .Nm utility exits immediately (without processing any further input) if a -command line cannot be assembled, +command line cannot be assembled, or .Ar utility -cannot be invoked, an invocation of +cannot be invoked. If an invocation of .Ar utility is terminated by a signal, or an invocation of .Ar utility -exits with a value of 255. +exits with a value of 255, the +.Nm +utility stops processing input and exits after all invocations of +.Ar utility +finish processing. .Sh EXIT STATUS The .Nm Index: xargs.c =================================================================== --- xargs.c (revision 232399) +++ xargs.c (working copy) @@ -597,6 +597,7 @@ { pid_t pid; int status; + int cause_exit = 0; while ((pid = xwait(waitall || pids_full(), &status)) > 0) { /* If we couldn't invoke the utility, exit. */ @@ -604,15 +605,27 @@ errno = childerr; err(errno == ENOENT ? 127 : 126, "%s", name); } - if (WIFSIGNALED(status)) - errx(1, "%s: terminated with signal %d; aborting", + /* + * If utility exited because of a signal or with a value of + * 255, warn (per POSIX), and then wait until all other + * children have exited before exiting 1-125. POSIX requires + * xargs to stop reading if child exits because of a signal or + * with 255, but it does not require us to exit immediately; + * waiting is preferable to orphaning. + */ + if (WIFSIGNALED(status)) { + waitall = cause_exit = 1; + warnx("%s: terminated with signal %d; aborting", name, WTERMSIG(status)); - if (WEXITSTATUS(status) == 255) - errx(1, "%s: exited with status 255; aborting", name); - if (WEXITSTATUS(status)) + } else if (WEXITSTATUS(status) == 255) { + waitall = cause_exit = 1; + warnx("%s: exited with status 255; aborting", name); + } else if (WEXITSTATUS(status)) rval = 1; } + if (cause_exit) + exit(1); if (pid == -1 && errno != ECHILD) err(1, "waitpid"); } -- Jilles Tjoelker