diff --git a/src/broker/runat.c b/src/broker/runat.c index 868860e6a2cd..f806d966787a 100644 --- a/src/broker/runat.c +++ b/src/broker/runat.c @@ -351,10 +351,10 @@ static struct runat_command *runat_command_create (char **env, int flags) if (flags & RUNAT_FLAG_FORK_EXEC) cmd->flags |= FLUX_SUBPROCESS_FLAGS_FORK_EXEC; /* - * Always run runat command in separate process group so that any - * processes spawned by command are signaled by flux_subprocess_signal() + * N.B. By default subprocesses call setpgrp() before exec(2). So + * any processes spawned by command are also signaled by + * flux_subprocess_signal() */ - cmd->flags |= FLUX_SUBPROCESS_FLAGS_SETPGRP; if (!(cmd->cmd = flux_cmd_create (0, NULL, env))) goto error; return cmd; diff --git a/src/cmd/builtin/proxy.c b/src/cmd/builtin/proxy.c index 1352ffbf6934..b4af9c6bd9aa 100644 --- a/src/cmd/builtin/proxy.c +++ b/src/cmd/builtin/proxy.c @@ -114,6 +114,7 @@ static int child_create (struct proxy_command *ctx, .on_stderr = NULL, }; flux_cmd_t *cmd = NULL; + int flags = 0; int i; if (!shell) @@ -154,10 +155,12 @@ static int child_create (struct proxy_command *ctx, goto error; /* We want stdio fallthrough so subprocess can capture tty if - * necessary (i.e. an interactive shell) + * necessary (i.e. an interactive shell). */ + flags |= FLUX_SUBPROCESS_FLAGS_STDIO_FALLTHROUGH; + flags |= FLUX_SUBPROCESS_FLAGS_NO_SETPGRP; if (!(p = flux_local_exec (flux_get_reactor (ctx->h), - FLUX_SUBPROCESS_FLAGS_STDIO_FALLTHROUGH, + flags, cmd, &ops))) goto error; diff --git a/src/cmd/flux-start.c b/src/cmd/flux-start.c index aa98854f86eb..9710732c4f88 100644 --- a/src/cmd/flux-start.c +++ b/src/cmd/flux-start.c @@ -735,6 +735,7 @@ int client_run (struct client *cli) .on_stdout = NULL, .on_stderr = NULL, }; + int flags = 0; if (cli->p) { errno = EEXIST; return -1; @@ -742,8 +743,10 @@ int client_run (struct client *cli) /* We want stdio fallthrough so subprocess can capture tty if * necessary (i.e. an interactive shell) */ + flags |= FLUX_SUBPROCESS_FLAGS_STDIO_FALLTHROUGH; + flags |= FLUX_SUBPROCESS_FLAGS_NO_SETPGRP; if (!(cli->p = flux_local_exec (ctx.reactor, - FLUX_SUBPROCESS_FLAGS_STDIO_FALLTHROUGH, + flags, cli->cmd, &ops))) log_err_exit ("flux_exec"); diff --git a/src/common/libsubprocess/fork.c b/src/common/libsubprocess/fork.c index 524b0a29fcc2..4d00e3961bff 100644 --- a/src/common/libsubprocess/fork.c +++ b/src/common/libsubprocess/fork.c @@ -166,7 +166,7 @@ static int local_child (flux_subprocess_t *p) p->in_hook = false; } - if (p->flags & FLUX_SUBPROCESS_FLAGS_SETPGRP + if (!(p->flags & FLUX_SUBPROCESS_FLAGS_NO_SETPGRP) && getpgrp () != getpid ()) { if (setpgrp () < 0) { fprintf (stderr, "setpgrp: %s\n", strerror (errno)); diff --git a/src/common/libsubprocess/posix_spawn.c b/src/common/libsubprocess/posix_spawn.c index 84121271f162..f4306473bdd0 100644 --- a/src/common/libsubprocess/posix_spawn.c +++ b/src/common/libsubprocess/posix_spawn.c @@ -136,7 +136,7 @@ int create_process_spawn (flux_subprocess_t *p) /* If setpgrp(2) is desired for the child process, then add this * flag to the spawnattr flags. */ - if (p->flags & FLUX_SUBPROCESS_FLAGS_SETPGRP) + if (!(p->flags & FLUX_SUBPROCESS_FLAGS_NO_SETPGRP)) flags |= POSIX_SPAWN_SETPGROUP; posix_spawnattr_setflags (&attr, flags); diff --git a/src/common/libsubprocess/server.c b/src/common/libsubprocess/server.c index 913d4b9d291e..41c4c4c21e9d 100644 --- a/src/common/libsubprocess/server.c +++ b/src/common/libsubprocess/server.c @@ -365,7 +365,7 @@ static void server_exec_cb (flux_t *h, flux_cmd_unsetenv (cmd, "FLUX_PROXY_REMOTE"); if (!(p = flux_local_exec_ex (flux_get_reactor (s->h), - FLUX_SUBPROCESS_FLAGS_SETPGRP, + 0, cmd, &ops, NULL, diff --git a/src/common/libsubprocess/subprocess.c b/src/common/libsubprocess/subprocess.c index 71e599792df5..73ec096c4e66 100644 --- a/src/common/libsubprocess/subprocess.c +++ b/src/common/libsubprocess/subprocess.c @@ -463,7 +463,7 @@ flux_subprocess_t *flux_local_exec_ex (flux_reactor_t *r, { flux_subprocess_t *p = NULL; int valid_flags = (FLUX_SUBPROCESS_FLAGS_STDIO_FALLTHROUGH - | FLUX_SUBPROCESS_FLAGS_SETPGRP + | FLUX_SUBPROCESS_FLAGS_NO_SETPGRP | FLUX_SUBPROCESS_FLAGS_FORK_EXEC); if (!r || !cmd) { @@ -946,7 +946,7 @@ flux_future_t *flux_subprocess_kill (flux_subprocess_t *p, int signum) errno = ESRCH; return NULL; } - if (p->flags & FLUX_SUBPROCESS_FLAGS_SETPGRP) + if (!(p->flags & FLUX_SUBPROCESS_FLAGS_NO_SETPGRP)) ret = killpg (p->pid, signum); else ret = kill (p->pid, signum); diff --git a/src/common/libsubprocess/subprocess.h b/src/common/libsubprocess/subprocess.h index b0a941543a2f..28471928d387 100644 --- a/src/common/libsubprocess/subprocess.h +++ b/src/common/libsubprocess/subprocess.h @@ -57,8 +57,8 @@ enum { * will fail on streams of "stdin", "stdout", or "stderr". */ FLUX_SUBPROCESS_FLAGS_STDIO_FALLTHROUGH = 1, - /* flux_local_exec(): call setpgrp() before exec(2) */ - FLUX_SUBPROCESS_FLAGS_SETPGRP = 2, + /* flux_local_exec(): do not call setpgrp() before exec(2) */ + FLUX_SUBPROCESS_FLAGS_NO_SETPGRP = 2, /* flux_local_exec(): use fork(2)/exec(2) even if posix_spawn(3) * available */ FLUX_SUBPROCESS_FLAGS_FORK_EXEC = 4, diff --git a/src/common/libsubprocess/test/subprocess.c b/src/common/libsubprocess/test/subprocess.c index fc93d95b3d18..4c4b4cea079e 100644 --- a/src/common/libsubprocess/test/subprocess.c +++ b/src/common/libsubprocess/test/subprocess.c @@ -323,7 +323,7 @@ void test_basic_fail (flux_reactor_t *r) flux_cmd_destroy (cmd); } -void test_flag_setpgrp (flux_reactor_t *r) +void test_flag_no_setpgrp (flux_reactor_t *r) { char *av[] = { "/bin/true", NULL }; flux_cmd_t *cmd; @@ -335,7 +335,7 @@ void test_flag_setpgrp (flux_reactor_t *r) .on_completion = completion_cb }; completion_cb_count = 0; - p = flux_local_exec (r, FLUX_SUBPROCESS_FLAGS_SETPGRP, cmd, &ops); + p = flux_local_exec (r, FLUX_SUBPROCESS_FLAGS_NO_SETPGRP, cmd, &ops); ok (p != NULL, "flux_local_exec"); ok (flux_subprocess_state (p) == FLUX_SUBPROCESS_RUNNING, @@ -551,7 +551,7 @@ void test_kill_setpgrp (flux_reactor_t *r) output_processes_cb_count = 0; parent_pid = -1; child_pid = -1; - p = flux_local_exec (r, FLUX_SUBPROCESS_FLAGS_SETPGRP, cmd, &ops); + p = flux_local_exec (r, 0, cmd, &ops); ok (p != NULL, "flux_local_exec"); ok (flux_subprocess_state (p) == FLUX_SUBPROCESS_RUNNING, @@ -1097,8 +1097,8 @@ int main (int argc, char *argv[]) test_basic_fail (r); diag ("env_passed"); test_env_passed (r); - diag ("flag_setpgrp"); - test_flag_setpgrp (r); + diag ("flag_no_setpgrp"); + test_flag_no_setpgrp (r); diag ("flag_fork_exec"); test_flag_fork_exec (r); diag ("kill"); diff --git a/src/shell/task.c b/src/shell/task.c index 42cd6f5cd5ce..6624972f749f 100644 --- a/src/shell/task.c +++ b/src/shell/task.c @@ -222,7 +222,7 @@ int shell_task_start (struct flux_shell *shell, shell_task_completion_f cb, void *arg) { - int flags = FLUX_SUBPROCESS_FLAGS_SETPGRP; + int flags = 0; flux_reactor_t *r = shell->r; flux_subprocess_hooks_t hooks = { .pre_exec = subproc_preexec_hook, @@ -230,7 +230,7 @@ int shell_task_start (struct flux_shell *shell, }; if (shell->nosetpgrp) - flags &= ~FLUX_SUBPROCESS_FLAGS_SETPGRP; + flags |= FLUX_SUBPROCESS_FLAGS_NO_SETPGRP; task->proc = flux_local_exec_ex (r, flags,