summaryrefslogtreecommitdiffstats
diff options
context:
space:
mode:
authorwm4 <wm4@nowhere>2020-02-16 21:57:17 +0100
committerwm4 <wm4@nowhere>2020-02-16 22:08:48 +0100
commitf2c7c641b38d060a7eea52a92bc0239f1603bffb (patch)
treed6f6df3c7a8325edf6c22d45c846d022f50e57a7
parent92fee4ebc4852f023527dc64b28e5b10e4507053 (diff)
downloadmpv-f2c7c641b38d060a7eea52a92bc0239f1603bffb.tar.bz2
mpv-f2c7c641b38d060a7eea52a92bc0239f1603bffb.tar.xz
subprocess: implement proper detached processes on POSIX
The previous method for this sucked: for every launched detached process, it started a thread, which then would leak if the launched process didn't end before the player uninitialized. This was very racy (although I bet the race condition wouldn't trigger in a 100 years), and wasteful (threads aren't a cheap resource). Implement it for POSIX directly. posix_spawn() has no direct support for this, so we need to do it ourselves with fork(). We could probably do it without fork(), and attempt to collect the PID in another thread. But then we'd either have a waiting thread again, or we'd need to do an unsafe waitpid(-1, ...) call. (POSIX process management sucks so badly, how did they even manage this. Hopefully I'm just missing something, but I'm not.) So now we depend on both posix_spawn() _and_ fork(), isn't it fun? Also call setsid(), to essentially detach the child process from the terminal. (Otherwise it can receive various signals from the terminal, which is probably not what you want.) posix_spawn() adds POSIX_SPAWN_SETSID in newer POSIX releases, but we don't want to rely on this yet. The posix_spawnp() call is duplicated, but this is better than somehow trying to unify the code paths. Only somewhat tested, so enjoy the bugs.
-rw-r--r--osdep/subprocess-posix.c47
-rw-r--r--osdep/subprocess.c27
-rw-r--r--osdep/subprocess.h2
3 files changed, 66 insertions, 10 deletions
diff --git a/osdep/subprocess-posix.c b/osdep/subprocess-posix.c
index 8165f7f641..fb11179618 100644
--- a/osdep/subprocess-posix.c
+++ b/osdep/subprocess-posix.c
@@ -17,6 +17,7 @@
#include "osdep/posix-spawn.h"
#include <poll.h>
+#include <pthread.h>
#include <unistd.h>
#include <sys/types.h>
#include <sys/wait.h>
@@ -43,7 +44,7 @@ void mp_subprocess2(struct mp_subprocess_opts *opts,
int status = -1;
int comm_pipe[MP_SUBPROCESS_MAX_FDS][2];
int devnull = -1;
- pid_t pid = -1;
+ pid_t pid = 0;
bool spawned = false;
bool killed_by_us = false;
int cancel_fd = -1;
@@ -84,11 +85,35 @@ void mp_subprocess2(struct mp_subprocess_opts *opts,
}
char **env = opts->env ? opts->env : environ;
- if (posix_spawnp(&pid, opts->exe, &fa, NULL, opts->args, env)) {
- pid = -1;
- goto done;
+
+ if (opts->detach) {
+ // If we run it detached, we fork a child to start the process; then
+ // it exits immediately, letting PID 1 inherit it. So we don't need
+ // anything else to collect these child PIDs.
+ sigset_t sigmask, oldmask;
+ sigfillset(&sigmask);
+ pthread_sigmask(SIG_BLOCK, &sigmask, &oldmask);
+ pid_t fres = fork();
+ if (fres < 0)
+ goto done;
+ if (fres == 0) {
+ // child
+ setsid();
+ if (posix_spawnp(&pid, opts->exe, &fa, NULL, opts->args, env))
+ _exit(1);
+ _exit(0);
+ }
+ pthread_sigmask(SIG_SETMASK, &oldmask, NULL);
+ int child_status = 0;
+ while (waitpid(fres, &child_status, 0) < 0 && errno == EINTR) {}
+ if (!WIFEXITED(child_status) || WEXITSTATUS(child_status) != 0)
+ goto done;
+ spawned = true;
+ } else {
+ if (posix_spawnp(&pid, opts->exe, &fa, NULL, opts->args, env))
+ goto done;
+ spawned = true;
}
- spawned = true;
for (int n = 0; n < opts->num_fds; n++)
SAFE_CLOSE(comm_pipe[n][1]);
@@ -122,7 +147,8 @@ void mp_subprocess2(struct mp_subprocess_opts *opts,
int n = map_fds[idx];
if (n < 0) {
// cancel_fd
- kill(pid, SIGKILL);
+ if (pid)
+ kill(pid, SIGKILL);
killed_by_us = true;
break;
} else {
@@ -144,7 +170,8 @@ void mp_subprocess2(struct mp_subprocess_opts *opts,
// a separate thread and use pthread_cancel(), or use other weird
// and laborious tricks in order to react to mp_cancel.
// So this isn't handled yet.
- while (waitpid(pid, &status, 0) < 0 && errno == EINTR) {}
+ if (pid)
+ while (waitpid(pid, &status, 0) < 0 && errno == EINTR) {}
done:
if (fa_destroy)
@@ -155,10 +182,12 @@ done:
}
SAFE_CLOSE(devnull);
- if (!spawned || (WIFEXITED(status) && WEXITSTATUS(status) == 127)) {
+ if (!spawned || (pid && WIFEXITED(status) && WEXITSTATUS(status) == 127)) {
res->error = MP_SUBPROCESS_EINIT;
- } else if (WIFEXITED(status)) {
+ } else if (pid && WIFEXITED(status)) {
res->exit_status = WEXITSTATUS(status);
+ } else if (spawned && opts->detach) {
+ // ok
} else if (killed_by_us) {
res->error = MP_SUBPROCESS_EKILLED_BY_US;
} else {
diff --git a/osdep/subprocess.c b/osdep/subprocess.c
index 9da5c10c1c..8bb2acd0b7 100644
--- a/osdep/subprocess.c
+++ b/osdep/subprocess.c
@@ -65,7 +65,30 @@ int mp_subprocess(char **args, struct mp_cancel *cancel, void *ctx,
return res.exit_status;
}
-#endif
+void mp_subprocess_detached(struct mp_log *log, char **args)
+{
+ mp_msg_flush_status_line(log);
+
+ struct mp_subprocess_opts opts = {
+ .exe = args[0],
+ .args = args,
+ .fds = {
+ {.fd = 0, .src_fd = 0,},
+ {.fd = 1, .src_fd = 1,},
+ {.fd = 2, .src_fd = 2,},
+ },
+ .num_fds = 3,
+ .detach = true,
+ };
+ struct mp_subprocess_result res;
+ mp_subprocess2(&opts, &res);
+ if (res.error < 0) {
+ mp_err(log, "Starting subprocess failed: %s\n",
+ mp_subprocess_err_str(res.error));
+ }
+}
+
+#else
struct subprocess_args {
struct mp_log *log;
@@ -100,6 +123,8 @@ void mp_subprocess_detached(struct mp_log *log, char **args)
talloc_free(p);
}
+#endif
+
const char *mp_subprocess_err_str(int num)
{
// Note: these are visible to the public client API
diff --git a/osdep/subprocess.h b/osdep/subprocess.h
index 6aa2981f1d..ea9c43ba34 100644
--- a/osdep/subprocess.h
+++ b/osdep/subprocess.h
@@ -48,12 +48,14 @@ struct mp_subprocess_opts {
struct mp_subprocess_fd fds[MP_SUBPROCESS_MAX_FDS];
int num_fds;
struct mp_cancel *cancel; // if !NULL, asynchronous process abort (kills it)
+ bool detach; // if true, do not wait for process to end
};
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
+ // if started with detach==true, this is always 0
uint32_t exit_status; // if error==0==MP_SUBPROCESS_OK, 0 otherwise
};