From 196a99db1ee9b2fdface2a36f18b2ec219603c94 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?=C3=89tienne=20Barri=C3=A9?= Date: Mon, 18 Dec 2023 11:36:02 +0100 Subject: [PATCH 01/15] Basic use posix_spawn Co-authored-by: Jean Boussier --- configure.ac | 1 + process.c | 38 ++++++++++++++++++++++++++++++++++++++ 2 files changed, 39 insertions(+) diff --git a/configure.ac b/configure.ac index 54d0b5b47428a4..e1eeaf13e9a5b8 100644 --- a/configure.ac +++ b/configure.ac @@ -2134,6 +2134,7 @@ AC_CHECK_FUNCS(popen) AC_CHECK_FUNCS(posix_fadvise) AC_CHECK_FUNCS(posix_madvise) AC_CHECK_FUNCS(posix_memalign) +AC_CHECK_FUNCS(posix_spawn) AC_CHECK_FUNCS(ppoll) AC_CHECK_FUNCS(pread) AC_CHECK_FUNCS(pwrite) diff --git a/process.c b/process.c index c2ef3979d85776..a6925b46951259 100644 --- a/process.c +++ b/process.c @@ -38,6 +38,10 @@ # include #endif +#ifdef HAVE_POSIX_SPAWN +#include +#endif + #ifndef EXIT_SUCCESS # define EXIT_SUCCESS 0 #endif @@ -4614,6 +4618,27 @@ rb_execarg_commandline(const struct rb_execarg *eargp, VALUE *prog) } #endif +#if HAVE_POSIX_SPAWN +static rb_pid_t +rb_posix_spawn(struct rb_execarg *eargp) +{ + pid_t pid; + char *abspath = NULL; + + if (!NIL_P(eargp->invoke.cmd.command_abspath)) { + abspath = RSTRING_PTR(eargp->invoke.cmd.command_abspath); + } + else { + errno = ENOENT; + return -1; + } + + pid = posix_spawn(&pid, abspath, NULL, NULL, NULL, NULL); + + return (rb_pid_t)pid; +} +#endif + static rb_pid_t rb_spawn_process(struct rb_execarg *eargp, char *errmsg, size_t errmsg_buflen) { @@ -4626,6 +4651,19 @@ rb_spawn_process(struct rb_execarg *eargp, char *errmsg, size_t errmsg_buflen) # endif #endif +#if HAVE_POSIX_SPAWN + if (!eargp->use_shell && + !eargp->pgroup_given && + !eargp->umask_given && + !eargp->unsetenv_others_given && + !eargp->close_others_given && + !eargp->chdir_given && + !eargp->uid_given && + !eargp->gid_given && + !eargp->exception_given) { + return rb_posix_spawn(eargp); + } +#endif #if defined HAVE_WORKING_FORK && !USE_SPAWNV pid = fork_check_err(eargp->status, rb_exec_atfork, eargp, eargp->redirect_fds, errmsg, errmsg_buflen, eargp); #else From 5b2cf4157c91c4114bd5f6bc08e23e544b966e81 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?=C3=89tienne=20Barri=C3=A9?= Date: Mon, 18 Dec 2023 11:42:44 +0100 Subject: [PATCH 02/15] args Co-authored-by: Jean Boussier --- process.c | 3 ++- 1 file changed, 2 insertions(+), 1 deletion(-) diff --git a/process.c b/process.c index a6925b46951259..e789bdbf738617 100644 --- a/process.c +++ b/process.c @@ -4633,7 +4633,8 @@ rb_posix_spawn(struct rb_execarg *eargp) return -1; } - pid = posix_spawn(&pid, abspath, NULL, NULL, NULL, NULL); + char **argv = ARGVSTR2ARGV(eargp->invoke.cmd.argv_str); + pid = posix_spawn(&pid, abspath, NULL, NULL, argv, NULL); return (rb_pid_t)pid; } From ea92bfd86e1890a6c3843fcfafcb653f80843c4c Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?=C3=89tienne=20Barri=C3=A9?= Date: Mon, 18 Dec 2023 11:46:53 +0100 Subject: [PATCH 03/15] env Co-authored-by: Jean Boussier --- process.c | 6 +++++- 1 file changed, 5 insertions(+), 1 deletion(-) diff --git a/process.c b/process.c index e789bdbf738617..eb3d56a61d68cf 100644 --- a/process.c +++ b/process.c @@ -4634,7 +4634,11 @@ rb_posix_spawn(struct rb_execarg *eargp) } char **argv = ARGVSTR2ARGV(eargp->invoke.cmd.argv_str); - pid = posix_spawn(&pid, abspath, NULL, NULL, argv, NULL); + + VALUE envp_str = eargp->envp_str; + char **envp = RTEST(envp_str) ? RB_IMEMO_TMPBUF_PTR(envp_str) : NULL; + + pid = posix_spawn(&pid, abspath, NULL, NULL, argv, envp); return (rb_pid_t)pid; } From 0d408515b4ca5b268ed69186a4e3ca0574400e2d Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?=C3=89tienne=20Barri=C3=A9?= Date: Mon, 18 Dec 2023 12:52:51 +0100 Subject: [PATCH 04/15] wip Co-authored-by: Jean Boussier --- process.c | 10 +++++++--- 1 file changed, 7 insertions(+), 3 deletions(-) diff --git a/process.c b/process.c index eb3d56a61d68cf..873330b4cd5a17 100644 --- a/process.c +++ b/process.c @@ -4624,8 +4624,9 @@ rb_posix_spawn(struct rb_execarg *eargp) { pid_t pid; char *abspath = NULL; + char **argv = NULL; - if (!NIL_P(eargp->invoke.cmd.command_abspath)) { + if (RTEST(eargp->invoke.cmd.command_abspath)) { abspath = RSTRING_PTR(eargp->invoke.cmd.command_abspath); } else { @@ -4633,12 +4634,15 @@ rb_posix_spawn(struct rb_execarg *eargp) return -1; } - char **argv = ARGVSTR2ARGV(eargp->invoke.cmd.argv_str); + argv = ARGVSTR2ARGV(eargp->invoke.cmd.argv_str); VALUE envp_str = eargp->envp_str; char **envp = RTEST(envp_str) ? RB_IMEMO_TMPBUF_PTR(envp_str) : NULL; - pid = posix_spawn(&pid, abspath, NULL, NULL, argv, envp); + int err = posix_spawn(&pid, abspath, NULL, NULL, argv, envp); + if (err) { + rb_sys_fail(abspath); + } return (rb_pid_t)pid; } From 37b89b6b398ecacfc97ca8ad754aa4f5c99d1d38 Mon Sep 17 00:00:00 2001 From: Jean Boussier Date: Mon, 15 Jan 2024 11:53:14 +0100 Subject: [PATCH 05/15] Support `eargp->use_shell` in `rb_posix_spawn` --- process.c | 36 ++++++++++++++++++++++++++++-------- 1 file changed, 28 insertions(+), 8 deletions(-) diff --git a/process.c b/process.c index 873330b4cd5a17..8a80fcc0feed31 100644 --- a/process.c +++ b/process.c @@ -4626,16 +4626,36 @@ rb_posix_spawn(struct rb_execarg *eargp) char *abspath = NULL; char **argv = NULL; - if (RTEST(eargp->invoke.cmd.command_abspath)) { - abspath = RSTRING_PTR(eargp->invoke.cmd.command_abspath); + if (eargp->use_shell) { + char *s = RSTRING_PTR(eargp->invoke.sh.shell_script); + while (*s == ' ' || *s == '\t' || *s == '\n') { + s++; + } + + if (!*s) { + errno = ENOENT; + return -1; + } + + // TODO: do we need dln_find_exe_r for __CYGWIN32__? Does __CYGWIN32__ even have posix_spawn? + abspath = (char *)"/bin/sh"; + argv = ALLOCA_N(char *, 4); + argv[0] = (char *)"sh"; + argv[1] = (char *)"-c"; + argv[2] = s; + argv[3] = NULL; } - else { - errno = ENOENT; - return -1; + else { // no-shell + if (RTEST(eargp->invoke.cmd.command_abspath)) { + abspath = RSTRING_PTR(eargp->invoke.cmd.command_abspath); + } + else { + errno = ENOENT; + return -1; + } + argv = ARGVSTR2ARGV(eargp->invoke.cmd.argv_str); } - argv = ARGVSTR2ARGV(eargp->invoke.cmd.argv_str); - VALUE envp_str = eargp->envp_str; char **envp = RTEST(envp_str) ? RB_IMEMO_TMPBUF_PTR(envp_str) : NULL; @@ -4661,7 +4681,7 @@ rb_spawn_process(struct rb_execarg *eargp, char *errmsg, size_t errmsg_buflen) #endif #if HAVE_POSIX_SPAWN - if (!eargp->use_shell && + if (//!eargp->use_shell && !eargp->pgroup_given && !eargp->umask_given && !eargp->unsetenv_others_given && From aad399e231f54f5447a8b32f9b6e0e7807b62580 Mon Sep 17 00:00:00 2001 From: Jean Boussier Date: Mon, 15 Jan 2024 12:17:13 +0100 Subject: [PATCH 06/15] posix_spawn: handle pgroup --- process.c | 19 +++++++++++++++++-- 1 file changed, 17 insertions(+), 2 deletions(-) diff --git a/process.c b/process.c index 8a80fcc0feed31..c69331430c9908 100644 --- a/process.c +++ b/process.c @@ -4659,7 +4659,22 @@ rb_posix_spawn(struct rb_execarg *eargp) VALUE envp_str = eargp->envp_str; char **envp = RTEST(envp_str) ? RB_IMEMO_TMPBUF_PTR(envp_str) : NULL; - int err = posix_spawn(&pid, abspath, NULL, NULL, argv, envp); + int err; + + posix_spawnattr_t attr; + posix_spawnattr_init(&attr); + + if (eargp->pgroup_given) { + if ((err = posix_spawnattr_setflags(&attr, POSIX_SPAWN_SETPGROUP))) { + rb_syserr_fail(err, "posix_spawnattr_setflags"); + } + + if ((err = posix_spawnattr_setpgroup(&attr, eargp->pgroup_pgid))) { + rb_syserr_fail(err, "posix_spawnattr_setpgroup"); + } + } + + err = posix_spawn(&pid, abspath, NULL, &attr, argv, envp); if (err) { rb_sys_fail(abspath); } @@ -4682,7 +4697,7 @@ rb_spawn_process(struct rb_execarg *eargp, char *errmsg, size_t errmsg_buflen) #if HAVE_POSIX_SPAWN if (//!eargp->use_shell && - !eargp->pgroup_given && + // !eargp->pgroup_given && !eargp->umask_given && !eargp->unsetenv_others_given && !eargp->close_others_given && From 536d1af3258ed2fff5dc35bd70c551c62e51779a Mon Sep 17 00:00:00 2001 From: Jean Boussier Date: Mon, 15 Jan 2024 12:43:09 +0100 Subject: [PATCH 07/15] Skip posix_spawn if IOs are redirected --- process.c | 1 + 1 file changed, 1 insertion(+) diff --git a/process.c b/process.c index c69331430c9908..9dbae824b49ff5 100644 --- a/process.c +++ b/process.c @@ -4698,6 +4698,7 @@ rb_spawn_process(struct rb_execarg *eargp, char *errmsg, size_t errmsg_buflen) #if HAVE_POSIX_SPAWN if (//!eargp->use_shell && // !eargp->pgroup_given && + eargp->close_others_maxhint == -1 && !eargp->umask_given && !eargp->unsetenv_others_given && !eargp->close_others_given && From b0088d641e62697a111a4bdb800ea4b5258cbfdb Mon Sep 17 00:00:00 2001 From: Jean Boussier Date: Mon, 15 Jan 2024 12:56:30 +0100 Subject: [PATCH 08/15] Call posix_spawnattr_destroy (generally noop, but by contract...) --- process.c | 2 ++ 1 file changed, 2 insertions(+) diff --git a/process.c b/process.c index 9dbae824b49ff5..893ba0eed19929 100644 --- a/process.c +++ b/process.c @@ -4675,6 +4675,8 @@ rb_posix_spawn(struct rb_execarg *eargp) } err = posix_spawn(&pid, abspath, NULL, &attr, argv, envp); + posix_spawnattr_destroy(&attr); + if (err) { rb_sys_fail(abspath); } From 9b75de1a29556e8f17932e7034c18a2a6f1664b4 Mon Sep 17 00:00:00 2001 From: Jean Boussier Date: Mon, 15 Jan 2024 13:05:23 +0100 Subject: [PATCH 09/15] posix_spawn: always compute envp posix_spawn doesn't inherit any env, so we need to always pass it all. --- process.c | 2 ++ 1 file changed, 2 insertions(+) diff --git a/process.c b/process.c index 893ba0eed19929..eceb63d75a28aa 100644 --- a/process.c +++ b/process.c @@ -372,6 +372,8 @@ static rb_pid_t cached_pid; #define execv(path, argv) (rb_async_bug_errno("unreachable: async-signal-unsafe execv() is called", 0)) #define execl(path, arg0, arg1, arg2, term) do { extern char **environ; execle((path), (arg0), (arg1), (arg2), (term), (environ)); } while (0) #define ALWAYS_NEED_ENVP 1 +#elif defined(HAVE_POSIX_SPAWN) +#define ALWAYS_NEED_ENVP 1 #else #define ALWAYS_NEED_ENVP 0 #endif From 650ff1628b7de8761c42e4ee24aaa870f5ea4595 Mon Sep 17 00:00:00 2001 From: Jean Boussier Date: Mon, 22 Jan 2024 11:13:25 +0100 Subject: [PATCH 10/15] Handle most basic FD redirection (dup2) --- process.c | 35 +++++++++++++++++++++++++++++++++-- 1 file changed, 33 insertions(+), 2 deletions(-) diff --git a/process.c b/process.c index eceb63d75a28aa..6b94892693ba9d 100644 --- a/process.c +++ b/process.c @@ -3558,6 +3558,18 @@ rb_execarg_run_options(const struct rb_execarg *eargp, struct rb_execarg *sargp, { VALUE obj; + if (eargp->fd_dup2 != Qfalse) { + fprintf(stderr, "eargp->fd_dup2 set\n"); + } + + if (eargp->fd_dup2_child != Qfalse) { + fprintf(stderr, "eargp->fd_dup2_child set\n"); + } + + if (eargp->fd_close != Qfalse) { + fprintf(stderr, "eargp->fd_close set\n"); + } + if (sargp) { /* assume that sargp is always NULL on fork-able environments */ MEMZERO(sargp, struct rb_execarg, 1); @@ -4676,8 +4688,26 @@ rb_posix_spawn(struct rb_execarg *eargp) } } - err = posix_spawn(&pid, abspath, NULL, &attr, argv, envp); + posix_spawn_file_actions_t file_actions; + posix_spawn_file_actions_init(&file_actions); + + if (RTEST(eargp->fd_dup2)) { + for (long index = 0; index < RARRAY_LEN(eargp->fd_dup2); index++) { + VALUE pair = RARRAY_AREF(eargp->fd_dup2, index); + VALUE fd = RARRAY_AREF(pair, 0); + VALUE params = RARRAY_AREF(pair, 1); + + int new_fd = NUM2INT(params); // TODO: params may not be a FD, may need more massaging. + fprintf(stderr, "posix_spawn_file_actions_adddup2(fops, %d, %d)\n", new_fd, NUM2INT(fd)); + if ((err = posix_spawn_file_actions_adddup2(&file_actions, new_fd, NUM2INT(fd)))) { + rb_syserr_fail(err, "posix_spawn_file_actions_adddup2"); + } + } + } + + err = posix_spawn(&pid, abspath, &file_actions, &attr, argv, envp); posix_spawnattr_destroy(&attr); + posix_spawn_file_actions_destroy(&file_actions); if (err) { rb_sys_fail(abspath); @@ -4700,9 +4730,10 @@ rb_spawn_process(struct rb_execarg *eargp, char *errmsg, size_t errmsg_buflen) #endif #if HAVE_POSIX_SPAWN + // fprintf(stderr, "eargp->close_others_maxhint = %d\n", eargp->close_others_maxhint); if (//!eargp->use_shell && // !eargp->pgroup_given && - eargp->close_others_maxhint == -1 && + // eargp->close_others_maxhint == -1 && !eargp->umask_given && !eargp->unsetenv_others_given && !eargp->close_others_given && From 0c313a44d2d8ac9ba641912da82c6a9cb53b361a Mon Sep 17 00:00:00 2001 From: Jean Boussier Date: Mon, 22 Jan 2024 11:30:05 +0100 Subject: [PATCH 11/15] posix_spawn: support eargp->fd_close --- process.c | 32 ++++++++++++++++++++++++++++++++ 1 file changed, 32 insertions(+) diff --git a/process.c b/process.c index 6b94892693ba9d..1fae657dbcb530 100644 --- a/process.c +++ b/process.c @@ -4691,6 +4691,26 @@ rb_posix_spawn(struct rb_execarg *eargp) posix_spawn_file_actions_t file_actions; posix_spawn_file_actions_init(&file_actions); + // TODO: do we need it? Currently the `open` seem to be done in the parent. + // if (RTEST(eargp->fd_open)) { + // for (long index = 0; index < RARRAY_LEN(eargp->fd_open); index++) { + // VALUE pair = RARRAY_AREF(eargp->fd_open, index); + // VALUE fd = RARRAY_AREF(pair, 0); + // VALUE params = RARRAY_AREF(pair, 1); + // + // VALUE path = RARRAY_AREF(param, 0); + // VALUE flags = RARRAY_AREF(param, 1); + // // param = rb_ary_new3(4, path, flags, perm, Qnil); + // + // + // int new_fd = NUM2INT(params); // TODO: params may not be a FD, may need more massaging. + // fprintf(stderr, "posix_spawn_file_actions_addopen(fops, %d, %d)\n", new_fd, NUM2INT(fd)); + // if ((err = posix_spawn_file_actions_addopen(&file_actions, fd, RSTRING_PTR(path), NUM2INT(flags)))) { + // rb_syserr_fail(err, "posix_spawn_file_actions_addopen"); + // } + // } + // } + if (RTEST(eargp->fd_dup2)) { for (long index = 0; index < RARRAY_LEN(eargp->fd_dup2); index++) { VALUE pair = RARRAY_AREF(eargp->fd_dup2, index); @@ -4705,6 +4725,18 @@ rb_posix_spawn(struct rb_execarg *eargp) } } + if (RTEST(eargp->fd_close)) { + for (long index = 0; index < RARRAY_LEN(eargp->fd_close); index++) { + VALUE pair = RARRAY_AREF(eargp->fd_close, index); + VALUE fd = RARRAY_AREF(pair, 0); + + fprintf(stderr, "posix_spawn_file_actions_addclose(fops, %d)\n", NUM2INT(fd)); + if ((err = posix_spawn_file_actions_addclose(&file_actions, NUM2INT(fd)))) { + rb_syserr_fail(err, "posix_spawn_file_actions_addclose"); + } + } + } + err = posix_spawn(&pid, abspath, &file_actions, &attr, argv, envp); posix_spawnattr_destroy(&attr); posix_spawn_file_actions_destroy(&file_actions); From 52410a136ad51d03e4d9cf3ea87dc968014660d1 Mon Sep 17 00:00:00 2001 From: Jean Boussier Date: Mon, 22 Jan 2024 11:35:09 +0100 Subject: [PATCH 12/15] posix_spawn: handle eargp->fd_dup2_child --- process.c | 14 ++++++++++++++ 1 file changed, 14 insertions(+) diff --git a/process.c b/process.c index 1fae657dbcb530..a3330fbd17c4e5 100644 --- a/process.c +++ b/process.c @@ -4725,6 +4725,20 @@ rb_posix_spawn(struct rb_execarg *eargp) } } + if (RTEST(eargp->fd_dup2_child)) { + for (long index = 0; index < RARRAY_LEN(eargp->fd_dup2_child); index++) { + VALUE pair = RARRAY_AREF(eargp->fd_dup2_child, index); + VALUE fd = RARRAY_AREF(pair, 0); + VALUE params = RARRAY_AREF(pair, 1); + + int new_fd = NUM2INT(params); // TODO: params may not be a FD, may need more massaging. + fprintf(stderr, "posix_spawn_file_actions_adddup2(fops, %d, %d)\n", new_fd, NUM2INT(fd)); + if ((err = posix_spawn_file_actions_adddup2(&file_actions, new_fd, NUM2INT(fd)))) { + rb_syserr_fail(err, "posix_spawn_file_actions_adddup2"); + } + } + } + if (RTEST(eargp->fd_close)) { for (long index = 0; index < RARRAY_LEN(eargp->fd_close); index++) { VALUE pair = RARRAY_AREF(eargp->fd_close, index); From 7d8a2d1bed502674e405828c618d346a1cde3678 Mon Sep 17 00:00:00 2001 From: Jean Boussier Date: Mon, 22 Jan 2024 12:08:18 +0100 Subject: [PATCH 13/15] posix_spawn: handle pgroup = -1 --- process.c | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/process.c b/process.c index a3330fbd17c4e5..e03a69ce887878 100644 --- a/process.c +++ b/process.c @@ -4678,7 +4678,7 @@ rb_posix_spawn(struct rb_execarg *eargp) posix_spawnattr_t attr; posix_spawnattr_init(&attr); - if (eargp->pgroup_given) { + if (eargp->pgroup_given && eargp->pgroup_pgid != -1) { if ((err = posix_spawnattr_setflags(&attr, POSIX_SPAWN_SETPGROUP))) { rb_syserr_fail(err, "posix_spawnattr_setflags"); } From fe34e95ac01833ce6a4f4df28865cacf7ce4205b Mon Sep 17 00:00:00 2001 From: Jean Boussier Date: Mon, 22 Jan 2024 12:20:07 +0100 Subject: [PATCH 14/15] posix_spawn: handle perm issues --- process.c | 5 +++++ 1 file changed, 5 insertions(+) diff --git a/process.c b/process.c index e03a69ce887878..4a2cde53abe3ae 100644 --- a/process.c +++ b/process.c @@ -4756,6 +4756,11 @@ rb_posix_spawn(struct rb_execarg *eargp) posix_spawn_file_actions_destroy(&file_actions); if (err) { + // posix_spawn only returns fork/vfork/clone failures. + // If it failed but errno == 0, then it must be an "exec" failure. + if (errno == 0) { + eaccess(abspath, X_OK); + } rb_sys_fail(abspath); } From bba8ef3cc0381870c0472f5fb9762e546a15c6ee Mon Sep 17 00:00:00 2001 From: Jean Boussier Date: Mon, 22 Jan 2024 12:26:36 +0100 Subject: [PATCH 15/15] posix_spawn: handle executable being a directory --- process.c | 11 ++++++++++- 1 file changed, 10 insertions(+), 1 deletion(-) diff --git a/process.c b/process.c index 4a2cde53abe3ae..12d40518ec7804 100644 --- a/process.c +++ b/process.c @@ -4759,7 +4759,16 @@ rb_posix_spawn(struct rb_execarg *eargp) // posix_spawn only returns fork/vfork/clone failures. // If it failed but errno == 0, then it must be an "exec" failure. if (errno == 0) { - eaccess(abspath, X_OK); + if (!eaccess(abspath, X_OK)) { + // abspath is executable + struct stat file_stat; + if (stat(abspath, &file_stat)) { + rb_sys_fail(abspath); + } + if (S_ISDIR(file_stat.st_mode)) { + errno = EISDIR; + } + } } rb_sys_fail(abspath); }