From: Nicolas Edel Date: Fri, 18 Nov 2011 10:49:22 +0000 (+0200) Subject: pass errno from exec() to parent in spawn() X-Git-Url: http://repo.macrolet.net/gitweb/?a=commitdiff_plain;h=6cfdd9e66519b513e0935c410fbb30fc880efb61;p=sbcl.git pass errno from exec() to parent in spawn() Open a pipe, set FD_CLOEXEC. Child: if exec() fails, grab errno and write it to pipe. Parent: try to read from the pipe -- if you get something, it means the child didn't exec and the reason is in the pipe. Wait for the child to exit and return -1 and set errno to whatever the child got. Also use _exit() instead of exit() when dying in the child after exec failure -- running exit hooks there would probably be bad. (Somewhat edited from Nicolas' original patch.) Signed-off-by: Nikodemus Siivola --- diff --git a/NEWS b/NEWS index d0c7328..809a63e 100644 --- a/NEWS +++ b/NEWS @@ -12,6 +12,8 @@ changes relative to sbcl-1.0.53: comparatively weak bounds-check against the heap spaces. * enhancement: on win32, ABS of complex floats guards better against overflows. (lp#888410) + * enhancement: RUN-PROGRAM now distinguishes exec() failing from child + process exiting with code 1. (lp#676987) * bug fix: on 64-bit targets, atomic-incf/aref does index computation correctly, even on wide-fixnum builds. (lp#887220) * bug fix: (directory "foo/*/*.*") did not follow symlinks in foo/ that diff --git a/src/code/run-program.lisp b/src/code/run-program.lisp index 0417401..b4394a1 100644 --- a/src/code/run-program.lisp +++ b/src/code/run-program.lisp @@ -773,7 +773,7 @@ Users Manual for details about the PROCESS structure."#-win32" (if search 1 0) environment-vec pty-name (if wait 1 0)))) - (unless (= child -1) + (when (plusp child) (setf proc (apply #'make-process @@ -791,9 +791,11 @@ Users Manual for details about the PROCESS structure."#-win32" (list :%status :running)))) (push proc *active-processes*))))) ;; Report the error outside the lock. - (when (= child -1) - (error "couldn't fork child process: ~A" - (strerror))))))))) + (case child + (0 + (error "Couldn't execute ~A: ~A" progname (strerror))) + (-1 + (error "Couldn't fork child process: ~A" (strerror)))))))))) (dolist (fd *close-in-parent*) (sb-unix:unix-close fd)) (unless proc diff --git a/src/runtime/run-program.c b/src/runtime/run-program.c index 64d9bdf..7377e7f 100644 --- a/src/runtime/run-program.c +++ b/src/runtime/run-program.c @@ -28,6 +28,7 @@ #include #include +#include #ifdef LISP_FEATURE_OPENBSD /* FIXME: there has to be a better way to avoid ./util.h here */ @@ -101,12 +102,51 @@ extern char **environ; int spawn(char *program, char *argv[], int sin, int sout, int serr, int search, char *envp[], char *pty_name, int wait) { - int pid = fork(); + pid_t pid; int fd; + int channel[2]; sigset_t sset; - if (pid != 0) + channel[0] = -1; + channel[1] = -1; + if (!pipe(channel)) { + if (-1==fcntl(channel[1], F_SETFD, FD_CLOEXEC)) { + close(channel[1]); + channel[1] = -1; + } + } + + pid = fork(); + if (pid) { + if ((-1 != pid) && (-1 != channel[1])) { + int child_errno = 0; + int bytes = sizeof(int); + int n; + char *p = (char*)&child_errno; + close(channel[1]); + /* Try to read child errno from channel. */ + while ((bytes > 0) && + (n = read(channel[0], p, bytes))) { + if (-1 == n) { + if (EINTR == errno) { + continue; + } else { + break; + } + } else { + bytes -= n; + p += n; + } + } + if (child_errno) { + waitpid(pid, NULL, 0); + pid = 0; + errno = child_errno; + } + } return pid; + } + close (channel[0]); /* Put us in our own process group, but only if we need not * share stdin with our parent. In the latter case we claim @@ -144,10 +184,10 @@ int spawn(char *program, char *argv[], int sin, int sout, int serr, /* Close all other fds. */ #ifdef SVR4 for (fd = sysconf(_SC_OPEN_MAX)-1; fd >= 3; fd--) - close(fd); + if (fd != channel[1]) close(fd); #else for (fd = getdtablesize()-1; fd >= 3; fd--) - close(fd); + if (fd != channel[1]) close(fd); #endif environ = envp; @@ -157,7 +197,28 @@ int spawn(char *program, char *argv[], int sin, int sout, int serr, else execv(program, argv); - exit (1); + /* When exec fails and channel is available, send the errno value. */ + if (-1 != channel[1]) { + int our_errno = errno; + int bytes = sizeof(int); + int n; + char *p = (char*)&our_errno; + while ((bytes > 0) && + (n = write(channel[1], p, bytes))) { + if (-1 == n) { + if (EINTR == errno) { + continue; + } else { + break; + } + } else { + bytes -= n; + p += n; + } + } + close(channel[1]); + } + _exit(1); } #else /* !LISP_FEATURE_WIN32 */ diff --git a/tests/run-program.impure.lisp b/tests/run-program.impure.lisp index 8eae0d0..76972d3 100644 --- a/tests/run-program.impure.lisp +++ b/tests/run-program.impure.lisp @@ -293,3 +293,16 @@ (setq had-error-p t))) (assert (not had-error-p))))) +(with-test (:name (:run-program :no-such-thing)) + (assert (search "Couldn't execute" + (handler-case + (progn (run-program "no-such-program-we-hope" '()) nil) + (error (e) + (princ-to-string e)))))) + +(with-test (:name (:run-program :not-executable)) + (assert (search "Couldn't execute" + (handler-case + (progn (run-program "run-program.impure.lisp" '()) nil) + (error (e) + (princ-to-string e))))))