diff --git a/osdep/subprocess-posix.c b/osdep/subprocess-posix.c index ff78eb42f8..8165f7f641 100644 --- a/osdep/subprocess-posix.c +++ b/osdep/subprocess-posix.c @@ -35,45 +35,34 @@ extern char **environ; #define SAFE_CLOSE(fd) do { if ((fd) >= 0) close((fd)); (fd) = -1; } while (0) -// A silly helper: automatically skips entries with negative FDs -static int sparse_poll(struct pollfd *fds, int num_fds, int timeout) -{ - struct pollfd p_fds[10]; - int map[10]; - if (num_fds > MP_ARRAY_SIZE(p_fds)) - return -1; - int p_num_fds = 0; - for (int n = 0; n < num_fds; n++) { - map[n] = -1; - if (fds[n].fd < 0) - continue; - map[n] = p_num_fds; - p_fds[p_num_fds++] = fds[n]; - } - int r = poll(p_fds, p_num_fds, timeout); - for (int n = 0; n < num_fds; n++) - fds[n].revents = (map[n] < 0 && r >= 0) ? 0 : p_fds[map[n]].revents; - return r; -} - -int mp_subprocess(char **args, struct mp_cancel *cancel, void *ctx, - subprocess_read_cb on_stdout, subprocess_read_cb on_stderr, - char **error) +void mp_subprocess2(struct mp_subprocess_opts *opts, + struct mp_subprocess_result *res) { posix_spawn_file_actions_t fa; bool fa_destroy = false; int status = -1; - int p_stdout[2] = {-1, -1}; - int p_stderr[2] = {-1, -1}; + int comm_pipe[MP_SUBPROCESS_MAX_FDS][2]; int devnull = -1; pid_t pid = -1; bool spawned = false; bool killed_by_us = false; + int cancel_fd = -1; - if (on_stdout && mp_make_cloexec_pipe(p_stdout) < 0) - goto done; - if (on_stderr && mp_make_cloexec_pipe(p_stderr) < 0) - goto done; + *res = (struct mp_subprocess_result){0}; + + for (int n = 0; n < opts->num_fds; n++) + comm_pipe[n][0] = comm_pipe[n][1] = -1; + + if (opts->cancel) { + cancel_fd = mp_cancel_get_fd(opts->cancel); + if (cancel_fd < 0) + goto done; + } + + for (int n = 0; n < opts->num_fds; n++) { + if (opts->fds[n].on_read && mp_make_cloexec_pipe(comm_pipe[n]) < 0) + goto done; + } devnull = open("/dev/null", O_RDONLY | O_CLOEXEC); if (devnull < 0) @@ -82,79 +71,97 @@ int mp_subprocess(char **args, struct mp_cancel *cancel, void *ctx, if (posix_spawn_file_actions_init(&fa)) goto done; fa_destroy = true; - // redirect stdin/stdout/stderr - if (posix_spawn_file_actions_adddup2(&fa, devnull, 0)) - goto done; - if (p_stdout[1] >= 0 && posix_spawn_file_actions_adddup2(&fa, p_stdout[1], 1)) - goto done; - if (p_stderr[1] >= 0 && posix_spawn_file_actions_adddup2(&fa, p_stderr[1], 2)) - goto done; - if (posix_spawnp(&pid, args[0], &fa, NULL, args, environ)) { + // redirect FDs + for (int n = 0; n < opts->num_fds; n++) { + int src_fd = devnull; + if (comm_pipe[n][1] >= 0) + src_fd = comm_pipe[n][1]; + if (opts->fds[n].src_fd >= 0) + src_fd = opts->fds[n].src_fd; + if (posix_spawn_file_actions_adddup2(&fa, src_fd, opts->fds[n].fd)) + goto done; + } + + char **env = opts->env ? opts->env : environ; + if (posix_spawnp(&pid, opts->exe, &fa, NULL, opts->args, env)) { pid = -1; goto done; } spawned = true; - SAFE_CLOSE(p_stdout[1]); - SAFE_CLOSE(p_stderr[1]); + for (int n = 0; n < opts->num_fds; n++) + SAFE_CLOSE(comm_pipe[n][1]); SAFE_CLOSE(devnull); - int *read_fds[2] = {&p_stdout[0], &p_stderr[0]}; - subprocess_read_cb read_cbs[2] = {on_stdout, on_stderr}; - - while (p_stdout[0] >= 0 || p_stderr[0] >= 0) { - struct pollfd fds[] = { - {.events = POLLIN, .fd = *read_fds[0]}, - {.events = POLLIN, .fd = *read_fds[1]}, - {.events = POLLIN, .fd = cancel ? mp_cancel_get_fd(cancel) : -1}, - }; - if (sparse_poll(fds, MP_ARRAY_SIZE(fds), -1) < 0 && errno != EINTR) - break; - for (int n = 0; n < 2; n++) { - if (fds[n].revents) { - char buf[4096]; - ssize_t r = read(*read_fds[n], buf, sizeof(buf)); - if (r < 0 && errno == EINTR) - continue; - if (r > 0 && read_cbs[n]) - read_cbs[n](ctx, buf, r); - if (r <= 0) - SAFE_CLOSE(*read_fds[n]); + while (1) { + struct pollfd fds[MP_SUBPROCESS_MAX_FDS + 1]; + int map_fds[MP_SUBPROCESS_MAX_FDS + 1]; + int num_fds = 0; + for (int n = 0; n < opts->num_fds; n++) { + if (comm_pipe[n][0] >= 0) { + map_fds[num_fds] = n; + fds[num_fds++] = (struct pollfd){ + .events = POLLIN, + .fd = comm_pipe[n][0], + }; } } - if (fds[2].revents) { - kill(pid, SIGKILL); - killed_by_us = true; + if (!num_fds) break; + if (cancel_fd >= 0) { + map_fds[num_fds] = -1; + fds[num_fds++] = (struct pollfd){.events = POLLIN, .fd = cancel_fd}; + } + + if (poll(fds, num_fds, -1) < 0 && errno != EINTR) + break; + + for (int idx = 0; idx < num_fds; idx++) { + if (fds[idx].revents) { + int n = map_fds[idx]; + if (n < 0) { + // cancel_fd + kill(pid, SIGKILL); + killed_by_us = true; + break; + } else { + char buf[4096]; + ssize_t r = read(comm_pipe[n][0], buf, sizeof(buf)); + if (r < 0 && errno == EINTR) + continue; + if (r > 0 && opts->fds[n].on_read) + opts->fds[n].on_read(opts->fds[n].on_read_ctx, buf, r); + if (r <= 0) + SAFE_CLOSE(comm_pipe[n][0]); + } + } } } // Note: it can happen that a child process closes the pipe, but does not // terminate yet. In this case, we would have to run waitpid() in // a separate thread and use pthread_cancel(), or use other weird - // and laborious tricks. So this isn't handled yet. + // and laborious tricks in order to react to mp_cancel. + // So this isn't handled yet. while (waitpid(pid, &status, 0) < 0 && errno == EINTR) {} done: if (fa_destroy) posix_spawn_file_actions_destroy(&fa); - SAFE_CLOSE(p_stdout[0]); - SAFE_CLOSE(p_stdout[1]); - SAFE_CLOSE(p_stderr[0]); - SAFE_CLOSE(p_stderr[1]); + for (int n = 0; n < opts->num_fds; n++) { + SAFE_CLOSE(comm_pipe[n][0]); + SAFE_CLOSE(comm_pipe[n][1]); + } SAFE_CLOSE(devnull); if (!spawned || (WIFEXITED(status) && WEXITSTATUS(status) == 127)) { - *error = "init"; - status = -1; + res->error = MP_SUBPROCESS_EINIT; } else if (WIFEXITED(status)) { - *error = NULL; - status = WEXITSTATUS(status); + res->exit_status = WEXITSTATUS(status); + } else if (killed_by_us) { + res->error = MP_SUBPROCESS_EKILLED_BY_US; } else { - *error = "killed"; - status = killed_by_us ? MP_SUBPROCESS_EKILLED_BY_US : -1; + res->error = MP_SUBPROCESS_EGENERIC; } - - return status; } diff --git a/osdep/subprocess.c b/osdep/subprocess.c index 1e94d23b67..9da5c10c1c 100644 --- a/osdep/subprocess.c +++ b/osdep/subprocess.c @@ -25,6 +25,48 @@ #include "subprocess.h" +void mp_devnull(void *ctx, char *data, size_t size) +{ +} + +#if HAVE_POSIX_SPAWN + +int mp_subprocess(char **args, struct mp_cancel *cancel, void *ctx, + subprocess_read_cb on_stdout, subprocess_read_cb on_stderr, + char **error) +{ + struct mp_subprocess_opts opts = { + .exe = args[0], + .args = args, + .cancel = cancel, + }; + opts.fds[opts.num_fds++] = (struct mp_subprocess_fd){ + .fd = 0, // stdin + .src_fd = 0, + }; + opts.fds[opts.num_fds++] = (struct mp_subprocess_fd){ + .fd = 1, // stdout + .on_read = on_stdout, + .on_read_ctx = ctx, + .src_fd = on_stdout ? -1 : 1, + }; + opts.fds[opts.num_fds++] = (struct mp_subprocess_fd){ + .fd = 2, // stderr + .on_read = on_stderr, + .on_read_ctx = ctx, + .src_fd = on_stderr ? -1 : 2, + }; + struct mp_subprocess_result res; + mp_subprocess2(&opts, &res); + if (res.error < 0) { + *error = (char *)mp_subprocess_err_str(res.error); + return res.error; + } + return res.exit_status; +} + +#endif + struct subprocess_args { struct mp_log *log; char **args; @@ -45,10 +87,6 @@ static void *run_subprocess(void *ptr) return NULL; } -void mp_devnull(void *ctx, char *data, size_t size) -{ -} - void mp_subprocess_detached(struct mp_log *log, char **args) { struct subprocess_args *p = talloc_zero(NULL, struct subprocess_args); @@ -61,3 +99,16 @@ void mp_subprocess_detached(struct mp_log *log, char **args) if (pthread_create(&thread, NULL, run_subprocess, p)) talloc_free(p); } + +const char *mp_subprocess_err_str(int num) +{ + // Note: these are visible to the public client API + switch (num) { + case MP_SUBPROCESS_OK: return "success"; + case MP_SUBPROCESS_EKILLED_BY_US: return "killed"; + case MP_SUBPROCESS_EINIT: return "init"; + case MP_SUBPROCESS_EUNSUPPORTED: return "unsupported"; + case MP_SUBPROCESS_EGENERIC: // fall through + default: return "unknown"; + } +} diff --git a/osdep/subprocess.h b/osdep/subprocess.h index f272e1ad42..6aa2981f1d 100644 --- a/osdep/subprocess.h +++ b/osdep/subprocess.h @@ -18,7 +18,9 @@ #ifndef MP_SUBPROCESS_H_ #define MP_SUBPROCESS_H_ +#include #include +#include struct mp_cancel; @@ -26,12 +28,54 @@ typedef void (*subprocess_read_cb)(void *ctx, char *data, size_t size); void mp_devnull(void *ctx, char *data, size_t size); +#define MP_SUBPROCESS_MAX_FDS 10 + +struct mp_subprocess_fd { + int fd; // target FD + + // Only one of on_read or src_fd can be set. If none are set, use /dev/null. + // Note: "neutral" initialization requires setting src_fd=-1. + subprocess_read_cb on_read; // if not NULL, serve reads + void *on_read_ctx; // for on_read(on_read_ctx, ...) + int src_fd; // if >=0, dup this FD to target FD +}; + +struct mp_subprocess_opts { + char *exe; // binary to execute (never non-NULL) + char **args; // argument list (NULL for none, otherwise NULL-terminated) + char **env; // if !NULL, set this as environment variable block + // Complete set of FDs passed down. All others are supposed to be closed. + struct mp_subprocess_fd fds[MP_SUBPROCESS_MAX_FDS]; + int num_fds; + struct mp_cancel *cancel; // if !NULL, asynchronous process abort (kills it) +}; + +struct mp_subprocess_result { + int error; // one of MP_SUBPROCESS_* (>0 on error) + // NB: if WIFEXITED applies, error==0, and this is WEXITSTATUS + // on win32, this can use the full 32 bit + uint32_t exit_status; // if error==0==MP_SUBPROCESS_OK, 0 otherwise +}; + +// Subprocess error values. +#define MP_SUBPROCESS_OK 0 // no error +#define MP_SUBPROCESS_EGENERIC -1 // unknown error +#define MP_SUBPROCESS_EKILLED_BY_US -2 // mp_cancel was triggered +#define MP_SUBPROCESS_EINIT -3 // error during initialization +#define MP_SUBPROCESS_EUNSUPPORTED -4 // API not supported + +// Turn MP_SUBPROCESS_* values into a static string. Never returns NULL. +const char *mp_subprocess_err_str(int num); + +// Caller must set *opts. +void mp_subprocess2(struct mp_subprocess_opts *opts, + struct mp_subprocess_result *res); + // Start a subprocess. Uses callbacks to read from stdout and stderr. +// Returns any of MP_SUBPROCESS_*, or a value >=0 for the process exir int mp_subprocess(char **args, struct mp_cancel *cancel, void *ctx, subprocess_read_cb on_stdout, subprocess_read_cb on_stderr, char **error); -// mp_subprocess return values. -1 is a generic error code. -#define MP_SUBPROCESS_EKILLED_BY_US -2 struct mp_log; void mp_subprocess_detached(struct mp_log *log, char **args);