Change to xargs to avoid orphaning of utility processes on
signal|exit 255 from child
Matthew Story
matthewstory at gmail.com
Fri Mar 9 23:13:30 UTC 2012
On Fri, Mar 9, 2012 at 5:54 PM, Jilles Tjoelker <jilles at stack.nl> wrote:
> 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 at 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");
> }
>
changes look good to me, thanks for cleaning it up for -CURRENT. I will
provide an additional patch to cover waiting for the other 2 cases (cannot
assemble, or cannot invoke) later tonight after some more testing.
--
regards,
matt
More information about the freebsd-arch
mailing list