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 <nikodemus@random-state.net>

diff --git a/NEWS b/NEWS
index d0c7328..809a63e 100644 (file)
--- 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 (file)
--- 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 (file)
--- a/src/runtime/run-program.c
+++ b/src/runtime/run-program.c
@@ -28,6 +28,7 @@
 
 #include <sys/ioctl.h>
 #include <termios.h>
+#include <errno.h>
 
 #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 (file)
--- 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))))))