Date: Fri, 9 Mar 2012 23:54:33 +0100 From: Jilles Tjoelker <jilles@stack.nl> To: Matthew Story <matthewstory@gmail.com> Cc: freebsd-arch@freebsd.org Subject: Re: Change to xargs to avoid orphaning of utility processes on signal|exit 255 from child Message-ID: <20120309225433.GA32725@stack.nl> In-Reply-To: <CAB%2B9ogcVr-pNof-Tncc6F8CKjm3suqt=U-zvy9We0YEouwxhFw@mail.gmail.com> References: <CAB%2B9ogetrht1Ttf53vSbq7L5Fe9DbB5igSynKM3=tjZh0z0_ew@mail.gmail.com> <CAB%2B9ogcVr-pNof-Tncc6F8CKjm3suqt=U-zvy9We0YEouwxhFw@mail.gmail.com>
next in thread | previous in thread | raw e-mail | index | archive | help
On Tue, Feb 21, 2012 at 12:50:19PM -0500, Matthew Story wrote: > On Thu, Feb 16, 2012 at 7:09 PM, Matthew Story <matthewstory@gmail.com>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
Want to link to this message? Use this URL: <https://mail-archive.FreeBSD.org/cgi/mid.cgi?20120309225433.GA32725>