Skip to content

Commit

Permalink
flux-exec: don't use flux imp kill
Browse files Browse the repository at this point in the history
Problem: flux-exec uses 'flux imp kill' to send signals to remote
process if the IMP started them, but the IMP will now forward signals
and 'flux imp kill' is deprecated.

Don't call 'flux imp kill'.

When the IMP starts a process, translate SIGKILL to SIGUSR1 per RFC 15.

Drop note about 'flux imp kill' from the flux-exec(1) man page.
  • Loading branch information
garlick committed Nov 4, 2024
1 parent 69af343 commit 1878238
Show file tree
Hide file tree
Showing 2 changed files with 14 additions and 59 deletions.
6 changes: 1 addition & 5 deletions doc/man1/flux-exec.rst
Original file line number Diff line number Diff line change
Expand Up @@ -95,11 +95,7 @@ OPTIONS

Prepend the full path to :program:`flux-imp run` to *COMMAND*. This option
is mostly meant for testing or as a convenience to execute a configured
``prolog`` or ``epilog`` command under the IMP. Note: When this option is
used, or if :program:`flux-imp` is detected as the first argument of
*COMMAND*, :program:`flux exec` will use :program:`flux-imp kill` to
signal remote commands instead of the normal builtin subprocess signaling
mechanism.
``prolog`` or ``epilog`` command under the IMP.

CAVEATS
=======
Expand Down
67 changes: 13 additions & 54 deletions src/cmd/flux-exec.c
Original file line number Diff line number Diff line change
Expand Up @@ -335,68 +335,27 @@ static void stdin_cb (flux_reactor_t *r,
}
}

static void kill_completion_cb (flux_subprocess_t *p)
{
flux_subprocess_destroy (p);
}

static flux_subprocess_t *imp_kill (flux_subprocess_t *p, int signum)
{
flux_cmd_t *cmd;
flux_subprocess_ops_t ops = {
.on_completion = kill_completion_cb,
.on_state_change = NULL,
.on_channel_out = NULL,
.on_stdout = output_cb,
.on_stderr = output_cb,
};

pid_t pid = flux_subprocess_pid (p);
int rank = flux_subprocess_rank (p);

if (!(cmd = flux_cmd_create (0, NULL, environ))
|| flux_cmd_argv_append (cmd, imp_path) < 0
|| flux_cmd_argv_append (cmd, "kill") < 0
|| flux_cmd_argv_appendf (cmd, "%d", signum) < 0
|| flux_cmd_argv_appendf (cmd, "-%ld", (long) pid) < 0) {
fprintf (stderr,
"Failed to create flux-imp kill command for rank %d pid %d\n",
rank, pid);
return NULL;
}
/* Note: subprocess object destroyed in completion callback
*/
return flux_rexec (flux_handle,
rank,
FLUX_SUBPROCESS_FLAGS_LOCAL_UNBUF,
cmd,
&ops);
}

static void killall (zlistx_t *l, int signum)
{
flux_subprocess_t *p = zlistx_first (l);
while (p) {
if (flux_subprocess_state (p) == FLUX_SUBPROCESS_RUNNING) {
if (use_imp) {
if (!imp_kill (p, signum))
/* RFC 15 states that the IMP will treat SIGUSR1 as a surrogate
* for SIGKILL.
*/
if (use_imp && signum == SIGKILL)
signum = SIGUSR1;

flux_future_t *f = flux_subprocess_kill (p, signum);
if (!f) {
if (optparse_getopt (opts, "verbose", NULL) > 0)
fprintf (stderr,
"failed to signal rank %d: %s\n",
flux_subprocess_rank (p),
strerror (errno));
}
else {
flux_future_t *f = flux_subprocess_kill (p, signum);
if (!f) {
if (optparse_getopt (opts, "verbose", NULL) > 0)
fprintf (stderr,
"failed to signal rank %d: %s\n",
flux_subprocess_rank (p),
strerror (errno));
}
/* don't care about response */
flux_future_destroy (f);
flux_subprocess_rank (p),
strerror (errno));
}
/* don't care about response */
flux_future_destroy (f);
}
p = zlistx_next (l);
}
Expand Down

0 comments on commit 1878238

Please sign in to comment.