diff --git a/System/Process/Posix.hs b/System/Process/Posix.hs --- a/System/Process/Posix.hs +++ b/System/Process/Posix.hs @@ -139,18 +139,20 @@ when mb_delegate_ctlc startDelegateControlC + let flags = (if mb_close_fds then RUN_PROCESS_IN_CLOSE_FDS else 0) + .|.(if mb_create_group then RUN_PROCESS_IN_NEW_GROUP else 0) + .|.(if mb_detach_console then RUN_PROCESS_DETACHED else 0) + .|.(if mb_create_new_console then RUN_PROCESS_NEW_CONSOLE else 0) + .|.(if mb_new_session then RUN_PROCESS_NEW_SESSION else 0) + .|.(if mb_delegate_ctlc then RESET_INT_QUIT_HANDLERS else 0) + -- See the comment on runInteractiveProcess_lock proc_handle <- withMVar runInteractiveProcess_lock $ \_ -> c_runInteractiveProcess pargs pWorkDir pEnv fdin fdout fderr pfdStdInput pfdStdOutput pfdStdError pChildGroup pChildUser - (if mb_delegate_ctlc then 1 else 0) - ((if mb_close_fds then RUN_PROCESS_IN_CLOSE_FDS else 0) - .|.(if mb_create_group then RUN_PROCESS_IN_NEW_GROUP else 0) - .|.(if mb_detach_console then RUN_PROCESS_DETACHED else 0) - .|.(if mb_create_new_console then RUN_PROCESS_NEW_CONSOLE else 0) - .|.(if mb_new_session then RUN_PROCESS_NEW_SESSION else 0)) + flags pFailedDoing when (proc_handle == -1) $ do @@ -273,7 +275,6 @@ -> Ptr FD -> Ptr CGid -> Ptr CUid - -> CInt -- reset child's SIGINT & SIGQUIT handlers -> CInt -- flags -> Ptr CString -> IO PHANDLE diff --git a/cbits/posix/find_executable.c b/cbits/posix/find_executable.c new file mode 100644 --- /dev/null +++ b/cbits/posix/find_executable.c @@ -0,0 +1,79 @@ +/* ---------------------------------------------------------------------------- + * search path search logic + * (c) Ben Gamari 2021 + */ + +#include +#include +#include +#include +#include + +#include "common.h" + +// the below is only necessary when we don't have execvpe. +#if !defined(HAVE_execvpe) + +/* Return true if the given file exists and is an executable. */ +static bool is_executable(const char *path) { + return access(path, X_OK) == 0; +} + +/* Find an executable with the given filename in the given search path. The + * result must be freed by the caller. Returns NULL if a matching file is not + * found. + */ +static char *find_in_search_path(char *search_path, const char *filename) { + const int filename_len = strlen(filename); + char *tokbuf; + char *path = strtok_r(search_path, ":", &tokbuf); + while (path != NULL) { + const int tmp_len = filename_len + 1 + strlen(path) + 1; + char *tmp = malloc(tmp_len); + snprintf(tmp, tmp_len, "%s/%s", path, filename); + if (is_executable(tmp)) { + return tmp; + } else { + free(tmp); + } + + path = strtok_r(NULL, ":", &tokbuf); + } + return NULL; +} + +/* Identify the executable search path. The result must be freed by the caller. */ +static char *get_executable_search_path(void) { + char *search_path; + + search_path = getenv("PATH"); + if (search_path) { + search_path = strdup(search_path); + return search_path; + } + +#if defined(HAVE_CONFSTR) + int len = confstr(_CS_PATH, NULL, 0); + search_path = malloc(len + 1) + if (search_path != NULL) { + search_path[0] = ':'; + (void) confstr (_CS_PATH, search_path + 1, len); + return search_path; + } +#endif + + return strdup(":"); +} + +/* Find the given executable in the executable search path. */ +char *find_executable(char *filename) { + /* If it's an absolute or relative path name, it's easy. */ + if (strchr(filename, '/') && is_executable(filename)) { + return filename; + } + + char *search_path = get_executable_search_path(); + return find_in_search_path(search_path, filename); +} + +#endif diff --git a/cbits/posix/fork_exec.c b/cbits/posix/fork_exec.c new file mode 100644 --- /dev/null +++ b/cbits/posix/fork_exec.c @@ -0,0 +1,312 @@ +#include "common.h" + +#include +#include +#include +#include +#include + +#if !(defined(_MSC_VER) || defined(__MINGW32__) || defined(_WIN32)) +#include +#include +#endif + +#if defined(HAVE_FCNTL_H) +#include +#endif + +#if defined(HAVE_SIGNAL_H) +#include +#endif + +#if defined(HAVE_VFORK_H) +#include +#endif + +#include + +#if defined(HAVE_WORKING_VFORK) +#define myfork vfork +#elif defined(HAVE_WORKING_FORK) +#define myfork fork +// We don't need a fork command on Windows +#else +#error Cannot find a working fork command +#endif + +// Rts internal API, not exposed in a public header file: +extern void blockUserSignals(void); +extern void unblockUserSignals(void); + +__attribute__((__noreturn__)) +static void +child_failed(int pipe, const char *failed_doing) { + int err; + ssize_t unused __attribute__((unused)); + + err = errno; + // Having the child send the failed_doing pointer across the pipe is safe as + // we know that the child still has the same address space as the parent. + unused = write(pipe, &failed_doing, sizeof(failed_doing)); + unused = write(pipe, &err, sizeof(err)); + // As a fallback, exit + _exit(127); +} + +static int +setup_std_handle_fork(int fd, + struct std_handle *b, + int pipe) +{ + switch (b->behavior) { + case STD_HANDLE_CLOSE: + if (close(fd) == -1) { + child_failed(pipe, "close"); + return -1; + } + return 0; + + case STD_HANDLE_USE_FD: + if (dup2(b->use_fd, fd) == -1) { + child_failed(pipe, "dup2"); + } + return 0; + + case STD_HANDLE_USE_PIPE: + if (b->use_pipe.child_end != fd) { + if (dup2(b->use_pipe.child_end, fd) == -1) { + child_failed(pipe, "dup2(child_end)"); + } + if (close(b->use_pipe.child_end) == -1) { + child_failed(pipe, "close(child_end)"); + } + } + if (close(b->use_pipe.parent_end) == -1) { + child_failed(pipe, "close(parent_end)"); + } + break; + } +} + +/* Try spawning with fork. */ +ProcHandle +do_spawn_fork (char *const args[], + char *workingDirectory, char **environment, + struct std_handle *stdInHdl, + struct std_handle *stdOutHdl, + struct std_handle *stdErrHdl, + gid_t *childGroup, uid_t *childUser, + int flags, + char **failed_doing) +{ + int forkCommunicationFds[2]; + int r = pipe(forkCommunicationFds); + if (r == -1) { + *failed_doing = "pipe"; + return -1; + } + + // Block signals with Haskell handlers. The danger here is that + // with the threaded RTS, a signal arrives in the child process, + // the RTS writes the signal information into the pipe (which is + // shared between parent and child), and the parent behaves as if + // the signal had been raised. + blockUserSignals(); + + // See #4074. Sometimes fork() gets interrupted by the timer + // signal and keeps restarting indefinitely. + stopTimer(); + + // N.B. execvpe is not supposed on some platforms. In this case + // we emulate this using fork and exec. However, to safely do so + // we need to perform all allocations *prior* to forking. Consequently, we + // need to find_executable before forking. +#if !defined(HAVE_execvpe) + char *exec_path; + if (environment) { + exec_path = find_executable(args[0]); + } +#endif + + int pid = myfork(); + switch(pid) + { + case -1: + unblockUserSignals(); + startTimer(); + close(forkCommunicationFds[0]); + close(forkCommunicationFds[1]); + *failed_doing = "fork"; + return -1; + + case 0: + // WARNING! We may now be in the child of vfork(), and any + // memory we modify below may also be seen in the parent + // process. + + close(forkCommunicationFds[0]); + fcntl(forkCommunicationFds[1], F_SETFD, FD_CLOEXEC); + + if ((flags & RUN_PROCESS_NEW_SESSION) != 0) { + setsid(); + } + if ((flags & RUN_PROCESS_IN_NEW_GROUP) != 0) { + setpgid(0, 0); + } + + if (childGroup) { + if (setgid( *childGroup) != 0) { + // ERROR + child_failed(forkCommunicationFds[1], "setgid"); + } + } + + if (childUser) { + // Using setuid properly first requires that we initgroups. + // However, to do this we must know the username of the user we are + // switching to. + struct passwd pw; + struct passwd *res = NULL; + int buf_len = sysconf(_SC_GETPW_R_SIZE_MAX); + // TODO: Strictly speaking malloc is a no-no after fork() since it + // may try to take a lock + char *buf = malloc(buf_len); + gid_t suppl_gid = childGroup ? *childGroup : getgid(); + if ( getpwuid_r(*childUser, &pw, buf, buf_len, &res) != 0) { + child_failed(forkCommunicationFds[1], "getpwuid"); + } + if ( res == NULL ) { + child_failed(forkCommunicationFds[1], "getpwuid"); + } + if ( initgroups(res->pw_name, suppl_gid) != 0) { + child_failed(forkCommunicationFds[1], "initgroups"); + } + if ( setuid( *childUser) != 0) { + // ERROR + child_failed(forkCommunicationFds[1], "setuid"); + } + } + + unblockUserSignals(); + + if (workingDirectory) { + if (chdir (workingDirectory) < 0) { + child_failed(forkCommunicationFds[1], "chdir"); + } + } + + // Note [Ordering of handle closing] + // ~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~ + // Ordering matters here. If any of the FDs + // 0,1,2 were initially closed, then our pipes may have used + // these FDs. So when we dup2 the pipe FDs down to 0,1,2, we + // must do it in that order, otherwise we could overwrite an + // FD that we need later. See ticket #431. + + setup_std_handle_fork(STDIN_FILENO, stdInHdl, forkCommunicationFds[1]); + setup_std_handle_fork(STDOUT_FILENO, stdOutHdl, forkCommunicationFds[1]); + setup_std_handle_fork(STDERR_FILENO, stdErrHdl, forkCommunicationFds[1]); + + if ((flags & RUN_PROCESS_IN_CLOSE_FDS) != 0) { + int max_fd = get_max_fd(); + // XXX Not the pipe + for (int i = 3; i < max_fd; i++) { + if (i != forkCommunicationFds[1]) { + close(i); + } + } + } + + /* Reset the SIGINT/SIGQUIT signal handlers in the child, if requested + */ + if (flags & RESET_INT_QUIT_HANDLERS != 0) { + struct sigaction dfl; + (void)sigemptyset(&dfl.sa_mask); + dfl.sa_flags = 0; + dfl.sa_handler = SIG_DFL; + (void)sigaction(SIGINT, &dfl, NULL); + (void)sigaction(SIGQUIT, &dfl, NULL); + } + + /* the child */ + if (environment) { +#if defined(HAVE_execvpe) + // XXX Check result + execvpe(args[0], args, environment); +#else + // XXX Check result + execve(exec_path, args, environment); +#endif + } else { + // XXX Check result + execvp(args[0], args); + } + + child_failed(forkCommunicationFds[1], "exec"); + + default: + if ((flags & RUN_PROCESS_IN_NEW_GROUP) != 0) { + setpgid(pid, pid); + } + close(forkCommunicationFds[1]); + fcntl(forkCommunicationFds[0], F_SETFD, FD_CLOEXEC); + + break; + } + + // If the child process had a problem, then it will tell us via the + // forkCommunicationFds pipe. First we try to read what the problem + // was. Note that if none of these conditionals match then we fall + // through and just return pid. + char *fail_reason; + r = read(forkCommunicationFds[0], &fail_reason, sizeof(fail_reason)); + if (r == -1) { + *failed_doing = "read pipe"; + pid = -1; + } + else if (r == sizeof(fail_reason)) { + *failed_doing = fail_reason; + + // Now we try to get the errno from the child + int err; + r = read(forkCommunicationFds[0], &err, sizeof(err)); + if (r == -1) { + *failed_doing = "read pipe"; + } else if (r != sizeof(err)) { + *failed_doing = "read pipe bad length"; + } else { + // If we succeed then we set errno. It'll be saved and + // restored again below. Note that in any other case we'll + // get the errno of whatever else went wrong instead. + errno = err; + } + + // We forked the child, but the child had a problem and stopped so it's + // our responsibility to reap here as nobody else can. + waitpid(pid, NULL, 0); + + // Already closed child ends above + if (stdInHdl->behavior == STD_HANDLE_USE_PIPE) { + close(stdInHdl->use_pipe.parent_end); + } + if (stdOutHdl->behavior == STD_HANDLE_USE_PIPE) { + close(stdOutHdl->use_pipe.parent_end); + } + if (stdErrHdl->behavior == STD_HANDLE_USE_PIPE) { + close(stdErrHdl->use_pipe.parent_end); + } + + pid = -1; + } + else if (r != 0) { + *failed_doing = "read pipe bad length"; + pid = -1; + } + + close(forkCommunicationFds[0]); + + unblockUserSignals(); + startTimer(); + + return pid; +} diff --git a/cbits/posix/posix_spawn.c b/cbits/posix/posix_spawn.c new file mode 100644 --- /dev/null +++ b/cbits/posix/posix_spawn.c @@ -0,0 +1,211 @@ +#include "runProcess.h" +#include "common.h" + +#include +#include +#include + +#if !defined(HAVE_POSIX_SPAWNP) +ProcHandle +do_spawn_posix (char *const args[], + char *workingDirectory, char **environment, + struct std_handle *stdInHdl, + struct std_handle *stdOutHdl, + struct std_handle *stdErrHdl, + gid_t *childGroup, uid_t *childUser, + int flags, + char **failed_doing) +{ + return -2; +} + +#else + +// Necessary for POSIX_SPAWN_SETSID under glibc. +#define _GNU_SOURCE +#include + +extern char **environ; + +static int +setup_std_handle_spawn (int fd, + struct std_handle *hdl, + posix_spawn_file_actions_t *fa, + char **failed_doing) +{ + switch (hdl->behavior) { + case STD_HANDLE_CLOSE: + if (posix_spawn_file_actions_addclose(fa, fd) != 0) { + *failed_doing = "posix_spawn_file_actions_addclose"; + return -1; + } + return 0; + + case STD_HANDLE_USE_FD: + if (posix_spawn_file_actions_adddup2(fa, hdl->use_fd, fd) != 0) { + *failed_doing = "posix_spawn_file_actions_adddup2"; + return -1; + } + return 0; + + case STD_HANDLE_USE_PIPE: + if (hdl->use_pipe.child_end != fd) { + if (posix_spawn_file_actions_adddup2(fa, hdl->use_pipe.child_end, fd) != 0) { + *failed_doing = "posix_spawn_file_actions_adddup2(child_end)"; + return -1; + } + if (posix_spawn_file_actions_addclose(fa, hdl->use_pipe.child_end) != 0) { + *failed_doing = "posix_spawn_file_actions_addclose(child_end)"; + return -1; + } + } + if (posix_spawn_file_actions_addclose(fa, hdl->use_pipe.parent_end) != 0) { + *failed_doing = "posix_spawn_file_actions_addclose(parent_end)"; + return -1; + } + return 0; + } +} + +/* Try spawning with posix_spawn. Returns -1 if posix_spawn (or the particular + * requested configuration) is not supported. + */ +ProcHandle +do_spawn_posix (char *const args[], + char *workingDirectory, char **environment, + struct std_handle *stdInHdl, + struct std_handle *stdOutHdl, + struct std_handle *stdErrHdl, + gid_t *childGroup, uid_t *childUser, + int flags, + char **failed_doing) +{ + // First do catch some cases that posix_spawn simply can't handle... + if (childGroup || childUser) { + return -2; + } + if ((flags & RUN_PROCESS_IN_CLOSE_FDS) != 0) { + // TODO: can this be efficiently supported? + return -2; + } + + // Now the main act... + pid_t pid; + posix_spawn_file_actions_t fa; + posix_spawnattr_t sa; + int r; + ProcHandle ret; + short spawn_flags = 0; + + r = posix_spawn_file_actions_init(&fa); + if (r != 0) { + *failed_doing = "posix_spawn_file_actions_init"; + return -1; + } + + r = posix_spawnattr_init(&sa); + if (r != 0) { + posix_spawn_file_actions_destroy(&fa); + *failed_doing = "posix_spawnattr_init"; + return -1; + } + + if (workingDirectory) { +#if defined(HAVE_posix_spawn_file_actions_addchdir_np) + // N.B. this function is broken on macOS. + // See https://github.com/rust-lang/rust/pull/80537. + r = posix_spawn_file_actions_addchdir(&fa, workingDirectory); + if (r != 0) { + *failed_doing = "posix_spawn_file_actions_addchdir"; + goto fail; + } +#else + goto not_supported; +#endif + } + + if ((flags & RUN_PROCESS_NEW_SESSION) != 0) { +#if defined(HAVE_POSIX_SPAWN_SETSID) + // N.B. POSIX_SPAWN_SETSID is only supported on macOS 10.15 and later, + // but an proper error is not returned in earlier versions. + // See https://bugs.python.org/issue37586. + if (posix_spawnattr_setflags(&sa, POSIX_SPAWN_SETSID) != 0) { + *failed_doing = "posix_spawnattr_setflags(POSIX_SPAWN_SETSID)"; + goto fail; + } +#elif defined(HAVE_POSIX_SPAWN_SETSID_NP) + if (posix_spawnattr_setflags(&sa, POSIX_SPAWN_SETSID_NP) != 0) { + *failed_doing = "posix_spawnattr_setflags(POSIX_SPAWN_SETSID_NP)"; + goto fail; + } +#else + goto not_supported; +#endif + } + +#if defined(HAVE_POSIX_SPAWN_SETPGROUP) + if ((flags & RUN_PROCESS_IN_NEW_GROUP) != 0) { + spawn_flags |= POSIX_SPAWN_SETPGROUP; + } +#endif + + if (setup_std_handle_spawn(STDIN_FILENO, stdInHdl, &fa, failed_doing) != 0) { + goto fail; + } + if (setup_std_handle_spawn(STDOUT_FILENO, stdOutHdl, &fa, failed_doing) != 0) { + goto fail; + } + if (setup_std_handle_spawn(STDERR_FILENO, stdErrHdl, &fa, failed_doing) != 0) { + goto fail; + } + + sigset_t ss; + if ((flags & RESET_INT_QUIT_HANDLERS) != 0) { + if (sigemptyset(&ss) == -1) { + *failed_doing = "sigemptyset"; + goto fail; + } + if (sigaddset(&ss, SIGINT) == -1) { + *failed_doing = "sigaddset(SIGINT)"; + goto fail; + } + if (sigaddset(&ss, SIGQUIT) == -1) { + *failed_doing = "sigaddset(SIGQUIT)"; + goto fail; + } + if (posix_spawnattr_setsigdefault(&sa, &ss) != 0) { + *failed_doing = "posix_spawnattr_setsigdefault"; + goto fail; + } + spawn_flags |= POSIX_SPAWN_SETSIGDEF; + } + + if (posix_spawnattr_setflags(&sa, spawn_flags) != 0) { + *failed_doing = "posix_spawnattr_setflags"; + goto fail; + } + + r = posix_spawnp(&pid, args[0], &fa, &sa, args, environment ? environment : environ); + if (r != 0) { + *failed_doing = "posix_spawnp"; + goto fail; + } else { + ret = pid; + goto finish; + } + +not_supported: + ret = -2; + goto finish; + +fail: + ret = -1; + goto finish; + +finish: + posix_spawn_file_actions_destroy(&fa); + posix_spawnattr_destroy(&sa); + return ret; +} + +#endif diff --git a/cbits/posix/runProcess.c b/cbits/posix/runProcess.c --- a/cbits/posix/runProcess.c +++ b/cbits/posix/runProcess.c @@ -4,409 +4,196 @@ Support for System.Process ------------------------------------------------------------------------- */ -/* XXX This is a nasty hack; should put everything necessary in this package */ -#include "HsBase.h" -#include "Rts.h" - #include "runProcess.h" +#include "common.h" -#include "execvpe.h" +#include +#include +#include -/* ---------------------------------------------------------------------------- - UNIX versions - ------------------------------------------------------------------------- */ +#ifdef HAVE_FCNTL_H +#include +#endif +#if defined(HAVE_SIGNAL_H) +#include +#endif + +int +get_max_fd() +{ + static int cache = 0; + if (cache == 0) { +#if HAVE_SYSCONF + cache = sysconf(_SC_OPEN_MAX); + if (cache == -1) { + cache = 256; + } +#else + cache = 256; +#endif + } + return cache; +} + // If a process was terminated by a signal, the exit status we return // via the System.Process API is (-signum). This encoding avoids collision with // normal process termination status codes. See also #7229. #define TERMSIG_EXITSTATUS(s) (-(WTERMSIG(s))) -static long max_fd = 0; - -// Rts internal API, not exposed in a public header file: -extern void blockUserSignals(void); -extern void unblockUserSignals(void); - -// These are arbitrarily chosen -- JP -#define forkSetgidFailed 124 -#define forkSetuidFailed 125 +/* + * Spawn a new process. We first try posix_spawn but since this isn't supported + * on all platforms we fall back to fork/exec in some cases on some platforms. + */ +static ProcHandle +do_spawn (char *const args[], + char *workingDirectory, char **environment, + struct std_handle *stdInHdl, + struct std_handle *stdOutHdl, + struct std_handle *stdErrHdl, + gid_t *childGroup, uid_t *childUser, + int flags, + char **failed_doing) +{ + ProcHandle r; + r = do_spawn_posix(args, + workingDirectory, environment, + stdInHdl, stdOutHdl, stdErrHdl, + childGroup, childUser, + flags, + failed_doing); + if (r == -2) { + // configuration not supported by posix_spawn, fall back to fork/exec + } else { + return r; + } -// See #1593. The convention for the exit code when -// exec() fails seems to be 127 (gleened from C's -// system()), but there's no equivalent convention for -// chdir(), so I'm picking 126 --SimonM. -#define forkChdirFailed 126 -#define forkExecFailed 127 + r = do_spawn_fork(args, + workingDirectory, environment, + stdInHdl, stdOutHdl, stdErrHdl, + childGroup, childUser, + flags, + failed_doing); + return r; +} -#define forkGetpwuidFailed 128 -#define forkInitgroupsFailed 129 +enum pipe_direction { + CHILD_READS, CHILD_WRITES +}; -__attribute__((__noreturn__)) -static void childFailed(int pipe, int failCode) { - int err; - ssize_t unused __attribute__((unused)); +/* Initialize a std_handle_behavior from a "pseudo-fd": + * - fd == -1 means create a pipe + * - fd == -2 means close the handle + * - otherwise use fd + * Returns 0 on success, -1 otherwise. + */ +static int +init_std_handle(int fd, enum pipe_direction direction, + /* out */ struct std_handle *hdl, + char **failed_doing) +{ + switch (fd) { + case -1: { + int pipe_fds[2]; + int r = pipe(pipe_fds); + if (r == -1) { + *failed_doing = "pipe"; + return -1; + } - err = errno; - unused = write(pipe, &failCode, sizeof(failCode)); - unused = write(pipe, &err, sizeof(err)); - // As a fallback, exit with the failCode - _exit(failCode); + int child_end = direction == CHILD_READS ? 0 : 1; + int parent_end = direction == CHILD_READS ? 1 : 0; + *hdl = (struct std_handle) { + .behavior = STD_HANDLE_USE_PIPE, + .use_pipe = { .child_end = pipe_fds[child_end], .parent_end = pipe_fds[parent_end] } + }; + break; + } + case -2: + *hdl = (struct std_handle) { + .behavior = STD_HANDLE_CLOSE + }; + break; + default: + *hdl = (struct std_handle) { + .behavior = STD_HANDLE_USE_FD, + .use_fd = fd + }; + break; + } + return 0; } ProcHandle runInteractiveProcess (char *const args[], char *workingDirectory, char **environment, + // handles to use for the standard handles. -1 indicates + // create pipe, -2 indicates close. int fdStdIn, int fdStdOut, int fdStdErr, + // output arguments to return created pipe handle to caller int *pfdStdInput, int *pfdStdOutput, int *pfdStdError, gid_t *childGroup, uid_t *childUser, - int reset_int_quit_handlers, int flags, char **failed_doing) { - int close_fds = ((flags & RUN_PROCESS_IN_CLOSE_FDS) != 0); - int pid; - int fdStdInput[2], fdStdOutput[2], fdStdError[2]; - int forkCommunicationFds[2]; - int r; - int failCode, err; + struct std_handle stdInHdl, stdOutHdl, stdErrHdl; + ProcHandle r; - // Ordering matters here, see below [Note #431]. - if (fdStdIn == -1) { - r = pipe(fdStdInput); - if (r == -1) { - *failed_doing = "runInteractiveProcess: pipe"; - return -1; - } - } - if (fdStdOut == -1) { - r = pipe(fdStdOutput); - if (r == -1) { - if (fdStdIn == -1) { - close(fdStdInput[0]); - close(fdStdInput[1]); - } - *failed_doing = "runInteractiveProcess: pipe"; - return -1; - } - } - if (fdStdErr == -1) { - r = pipe(fdStdError); - if (r == -1) { - *failed_doing = "runInteractiveProcess: pipe"; - if (fdStdIn == -1) { - close(fdStdInput[0]); - close(fdStdInput[1]); - } - if (fdStdOut == -1) { - close(fdStdOutput[0]); - close(fdStdOutput[1]); - } - return -1; - } - } + // A bit of paranoia to ensure that we catch if we fail to set this on + // failure. + *failed_doing = NULL; - r = pipe(forkCommunicationFds); - if (r == -1) { - *failed_doing = "runInteractiveProcess: pipe"; - if (fdStdIn == -1) { - close(fdStdInput[0]); - close(fdStdInput[1]); - } - if (fdStdOut == -1) { - close(fdStdOutput[0]); - close(fdStdOutput[1]); - } - if (fdStdErr == -1) { - close(fdStdError[0]); - close(fdStdError[1]); - } - return -1; + // Ordering matters here, see below Note [Ordering of handle closing]. + if (init_std_handle(fdStdIn, CHILD_READS, &stdInHdl, failed_doing) != 0) { + goto fail; } - // Block signals with Haskell handlers. The danger here is that - // with the threaded RTS, a signal arrives in the child process, - // the RTS writes the signal information into the pipe (which is - // shared between parent and child), and the parent behaves as if - // the signal had been raised. - blockUserSignals(); - - // See #4074. Sometimes fork() gets interrupted by the timer - // signal and keeps restarting indefinitely. - stopTimer(); - - switch(pid = myfork()) - { - case -1: - unblockUserSignals(); - startTimer(); - if (fdStdIn == -1) { - close(fdStdInput[0]); - close(fdStdInput[1]); - } - if (fdStdOut == -1) { - close(fdStdOutput[0]); - close(fdStdOutput[1]); - } - if (fdStdErr == -1) { - close(fdStdError[0]); - close(fdStdError[1]); - } - close(forkCommunicationFds[0]); - close(forkCommunicationFds[1]); - *failed_doing = "fork"; - return -1; - - case 0: - // WARNING! We may now be in the child of vfork(), and any - // memory we modify below may also be seen in the parent - // process. - - close(forkCommunicationFds[0]); - fcntl(forkCommunicationFds[1], F_SETFD, FD_CLOEXEC); - - if ((flags & RUN_PROCESS_NEW_SESSION) != 0) { - setsid(); - } - if ((flags & RUN_PROCESS_IN_NEW_GROUP) != 0) { - setpgid(0, 0); - } - - if ( childGroup) { - if ( setgid( *childGroup) != 0) { - // ERROR - childFailed(forkCommunicationFds[1], forkSetgidFailed); - } - } - - if ( childUser) { - // Using setuid properly first requires that we initgroups. - // However, to do this we must know the username of the user we are - // switching to. - struct passwd pw; - struct passwd *res = NULL; - int buf_len = sysconf(_SC_GETPW_R_SIZE_MAX); - char *buf = malloc(buf_len); - gid_t suppl_gid = childGroup ? *childGroup : getgid(); - if ( getpwuid_r(*childUser, &pw, buf, buf_len, &res) != 0) { - childFailed(forkCommunicationFds[1], forkGetpwuidFailed); - } - if ( res == NULL ) { - childFailed(forkCommunicationFds[1], forkGetpwuidFailed); - } - if ( initgroups(res->pw_name, suppl_gid) != 0) { - childFailed(forkCommunicationFds[1], forkInitgroupsFailed); - } - if ( setuid( *childUser) != 0) { - // ERROR - childFailed(forkCommunicationFds[1], forkSetuidFailed); - } - } - - unblockUserSignals(); - - if (workingDirectory) { - if (chdir (workingDirectory) < 0) { - childFailed(forkCommunicationFds[1], forkChdirFailed); - } - } - - // [Note #431]: Ordering matters here. If any of the FDs - // 0,1,2 were initially closed, then our pipes may have used - // these FDs. So when we dup2 the pipe FDs down to 0,1,2, we - // must do it in that order, otherwise we could overwrite an - // FD that we need later. - - if (fdStdIn == -1) { - if (fdStdInput[0] != STDIN_FILENO) { - dup2 (fdStdInput[0], STDIN_FILENO); - close(fdStdInput[0]); - } - close(fdStdInput[1]); - } else if (fdStdIn == -2) { - close(STDIN_FILENO); - } else { - dup2(fdStdIn, STDIN_FILENO); - } - - if (fdStdOut == -1) { - if (fdStdOutput[1] != STDOUT_FILENO) { - dup2 (fdStdOutput[1], STDOUT_FILENO); - close(fdStdOutput[1]); - } - close(fdStdOutput[0]); - } else if (fdStdOut == -2) { - close(STDOUT_FILENO); - } else { - dup2(fdStdOut, STDOUT_FILENO); - } - - if (fdStdErr == -1) { - if (fdStdError[1] != STDERR_FILENO) { - dup2 (fdStdError[1], STDERR_FILENO); - close(fdStdError[1]); - } - close(fdStdError[0]); - } else if (fdStdErr == -2) { - close(STDERR_FILENO); - } else { - dup2(fdStdErr, STDERR_FILENO); - } - - if (close_fds) { - int i; - if (max_fd == 0) { -#if HAVE_SYSCONF - max_fd = sysconf(_SC_OPEN_MAX); - if (max_fd == -1) { - max_fd = 256; - } -#else - max_fd = 256; -#endif - } - // XXX Not the pipe - for (i = 3; i < max_fd; i++) { - if (i != forkCommunicationFds[1]) { - close(i); - } - } - } - - /* Reset the SIGINT/SIGQUIT signal handlers in the child, if requested - */ - if (reset_int_quit_handlers) { - struct sigaction dfl; - (void)sigemptyset(&dfl.sa_mask); - dfl.sa_flags = 0; - dfl.sa_handler = SIG_DFL; - (void)sigaction(SIGINT, &dfl, NULL); - (void)sigaction(SIGQUIT, &dfl, NULL); - } - - /* the child */ - if (environment) { - // XXX Check result - execvpe(args[0], args, environment); - } else { - // XXX Check result - execvp(args[0], args); - } - - childFailed(forkCommunicationFds[1], forkExecFailed); - - default: - if ((flags & RUN_PROCESS_IN_NEW_GROUP) != 0) { - setpgid(pid, pid); - } - if (fdStdIn == -1) { - close(fdStdInput[0]); - fcntl(fdStdInput[1], F_SETFD, FD_CLOEXEC); - *pfdStdInput = fdStdInput[1]; - } - if (fdStdOut == -1) { - close(fdStdOutput[1]); - fcntl(fdStdOutput[0], F_SETFD, FD_CLOEXEC); - *pfdStdOutput = fdStdOutput[0]; - } - if (fdStdErr == -1) { - close(fdStdError[1]); - fcntl(fdStdError[0], F_SETFD, FD_CLOEXEC); - *pfdStdError = fdStdError[0]; - } - close(forkCommunicationFds[1]); - fcntl(forkCommunicationFds[0], F_SETFD, FD_CLOEXEC); - - break; + if (init_std_handle(fdStdOut, CHILD_WRITES, &stdOutHdl, failed_doing) != 0) { + goto fail; } - // If the child process had a problem, then it will tell us via the - // forkCommunicationFds pipe. First we try to read what the problem - // was. Note that if none of these conditionals match then we fall - // through and just return pid. - r = read(forkCommunicationFds[0], &failCode, sizeof(failCode)); - if (r == -1) { - *failed_doing = "runInteractiveProcess: read pipe"; - pid = -1; + if (init_std_handle(fdStdErr, CHILD_WRITES, &stdErrHdl, failed_doing) != 0) { + goto fail; } - else if (r == sizeof(failCode)) { - // This is the case where we successfully managed to read - // the problem - switch (failCode) { - case forkChdirFailed: - *failed_doing = "runInteractiveProcess: chdir"; - break; - case forkExecFailed: - *failed_doing = "runInteractiveProcess: exec"; - break; - case forkSetgidFailed: - *failed_doing = "runInteractiveProcess: setgid"; - break; - case forkSetuidFailed: - *failed_doing = "runInteractiveProcess: setuid"; - break; - case forkGetpwuidFailed: - *failed_doing = "runInteractiveProcess: getpwuid"; - break; - case forkInitgroupsFailed: - *failed_doing = "runInteractiveProcess: initgroups"; - break; - default: - *failed_doing = "runInteractiveProcess: unknown"; - break; - } - // Now we try to get the errno from the child - r = read(forkCommunicationFds[0], &err, sizeof(err)); - if (r == -1) { - *failed_doing = "runInteractiveProcess: read pipe"; - } - else if (r != sizeof(failCode)) { - *failed_doing = "runInteractiveProcess: read pipe bad length"; - } - else { - // If we succeed then we set errno. It'll be saved and - // restored again below. Note that in any other case we'll - // get the errno of whatever else went wrong instead. - errno = err; - } - // We forked the child, but the child had a problem and stopped so it's - // our responsibility to reap here as nobody else can. - waitpid(pid, NULL, 0); - - if (fdStdIn == -1) { - // Already closed fdStdInput[0] above - close(fdStdInput[1]); - } - if (fdStdOut == -1) { - close(fdStdOutput[0]); - // Already closed fdStdOutput[1] above - } - if (fdStdErr == -1) { - close(fdStdError[0]); - // Already closed fdStdError[1] above - } - - pid = -1; - } - else if (r != 0) { - *failed_doing = "runInteractiveProcess: read pipe bad length"; - pid = -1; + r = do_spawn(args, + workingDirectory, environment, + &stdInHdl, &stdOutHdl, &stdErrHdl, + childGroup, childUser, + flags, + failed_doing); + if (r == -1) { + goto fail; } - if (pid == -1) { - err = errno; + // Close the remote ends of any pipes we created. +#define FINALISE_STD_HANDLE(hdl, pfd) \ + if (hdl.behavior == STD_HANDLE_USE_PIPE) { \ + close(hdl.use_pipe.child_end); \ + fcntl(hdl.use_pipe.parent_end, F_SETFD, FD_CLOEXEC); \ + *pfd = hdl.use_pipe.parent_end; \ } - close(forkCommunicationFds[0]); + FINALISE_STD_HANDLE(stdInHdl, pfdStdInput); + FINALISE_STD_HANDLE(stdOutHdl, pfdStdOutput); + FINALISE_STD_HANDLE(stdErrHdl, pfdStdError); +#undef FINALISE_STD_HANDLE - unblockUserSignals(); - startTimer(); + return r; - if (pid == -1) { - errno = err; +fail: +#define CLOSE_PIPE(hdl) \ + if (hdl.behavior == STD_HANDLE_USE_PIPE) { \ + close(hdl.use_pipe.child_end); \ + close(hdl.use_pipe.parent_end); \ } - return pid; + CLOSE_PIPE(stdInHdl); + CLOSE_PIPE(stdOutHdl); + CLOSE_PIPE(stdErrHdl); +#undef CLOSE_PIPE + + return -1; } int @@ -422,14 +209,11 @@ *pExitCode = 0; - if ((res = waitpid(handle, &wstat, WNOHANG)) > 0) - { - if (WIFEXITED(wstat)) - { + if ((res = waitpid(handle, &wstat, WNOHANG)) > 0) { + if (WIFEXITED(wstat)) { *pExitCode = WEXITSTATUS(wstat); return 1; - } - else + } else { if (WIFSIGNALED(wstat)) { *pExitCode = TERMSIG_EXITSTATUS(wstat); @@ -439,6 +223,7 @@ { /* This should never happen */ } + } } if (res == 0) return 0; @@ -452,7 +237,8 @@ return -1; } -int waitForProcess (ProcHandle handle, int *pret) +int +waitForProcess (ProcHandle handle, int *pret) { int wstat; diff --git a/cbits/win32/runProcess.c b/cbits/win32/runProcess.c --- a/cbits/win32/runProcess.c +++ b/cbits/win32/runProcess.c @@ -553,6 +553,10 @@ if (pid_list == NULL) { pid_list = malloc(pid_list_size); + if (pid_list == NULL) { + errno = ENOMEM; + return false; + } pid_list->NumberOfAssignedProcesses = process_count; } diff --git a/changelog.md b/changelog.md --- a/changelog.md +++ b/changelog.md @@ -1,5 +1,9 @@ # Changelog for [`process` package](http://hackage.haskell.org/package/process) +## 1.6.13.0 *July 2021* + +* Refactoring of POSIX process logic [#208](https://github.com/haskell/process/pull/208) + ## 1.6.12.0 *June 2021* * Add function `getCurrentPid` to get the currently executing process' ID [#205](https://github.com/haskell/process/pull/205) diff --git a/configure b/configure --- a/configure +++ b/configure @@ -1798,6 +1798,52 @@ } # ac_fn_c_check_func +# ac_fn_c_check_decl LINENO SYMBOL VAR INCLUDES +# --------------------------------------------- +# Tests whether SYMBOL is declared in INCLUDES, setting cache variable VAR +# accordingly. +ac_fn_c_check_decl () +{ + as_lineno=${as_lineno-"$1"} as_lineno_stack=as_lineno_stack=$as_lineno_stack + as_decl_name=`echo $2|sed 's/ *(.*//'` + as_decl_use=`echo $2|sed -e 's/(/((/' -e 's/)/) 0&/' -e 's/,/) 0& (/g'` + { $as_echo "$as_me:${as_lineno-$LINENO}: checking whether $as_decl_name is declared" >&5 +$as_echo_n "checking whether $as_decl_name is declared... " >&6; } +if eval \${$3+:} false; then : + $as_echo_n "(cached) " >&6 +else + cat confdefs.h - <<_ACEOF >conftest.$ac_ext +/* end confdefs.h. */ +$4 +int +main () +{ +#ifndef $as_decl_name +#ifdef __cplusplus + (void) $as_decl_use; +#else + (void) $as_decl_name; +#endif +#endif + + ; + return 0; +} +_ACEOF +if ac_fn_c_try_compile "$LINENO"; then : + eval "$3=yes" +else + eval "$3=no" +fi +rm -f core conftest.err conftest.$ac_objext conftest.$ac_ext +fi +eval ac_res=\$$3 + { $as_echo "$as_me:${as_lineno-$LINENO}: result: $ac_res" >&5 +$as_echo "$ac_res" >&6; } + eval $as_lineno_stack; ${as_lineno_stack:+:} unset as_lineno + +} # ac_fn_c_check_decl + # ac_fn_c_compute_int LINENO EXPR VAR INCLUDES # -------------------------------------------- # Tries to find the compile-time value of EXPR in a program that includes @@ -3778,6 +3824,75 @@ fi done + +for ac_func in execvpe +do : + ac_fn_c_check_func "$LINENO" "execvpe" "ac_cv_func_execvpe" +if test "x$ac_cv_func_execvpe" = xyes; then : + cat >>confdefs.h <<_ACEOF +#define HAVE_EXECVPE 1 +_ACEOF + +fi +done + + +# posix_spawn checks +for ac_header in spawn.h +do : + ac_fn_c_check_header_mongrel "$LINENO" "spawn.h" "ac_cv_header_spawn_h" "$ac_includes_default" +if test "x$ac_cv_header_spawn_h" = xyes; then : + cat >>confdefs.h <<_ACEOF +#define HAVE_SPAWN_H 1 +_ACEOF + +fi + +done + +for ac_func in posix_spawnp posix_spawn_file_actions_addchdir +do : + as_ac_var=`$as_echo "ac_cv_func_$ac_func" | $as_tr_sh` +ac_fn_c_check_func "$LINENO" "$ac_func" "$as_ac_var" +if eval test \"x\$"$as_ac_var"\" = x"yes"; then : + cat >>confdefs.h <<_ACEOF +#define `$as_echo "HAVE_$ac_func" | $as_tr_cpp` 1 +_ACEOF + +fi +done + +ac_fn_c_check_decl "$LINENO" "POSIX_SPAWN_SETSID" "ac_cv_have_decl_POSIX_SPAWN_SETSID" "$ac_includes_default" +if test "x$ac_cv_have_decl_POSIX_SPAWN_SETSID" = xyes; then : + ac_have_decl=1 +else + ac_have_decl=0 +fi + +cat >>confdefs.h <<_ACEOF +#define HAVE_DECL_POSIX_SPAWN_SETSID $ac_have_decl +_ACEOF +ac_fn_c_check_decl "$LINENO" "POSIX_SPAWN_SETSID_NP" "ac_cv_have_decl_POSIX_SPAWN_SETSID_NP" "$ac_includes_default" +if test "x$ac_cv_have_decl_POSIX_SPAWN_SETSID_NP" = xyes; then : + ac_have_decl=1 +else + ac_have_decl=0 +fi + +cat >>confdefs.h <<_ACEOF +#define HAVE_DECL_POSIX_SPAWN_SETSID_NP $ac_have_decl +_ACEOF + +ac_fn_c_check_decl "$LINENO" "POSIX_SPAWN_SETPGROUP" "ac_cv_have_decl_POSIX_SPAWN_SETPGROUP" "$ac_includes_default" +if test "x$ac_cv_have_decl_POSIX_SPAWN_SETPGROUP" = xyes; then : + ac_have_decl=1 +else + ac_have_decl=0 +fi + +cat >>confdefs.h <<_ACEOF +#define HAVE_DECL_POSIX_SPAWN_SETPGROUP $ac_have_decl +_ACEOF for fp_const_name in SIG_DFL SIG_IGN diff --git a/configure.ac b/configure.ac --- a/configure.ac +++ b/configure.ac @@ -14,6 +14,13 @@ AC_CHECK_HEADERS([signal.h sys/wait.h fcntl.h]) AC_CHECK_FUNCS([setitimer sysconf]) +AC_CHECK_FUNCS([execvpe]) + +# posix_spawn checks +AC_CHECK_HEADERS([spawn.h]) +AC_CHECK_FUNCS([posix_spawnp posix_spawn_file_actions_addchdir]) +AC_CHECK_DECLS([POSIX_SPAWN_SETSID, POSIX_SPAWN_SETSID_NP]) +AC_CHECK_DECLS([POSIX_SPAWN_SETPGROUP]) FP_CHECK_CONSTS([SIG_DFL SIG_IGN]) diff --git a/include/HsProcessConfig.h.in b/include/HsProcessConfig.h.in --- a/include/HsProcessConfig.h.in +++ b/include/HsProcessConfig.h.in @@ -6,6 +6,21 @@ /* The value of SIG_IGN. */ #undef CONST_SIG_IGN +/* Define to 1 if you have the declaration of `POSIX_SPAWN_SETPGROUP', and to + 0 if you don't. */ +#undef HAVE_DECL_POSIX_SPAWN_SETPGROUP + +/* Define to 1 if you have the declaration of `POSIX_SPAWN_SETSID', and to 0 + if you don't. */ +#undef HAVE_DECL_POSIX_SPAWN_SETSID + +/* Define to 1 if you have the declaration of `POSIX_SPAWN_SETSID_NP', and to + 0 if you don't. */ +#undef HAVE_DECL_POSIX_SPAWN_SETSID_NP + +/* Define to 1 if you have the `execvpe' function. */ +#undef HAVE_EXECVPE + /* Define to 1 if you have the header file. */ #undef HAVE_FCNTL_H @@ -18,11 +33,21 @@ /* Define to 1 if you have the header file. */ #undef HAVE_MEMORY_H +/* Define to 1 if you have the `posix_spawnp' function. */ +#undef HAVE_POSIX_SPAWNP + +/* Define to 1 if you have the `posix_spawn_file_actions_addchdir' function. + */ +#undef HAVE_POSIX_SPAWN_FILE_ACTIONS_ADDCHDIR + /* Define to 1 if you have the `setitimer' function. */ #undef HAVE_SETITIMER /* Define to 1 if you have the header file. */ #undef HAVE_SIGNAL_H + +/* Define to 1 if you have the header file. */ +#undef HAVE_SPAWN_H /* Define to 1 if you have the header file. */ #undef HAVE_STDINT_H diff --git a/include/processFlags.h b/include/processFlags.h --- a/include/processFlags.h +++ b/include/processFlags.h @@ -9,3 +9,4 @@ #define RUN_PROCESS_DETACHED 0x4 #define RUN_PROCESS_NEW_SESSION 0x8 #define RUN_PROCESS_NEW_CONSOLE 0x10 +#define RESET_INT_QUIT_HANDLERS 0x20 diff --git a/include/runProcess.h b/include/runProcess.h --- a/include/runProcess.h +++ b/include/runProcess.h @@ -4,6 +4,8 @@ Interface for code in runProcess.c (providing support for System.Process) ------------------------------------------------------------------------- */ +#pragma once + #include "HsProcessConfig.h" // Otherwise these clash with similar definitions from other packages: #undef PACKAGE_BUGREPORT @@ -19,35 +21,8 @@ #include #endif -#include -#include #if !(defined(_MSC_VER) || defined(__MINGW32__) || defined(_WIN32)) -#include -#include -#endif - -#ifdef HAVE_FCNTL_H -#include -#endif - -#ifdef HAVE_VFORK_H -#include -#endif - -#if defined(HAVE_WORKING_VFORK) -#define myfork vfork -#elif defined(HAVE_WORKING_FORK) -#define myfork fork -// We don't need a fork command on Windows -#elif !(defined(_MSC_VER) || defined(__MINGW32__) || defined(_WIN32)) -#error Cannot find a working fork command -#endif - -#ifdef HAVE_SIGNAL_H -#include -#endif - -#if !(defined(_MSC_VER) || defined(__MINGW32__) || defined(_WIN32)) +#include typedef pid_t ProcHandle; #else // Should really be intptr_t, but we don't have that type on the Haskell side @@ -58,6 +33,9 @@ #if !(defined(_MSC_VER) || defined(__MINGW32__) || defined(_WIN32)) +#include +#include + extern ProcHandle runInteractiveProcess( char *const args[], char *workingDirectory, char **environment, @@ -69,7 +47,6 @@ int *pfdStdError, gid_t *childGroup, uid_t *childUser, - int reset_int_quit_handlers, int flags, char **failed_doing); diff --git a/process.cabal b/process.cabal --- a/process.cabal +++ b/process.cabal @@ -1,5 +1,5 @@ name: process -version: 1.6.12.0 +version: 1.6.13.0 -- NOTE: Don't forget to update ./changelog.md license: BSD3 license-file: LICENSE @@ -65,6 +65,9 @@ else c-sources: cbits/posix/runProcess.c + cbits/posix/fork_exec.c + cbits/posix/posix_spawn.c + cbits/posix/find_executable.c other-modules: System.Process.Posix build-depends: unix >= 2.5 && < 2.8 diff --git a/test/main.hs b/test/main.hs --- a/test/main.hs +++ b/test/main.hs @@ -14,6 +14,11 @@ import qualified Data.ByteString.Char8 as S8 import System.Directory (getTemporaryDirectory, removeFile) +ifWindows :: IO () -> IO () +ifWindows action + | not isWindows = return () + | otherwise = action + isWindows :: Bool #if WINDOWS isWindows = True @@ -51,7 +56,7 @@ putStrLn "Testing subdirectories" - withCurrentDirectory "exes" $ do + ifWindows $ withCurrentDirectory "exes" $ do res1 <- readCreateProcess (proc "./echo.bat" []) "" unless ("parent" `isInfixOf` res1 && not ("child" `isInfixOf` res1)) $ error $ "echo.bat with cwd failed: " ++ show res1