[PATCH] [RFC] sh(1) vfork support

[Adrian Chadd]

If you're going to go down this path, would you mind making it depend
upon the contents of an environment variable?

Ie, DO_SH_VFORK="yes|no"

Then debugging can occur without having to recompile sh.

I ask simply because I've been down this path before in other
projects, and trying to debug vfork() vs fork() (and getting users to
patch source code to test) became extremely tedious and annoying.

2c,


Adrian

On 14 June 2011 06:52, Jilles Tjoelker <jilles at stack.nl> wrote:
> The below patch changes sh to use vfork(2) instead of fork(2) in some
> simple cases: a foreground simple command, not being a command
> substitution, without redirections or assignments in a non-interactive
> shell with job control disabled.
>
> By restricting the use of vfork, different from what NetBSD has done, I
> limit what code is executed in the vforked child process. For example,
> there is no way opening general redirections in a vforked background
> process can work, as opening a fifo may block, and any long-term
> blocking of a vforked-but-not-execed process is bad.
>
> Background simple commands are also excluded because our current
> approach of expanding their arguments in the main shell environment is
> incorrect (zsh is broken in the same way).
>
> The restriction to non-interactive shells with job control disabled also
> limits the amount of code I have to copy to the vfork code path.
>
> Command substitutions containing one simple command could also use vfork
> but this needs a little extra code.
>
> The patch does not depend on the memory sharing semantics of vfork. As a
> result, however, it calls strerror() (and therefore all sorts of NLS
> code) and stdio in the child, in the case that exec fails. Even in the
> normal case, malloc() may be called to allocate space for the full
> pathname of each executable to be tried. This could be fixed by
> reserving enough space for the pathname first and passing the errno
> value back through memory and printing the message in the parent process
> but this also introduces additional differences with the regular code
> path.
>
> The functions setjmp() and longjmp() are also used but only within a
> process; no jump is made by the vforked process outside
> vforkexecshell().
>
> Simple tests on various microbenchmarks (i386, calling /bin/echo 10000
> times, possibly from multiple processes at once) show a performance
> improvement of 20-30%. The gain might be bigger on embedded
> architectures where less time has been spent to optimize fork(2).
>
> Unfortunately, it looks like rc.d may not be helped much by this. It
> uses many subshells, most of which require a full fork() with the
> current implementation of sh.
>
> Index: bin/sh/jobs.h
> ===================================================================
> --- bin/sh/jobs.h       (revision 223060)
> +++ bin/sh/jobs.h       (working copy)
> @@ -91,6 +91,7 @@
>  void showjobs(int, int);
>  struct job *makejob(union node *, int);
>  pid_t forkshell(struct job *, union node *, int);
> +pid_t vforkexecshell(struct job *, char **, char **, const char *, int);
>  int waitforjob(struct job *, int *);
>  int stoppedjobs(void);
>  int backgndpidset(void);
> Index: bin/sh/eval.c
> ===================================================================
> --- bin/sh/eval.c       (revision 223024)
> +++ bin/sh/eval.c       (working copy)
> @@ -899,6 +899,15 @@
>                        if (pipe(pip) < 0)
>                                error("Pipe call failed: %s", strerror(errno));
>                }
> +               if (cmdentry.cmdtype == CMDNORMAL &&
> +                   cmd->ncmd.redirect == NULL &&
> +                   varlist.list == NULL &&
> +                   mode == FORK_FG &&
> +                   !iflag && !mflag) {
> +                       vforkexecshell(jp, argv, environment(), path,
> +                           cmdentry.u.index);
> +                       goto parent;
> +               }
>                if (forkshell(jp, cmd, mode) != 0)
>                        goto parent;    /* at end of routine */
>                if (flags & EV_BACKCMD) {
> Index: bin/sh/jobs.c
> ===================================================================
> --- bin/sh/jobs.c       (revision 223060)
> +++ bin/sh/jobs.c       (working copy)
> @@ -57,6 +57,7 @@
>  #undef CEOF                    /* syntax.h redefines this */
>  #endif
>  #include "redir.h"
> +#include "exec.h"
>  #include "show.h"
>  #include "main.h"
>  #include "parser.h"
> @@ -884,7 +885,48 @@
>  }
>
>
> +pid_t
> +vforkexecshell(struct job *jp, char **argv, char **envp, const char *path, int idx)
> +{
> +       pid_t pid;
> +       struct jmploc jmploc;
> +       struct jmploc *savehandler;
>
> +       TRACE(("vforkexecshell(%%%td, %p, %d) called\n", jp - jobtab, (void *)n,
> +           mode));
> +       INTOFF;
> +       flushall();
> +       savehandler = handler;
> +       pid = vfork();
> +       if (pid == -1) {
> +               TRACE(("Vfork failed, errno=%d\n", errno));
> +               INTON;
> +               error("Cannot fork: %s", strerror(errno));
> +       }
> +       if (pid == 0) {
> +               TRACE(("Child shell %d\n", (int)getpid()));
> +               if (setjmp(jmploc.loc))
> +                       _exit(exception == EXEXEC ? exerrno : 2);
> +               handler = &jmploc;
> +               shellexec(argv, envp, path, idx);
> +       }
> +       handler = savehandler;
> +       if (jp) {
> +               struct procstat *ps = &jp->ps[jp->nprocs++];
> +               ps->pid = pid;
> +               ps->status = -1;
> +               ps->cmd = nullstr;
> +               jp->foreground = 1;
> +#if JOBS
> +               setcurjob(jp);
> +#endif
> +       }
> +       INTON;
> +       TRACE(("In parent shell:  child = %d\n", (int)pid));
> +       return pid;
> +}
> +
> +
>  /*
>  * Wait for job to finish.
>  *
>
> --
> Jilles Tjoelker
> _______________________________________________
> freebsd-hackers at freebsd.org mailing list
> http://lists.freebsd.org/mailman/listinfo/freebsd-hackers
> To unsubscribe, send any mail to "freebsd-hackers-unsubscribe at freebsd.org"
>