From 2c73d0c73357594b5fb1262510c6454d3c920961 Mon Sep 17 00:00:00 2001 From: Nicholas Chaimov Date: Wed, 8 Jul 2026 14:50:53 -0700 Subject: [PATCH 1/9] Handle PrologFlags=Alloc in Slurm plugin We only need workarounds for delayed prolog in the default mode. If PrologFlags=Alloc is set in slurm.conf, don't use the workarounds and instead launch Spindle on all nodes in the prolog. --- src/slurm_plugin/slurm_plugin.c | 63 ++++++++++++++++++++++++++++----- 1 file changed, 54 insertions(+), 9 deletions(-) diff --git a/src/slurm_plugin/slurm_plugin.c b/src/slurm_plugin/slurm_plugin.c index 0989299c..103e0a07 100644 --- a/src/slurm_plugin/slurm_plugin.c +++ b/src/slurm_plugin/slurm_plugin.c @@ -27,6 +27,7 @@ Place, Suite 330, Boston, MA 02111-1307 USA #include #include +#include #include #include "spindle_launch.h" @@ -102,6 +103,7 @@ static __thread spank_t current_spank; static const char *user_options = NULL; static int enable_spindle = 0; static int start_session = 0; +static int prolog_alloc_mode = 0; extern char *parse_location(char *loc, number_t number); extern char *realize(char *path); @@ -156,7 +158,14 @@ static int should_use_session(spank_t spank) { if (session_env) return 1; err = spank_option_getopt(spank, &session_option, NULL); - return (err == ESPANK_SUCCESS); + return (err == ESPANK_SUCCESS); + } + + /* In the allocator, we are running in the same process that + * handled the command line arguments and can check the + * flag directly. */ + if (context == S_CTX_ALLOCATOR) { + return start_session; } return 0; @@ -168,6 +177,24 @@ int slurm_spank_init(spank_t spank, int ac, char *argv[]) { if (context == S_CTX_ALLOCATOR) { spank_option_register(spank, &session_option); } + +#if defined(PROLOG_FLAG_ALLOC) + /* Check whether Slurm is configured to run the prolog at + * allocation time (PROLOG_FLAG_ALLOC), or the default mode + * where the prolog runs on the first step. */ + if (!prolog_alloc_mode) { + slurm_conf_t *conf = NULL; + if (slurm_load_ctl_conf(0, &conf) == SLURM_SUCCESS) { + if (conf->prolog_flags & PROLOG_FLAG_ALLOC) + prolog_alloc_mode = 1; + slurm_free_ctl_conf(conf); + } else { + sdprintf(1, "Could not read Slurm config.\n"); + } + } + sdprintf(2, "prolog_alloc_mode = %d\n", prolog_alloc_mode); +#endif + return 0; } @@ -186,6 +213,17 @@ int slurm_spank_init_post_opt(spank_t spank, int ac, char *argv[]) { if (start_session) { setenv(SPANK_SPINDLE_USE_SESSION, "1", 1); } + + /* With PrologFlags=Alloc, forward env vars to job control here; + them. Without PrologFlags=Alloc, this forwarding happens later + in local context (srun). */ + if (prolog_alloc_mode && start_session) { + int result = forward_environment_to_job_control(spank); + if (result == -1) { + slurm_error("ERROR: Spindle plugin error. Unable to forward environment variables to job control.\n"); + return result; + } + } } return 0; } @@ -314,10 +352,13 @@ int slurm_spank_local_user_init(spank_t spank, int ac, char *argv[]) goto done; } - use_session = should_use_session(spank); + use_session = should_use_session(spank); if (!use_session) goto done; + if (prolog_alloc_mode) + goto done; + result = process_spindle_args(spank, ac, argv, ¶ms, NULL, NULL, use_session); if (result == -1) { slurm_error("ERROR: Spindle plugin error. Could not process spindle args in local user init.\n"); @@ -384,9 +425,11 @@ int slurm_spank_job_prolog(spank_t spank, int ac, char *argv[]) { return 0; - envVal = getenv("SPANK_SPINDLE_RSHLAUNCH"); - if (envVal && strcmp(envVal, "1") == 0) - return 0; + if (!prolog_alloc_mode) { + envVal = getenv("SPANK_SPINDLE_RSHLAUNCH"); + if (envVal && strcmp(envVal, "1") == 0) + return 0; + } // The prolog starts in the user's home directory. // Change to $SLURM_JOB_WORK_DIR so logs go to right place. @@ -508,8 +551,9 @@ int slurm_spank_task_init(spank_t spank, int site_argc, char *site_argv[]) return 0; } - /* When using a session without RSHLAUNCH, handle start in job prolog, not here. */ - if ((!use_session) || (params.opts & OPT_RSHLAUNCH)) { + /* When using a session without RSHLAUNCH, handle start in job prolog, not here. + With PrologFlags=Alloc, session+RSHLAUNCH is also handled in the prolog. */ + if ((!use_session) || ((params.opts & OPT_RSHLAUNCH) && !prolog_alloc_mode)) { start_params.spank = spank; start_params.site_argc = site_argc; start_params.site_argv = site_argv; @@ -560,8 +604,9 @@ static int handleStart(void *params, char **output_str) return 0; } - // Only initialize a session once - if (use_session && (args.opts & OPT_RSHLAUNCH)) { + /* Only initialize a session once. In prolog context (S_CTX_JOB_SCRIPT), + * there is no step yet, so skip the step ID check. */ + if (use_session && (args.opts & OPT_RSHLAUNCH) && spank_context() != S_CTX_JOB_SCRIPT) { err = get_stepid(spank, &stepid); if (err != ESPANK_SUCCESS) { slurm_error("ERROR: Spindle plugin error. Could not get step id."); From fa9805166ba92e3eb770cd25d6184af4c0d941ac Mon Sep 17 00:00:00 2001 From: Nicholas Chaimov Date: Wed, 8 Jul 2026 14:50:53 -0700 Subject: [PATCH 2/9] Bug fixes in Slurm plugin - Fix warning about unchecked pipe, chdir - Fix write from wrong string in dropPrivilegeAndRun - Fix useless check that substituted NULL for NULL --- src/slurm_plugin/plugin_utils.c | 33 +++++++++++++++++++++++++-------- src/slurm_plugin/plugin_utils.h | 1 + src/slurm_plugin/slurm_plugin.c | 11 +++++------ 3 files changed, 31 insertions(+), 14 deletions(-) diff --git a/src/slurm_plugin/plugin_utils.c b/src/slurm_plugin/plugin_utils.c index f77531e4..87c2a37d 100644 --- a/src/slurm_plugin/plugin_utils.c +++ b/src/slurm_plugin/plugin_utils.c @@ -254,11 +254,11 @@ int isFEHost(char **hostlist, unsigned int num_hosts) int feresult = -1; for (i = 0; i < num_hosts; i++) { - if (!last_host || strcmp(hostlist[i], last_host) == 1) { + if (!last_host || strcmp(hostlist[i], last_host) > 0) { last_host = hostlist[i]; } } - sdprintf(2, "last_host = %s\n", last_host ? last_host : NULL); + sdprintf(2, "last_host = %s\n", last_host ? last_host : "(null)"); if (!last_host) { error = errno; sdprintf(1, "ERROR: Could not get current system's hostname: %s\n", strerror(error)); @@ -515,6 +515,15 @@ int signalSpankSessionEnd(spindle_args_t *params) char *unique_file = NULL; +void cleanup_unique_file() +{ + if (unique_file) { + unlink(unique_file); + free(unique_file); + unique_file = NULL; + } +} + #define UNIQUE_FILE_NAME "spindle_unique" int isBEProc(spindle_args_t *params, unsigned int exit_phase) @@ -687,8 +696,10 @@ void push_env(spank_t spank, saved_env_t **env) e->new_spindledebug = readSpankEnv(spank, "SPINDLE_DEBUG"); e->old_spindledebug = getenv("SPINDLE_DEBUG"); - if (e->new_pwd) - chdir(e->new_pwd); + if (e->new_pwd) { + if (chdir(e->new_pwd) == -1) + sdprintf(1, "WARNING: Could not chdir to %s: %s\n", e->new_pwd, strerror(errno)); + } if (e->new_home) setenv("HOME", e->new_home, 1); @@ -727,8 +738,10 @@ void pop_env(saved_env_t *env) else unsetenv("SPINDLE_DEBUG"); - if (env->old_pwd) - chdir(env->old_pwd); + if (env->old_pwd) { + if (chdir(env->old_pwd) == -1) + sdprintf(1, "WARNING: Could not chdir to %s: %s\n", env->old_pwd, strerror(errno)); + } if (env->new_home) free(env->new_home); @@ -815,7 +828,7 @@ int dropPrivilegeAndRun(dpr_function_t func, uid_t uid, void *input, char **outp exit(-1); } if (output_len) { - result = safe_write(pipe_fds[1], output_str, output_len+1); + result = safe_write(pipe_fds[1], child_output_str, output_len+1); if (result != output_len+1) { error = errno; fprintf(stderr, "Spindle error. Could not write result string to pipe: %s\n", strerror(error)); @@ -989,7 +1002,11 @@ pid_t grandchild_fork() int result, fork_result = -1; pipe_fds[0] = pipe_fds[1] = -1; - pipe(pipe_fds); + result = pipe(pipe_fds); + if (result == -1) { + sdprintf(1, "ERROR: pipe() failed in grandchild_fork. Aborting spindle\n"); + return -1; + } child_pid = fork(); if (child_pid == -1) { diff --git a/src/slurm_plugin/plugin_utils.h b/src/slurm_plugin/plugin_utils.h index 0ac2c8c8..a29b9805 100644 --- a/src/slurm_plugin/plugin_utils.h +++ b/src/slurm_plugin/plugin_utils.h @@ -45,6 +45,7 @@ char **getHostsParse(unsigned int num_hosts, const char *shortlist); int isFEHost(char **hostlist, unsigned int num_hosts); extern char *unique_file; +void cleanup_unique_file(); int isBEProc(spindle_args_t *params, unsigned int exit_phase); int doesFEExitSocketExist(spindle_args_t *params); diff --git a/src/slurm_plugin/slurm_plugin.c b/src/slurm_plugin/slurm_plugin.c index 103e0a07..aa27f6bb 100644 --- a/src/slurm_plugin/slurm_plugin.c +++ b/src/slurm_plugin/slurm_plugin.c @@ -189,10 +189,9 @@ int slurm_spank_init(spank_t spank, int ac, char *argv[]) { prolog_alloc_mode = 1; slurm_free_ctl_conf(conf); } else { - sdprintf(1, "Could not read Slurm config.\n"); + sdprintf(1, "Could not read Slurm config, falling back to non-prolog launch.\n"); } } - sdprintf(2, "prolog_alloc_mode = %d\n", prolog_alloc_mode); #endif return 0; @@ -435,7 +434,8 @@ int slurm_spank_job_prolog(spank_t spank, int ac, char *argv[]) { // Change to $SLURM_JOB_WORK_DIR so logs go to right place. work_dir = getenv("SLURM_JOB_WORK_DIR"); if (work_dir) { - chdir(work_dir); + if (chdir(work_dir) == -1) + sdprintf(1, "WARNING: Could not chdir to %s: %s\n", work_dir, strerror(errno)); } err = spank_get_item(spank, S_JOB_UID, &userid); @@ -548,6 +548,7 @@ int slurm_spank_task_init(spank_t spank, int site_argc, char *site_argv[]) } if (params.opts & OPT_OFF) { + pop_env(env); return 0; } @@ -1220,9 +1221,7 @@ static int launchBE(spank_t spank, spindle_args_t *params) else sdprintf(1, "spindleRunBE completed. Session finishing.\n"); - if (unique_file) unlink(unique_file); - free(unique_file); - unique_file = NULL; + cleanup_unique_file(); exit(result); From ed352bd887bbbd0ad811c824e5594aede644bfb9 Mon Sep 17 00:00:00 2001 From: Nicholas Chaimov Date: Wed, 8 Jul 2026 14:50:54 -0700 Subject: [PATCH 3/9] Use PrologFlags=Contain in slurm-plugin container --- containers/spindle-slurm-ubuntu/testing-plugin/conf/slurm.conf | 1 + 1 file changed, 1 insertion(+) diff --git a/containers/spindle-slurm-ubuntu/testing-plugin/conf/slurm.conf b/containers/spindle-slurm-ubuntu/testing-plugin/conf/slurm.conf index abf060d5..e4298157 100644 --- a/containers/spindle-slurm-ubuntu/testing-plugin/conf/slurm.conf +++ b/containers/spindle-slurm-ubuntu/testing-plugin/conf/slurm.conf @@ -34,6 +34,7 @@ JobAcctGatherFrequency=30 AccountingStorageType=accounting_storage/slurmdbd AccountingStorageHost=slurm-db AccountingStoragePort=6819 +PrologFlags=Contain NodeName=slurm-node-1 NodeAddr=slurm-node-1 CPUs=3 RealMemory=1000 State=UNKNOWN NodeName=slurm-node-2 NodeAddr=slurm-node-2 CPUs=3 RealMemory=1000 State=UNKNOWN NodeName=slurm-node-3 NodeAddr=slurm-node-3 CPUs=3 RealMemory=1000 State=UNKNOWN From 8933cfec0f1c283358f267e2e74fc3ad9a8a6e82 Mon Sep 17 00:00:00 2001 From: Nicholas Chaimov Date: Wed, 8 Jul 2026 14:50:54 -0700 Subject: [PATCH 4/9] Documentation for Slurm plugin --- doc/slurm_plugin.md | 115 ++++++++++++++++++++++++++++++++++++++++++++ 1 file changed, 115 insertions(+) create mode 100644 doc/slurm_plugin.md diff --git a/doc/slurm_plugin.md b/doc/slurm_plugin.md new file mode 100644 index 00000000..ab8765c1 --- /dev/null +++ b/doc/slurm_plugin.md @@ -0,0 +1,115 @@ +Spindle Slurm plugin +==================== + +The Spindle Slurm plugin integrates Spindle into Slurm through the +SPANK interface as an alternative launch mechanism to the srun wrapper. +It adds the ability to launch job steps using `srun --spindle`. + +## Building and configuring the plugin + +Configure Spindle with `--enable-slurm-plugin`: + +```bash +./configure --with-rm=slurm-plugin --enable-slurm-plugin [--with-slurm-dir=/path/to/slurm] ... +make +make install +``` + +Refer to `INSTALL` for more details on configuring Spindle. + +After installation of Spindle, the plugin is installed at +`$PREFIX/lib/libspindleslurm.so`. It is registered with Slurm by adding the +following line to `/etc/slurm/plugstack.conf`: + +``` +required /path/to/spindle/lib/libspindleslurm.so +``` + +## Session launch modes + +The manner in which Spindle sessions are started varies depending on +the configuration of Spindle and of Slurm. + +When starting a session, the plugin must arrange for Spindle to start +on each compute node before any step runs within the allocation. +The most straightforward way to do this is to configure the cluster +to run job prologs at allocation time. If your `slurm.conf` includes +`PrologFlags=Alloc` (or another flag that implies it: `Contain`, +`RunInJob`, `X11`, `ForceRequeueOnFail`, or `NoHold`), then sessions +will be started on each node of the allocation at the time the allocation +is made. + +If `PrologFlags=Alloc` or a related setting is *not* used, one of two +mechanisms is used to start the job on every node: + +**RSH launch**: Spindle can use RSH/SSH to launch daemons from the +frontend (FE) process. To use the RSH launch mode, the cluster must be configured +such that passwordless ssh can be used to run commands on every compute +node within the allocation without any interactive user input. +This mode is enabled by configuring Spindle with: + +```bash +./configure --with-rm=slurm-plugin --enable-slurm-plugin --with-rsh-launch [--with-rsh-cmd=/usr/bin/ssh] ... +``` + +**Dummy srun fallback**: If neither `PrologFlags=Alloc` nor RSH launch is available, +Spindle will fall back on using a dummy `srun` invocation to force the prolog +to run on every compute node of the allocation. Note that this has the side-effect +of consuming step 0, so that the user's first step will instead be numbered 1. + +## Using Spindle through the Slurm plugin + +### Per-step mode: `--spindle` + +Add `--spindle` to any `srun` command to use Spindle for that step. +Spindle daemons start before the application runs and shut down when +the step finishes. + +```bash +srun --spindle ./my_application +``` + +Additional arguments can be passed to Spindle as an optional value of the argument `--spindle`: + +```bash +srun --spindle="--level=low" ./my_application +``` + +### Session mode: `--spindle-session` + +Session mode shares a Spindle session across multiple steps. +The use of sessions in the Slurm plugin differs from its use with +the other launchers. Unlike the other launchers, sessions are *not* +started with `spindle --start-session`. Rather, an additional argument +`--spindle-session` is added to `salloc` and `sbatch`. + +To use a session, include `--spindle-session` when creating the allocation: + +```bash +salloc --spindle-session ... +``` + +Then run steps with `--spindle`: + +```bash +srun --spindle ./app1 +srun --spindle ./app2 +srun --spindle ./app3 +``` + +All steps within the allocation will run in the same Spindle session. +When the allocation exits, the session will terminate automatically. + +Sessions can be used with an `sbatch` script as shown below: + +```bash +#!/bin/bash +#SBATCH --spindle-session +#SBATCH -N 4 +#SBATCH -n 4 + +srun --spindle ./app1 +srun --spindle ./app2 +srun --spindle ./app3 +``` + From f24ecc5c7d71015e3e22971a497dd95657f2ac17 Mon Sep 17 00:00:00 2001 From: Nicholas Chaimov Date: Thu, 9 Jul 2026 12:35:38 -0700 Subject: [PATCH 5/9] Bug fixes in Slurm plugin - Fix double free of hostlist in case of error - Fix grandchild_fork exit check - Fix leaks in get_num_hosts_step, dropPrivilegeAndRun - Fix allocation size in isBEProc - Fix outdated fillInArgs args in SPLIT_CALLBACK_MODE --- src/slurm_plugin/plugin_utils.c | 6 +++--- src/slurm_plugin/slurm_plugin.c | 31 +++++++++++++++++++++---------- 2 files changed, 24 insertions(+), 13 deletions(-) diff --git a/src/slurm_plugin/plugin_utils.c b/src/slurm_plugin/plugin_utils.c index 87c2a37d..317d010d 100644 --- a/src/slurm_plugin/plugin_utils.c +++ b/src/slurm_plugin/plugin_utils.c @@ -240,7 +240,7 @@ char **getHostAddrSinfo(unsigned int num_hosts, char **hostlist) free(sinfo_cmdline); if (!ret && hostaddrlist) { for (i = 0; i < num_hosts; i++) free(hostaddrlist[i]); - free(hostlist); + free(hostaddrlist); } return ret; } @@ -549,7 +549,7 @@ int isBEProc(spindle_args_t *params, unsigned int exit_phase) strlen(hostname) + 1 + strlen(session_id_str) + 1; - unique_file = (char *) malloc(sizeof(char*) * unique_file_len); + unique_file = (char *) malloc(sizeof(char) * unique_file_len); snprintf(unique_file, unique_file_len, "%s/%s.%s.%s.%s", realized_dir, UNIQUE_FILE_NAME, phase_name, hostname, session_id_str); spindle_mkdir(realized_dir); @@ -1025,7 +1025,7 @@ pid_t grandchild_fork() sdprintf(1, "ERROR collecting pid after fork. Aborting spindle\n"); goto done; } - if (!WIFEXITED(status) && WEXITSTATUS(status) != 0) { + if (!WIFEXITED(status) || WEXITSTATUS(status) != 0) { sdprintf(1, "ERROR with invalid child exit during grandchild fork. Aborting spindle\n"); goto done; } diff --git a/src/slurm_plugin/slurm_plugin.c b/src/slurm_plugin/slurm_plugin.c index aa27f6bb..4664cc57 100644 --- a/src/slurm_plugin/slurm_plugin.c +++ b/src/slurm_plugin/slurm_plugin.c @@ -415,8 +415,8 @@ int slurm_spank_job_prolog(spank_t spank, int ac, char *argv[]) { start_params_t start_params; spank_err_t err; int result, use_session; - char *result_str, *work_dir, *envVal; - + char *result_str = NULL, *work_dir, *envVal; + handle_forwarded_environment(); use_session = should_use_session(spank); @@ -455,15 +455,18 @@ int slurm_spank_job_prolog(spank_t spank, int ac, char *argv[]) { return -1; } + if (result_str) + free(result_str); + return 0; } -/* job_epilog called on every compute node when allocation ends, +/* job_epilog called on every compute node when allocation ends, * regardless of whether any step ever ran on that node and * even if the prolog never ran. */ int slurm_spank_job_epilog(spank_t spank, int ac, char *argv[]) { int result, use_session; - char *result_str; + char *result_str = NULL; spank_err_t err; uid_t userid; exit_params_t exit_params; @@ -486,12 +489,15 @@ int slurm_spank_job_epilog(spank_t spank, int ac, char *argv[]) { exit_params.site_argv = argv; result = dropPrivilegeAndRun(handleExit, userid, &exit_params, &result_str); - + if (result == -1) { slurm_error("Failed to run handleExit. Spindle may not shutdown properly\n"); return -1; } - + + if (result_str) + free(result_str); + return 0; } @@ -630,7 +636,7 @@ static int handleStart(void *params, char **output_str) int slurm_spank_task_exit(spank_t spank, int site_argc, char *site_argv[]) { spank_context_t context; - char *result_str; + char *result_str = NULL; int result, use_session; uid_t userid; spank_err_t err; @@ -671,11 +677,15 @@ int slurm_spank_task_exit(spank_t spank, int site_argc, char *site_argv[]) push_env(spank, &saved_env); result = dropPrivilegeAndRun(handleExit, userid, &exit_params, &result_str); pop_env(saved_env); - + if (result == -1) { slurm_error("ERROR: Failed to run handleExit. Spindle may not shutdown properly\n"); return -1; } + + if (result_str) + free(result_str); + return 0; } @@ -890,6 +900,7 @@ static int get_num_hosts_step(spank_t spank) num_hosts_str = readSpankEnv(spank, "SLURM_STEP_NUM_NODES"); if (num_hosts_str) { result = atoi(num_hosts_str); + free(num_hosts_str); if (result > 0) return (int) result; } @@ -1018,7 +1029,7 @@ static int get_spindle_args(spank_t spank, spindle_args_t *params) goto done; } - result = fillInArgs(spank, params, spindle_argc, spindle_argv, unique_id); + result = fillInArgs(spank, params, spindle_argc, spindle_argv, unique_id, 0); if (result == -1) goto done; @@ -1121,7 +1132,7 @@ static int launch_spindle(spank_t spank, spindle_args_t *params) free(hostlist_job); } if (hostaddrlist) { - for (i = 0; i < num_hosts; i++) free(hostaddrlist[i]); + for (i = 0; i < num_hosts_fe; i++) free(hostaddrlist[i]); free(hostaddrlist); } From df62f65ff01b9ba78fa70be7d41b046f431400c3 Mon Sep 17 00:00:00 2001 From: Nicholas Chaimov Date: Thu, 9 Jul 2026 13:20:02 -0700 Subject: [PATCH 6/9] More robust Slurm plugin exit handling - Check exit socket path length - Verify that correct exit message received - Handle forwarded environment in exit --- src/slurm_plugin/plugin_utils.c | 71 ++++++++++++++++++++++----------- src/slurm_plugin/slurm_plugin.c | 2 + 2 files changed, 50 insertions(+), 23 deletions(-) diff --git a/src/slurm_plugin/plugin_utils.c b/src/slurm_plugin/plugin_utils.c index 317d010d..ef3bc996 100644 --- a/src/slurm_plugin/plugin_utils.c +++ b/src/slurm_plugin/plugin_utils.c @@ -360,6 +360,12 @@ static int createFEExitSocket(char *socket_path) int result, sock = -1, retval = -1; debug_printf("Creating unix socket for session at %s\n", socket_path); + + if (strlen(socket_path) > sizeof(local.sun_path)-1) { + err_printf("Session exit socket path too long: %s\n", socket_path); + goto done; + } + sock = socket(AF_UNIX, SOCK_STREAM, 0); if (sock == -1) { int error = errno; @@ -367,8 +373,14 @@ static int createFEExitSocket(char *socket_path) goto done; } + memset(&local, 0, sizeof(local)); local.sun_family = AF_UNIX; strncpy(local.sun_path, socket_path, sizeof(local.sun_path)-1); + + /* If there's an exit socket left over from a previous run that + * failed before removing it, remove it here */ + unlink(socket_path); + result = bind(sock, (struct sockaddr *) &local, sizeof(local)); if (result == -1) { int error = errno; @@ -419,32 +431,39 @@ int waitForSpankSessionEnd(spindle_args_t *params) goto done; sockfd = createFEExitSocket(socket_path); - if (sockfd == -1) - goto done; - - fd = accept(sockfd, NULL, NULL); - if (fd == -1) { - error = errno; - err_printf("Could not accept session exit socket connection: %s\n", strerror(error)); + if (sockfd == -1) goto done; - } - do { - result = read(fd, &msg, 1); - } while (result == -1 && errno == EINTR); - if (result == -1) { - error = errno; - err_printf("Failed to read from session exit socket: %s\n", strerror(error)); - goto done; - } - if (msg != 'q') { - error = errno; - err_printf("Recieved incorrect msg character: %c\n", msg); - goto done; + for (;;) { + fd = accept(sockfd, NULL, NULL); + if (fd == -1) { + error = errno; + err_printf("Could not accept session exit socket connection: %s\n", strerror(error)); + goto done; + } + + msg = 0; + do { + result = read(fd, &msg, 1); + } while (result == -1 && errno == EINTR); + + if (result == 1 && msg == 'q') { + close(fd); + fd = -1; + sdprintf(2, "Received session exit message\n"); + retval = 0; + break; + } + + if (result == -1) { + error = errno; + err_printf("Failed read from session exit socket: %s\n", strerror(error)); + } else { + sdprintf(2, "Received message other than exit on exit socket"); + } + close(fd); + fd = -1; } - - sdprintf(2, "Received session exit message\n"); - retval = 0; done: if (fd != -1) @@ -478,6 +497,12 @@ int signalSpankSessionEnd(spindle_args_t *params) goto done; } + if (strlen(socket_path) > sizeof(saddr.sun_path)-1) { + err_printf("Session exit socket path too long: %s\n", socket_path); + goto done; + } + + memset(&saddr, 0, sizeof(saddr)); saddr.sun_family = AF_UNIX; strncpy(saddr.sun_path, socket_path, sizeof(saddr.sun_path)-1); diff --git a/src/slurm_plugin/slurm_plugin.c b/src/slurm_plugin/slurm_plugin.c index 4664cc57..318f1afa 100644 --- a/src/slurm_plugin/slurm_plugin.c +++ b/src/slurm_plugin/slurm_plugin.c @@ -471,6 +471,8 @@ int slurm_spank_job_epilog(spank_t spank, int ac, char *argv[]) { uid_t userid; exit_params_t exit_params; + handle_forwarded_environment(); + use_session = should_use_session(spank); if (!use_session) From 54a3ed1d14744f4a2c0618fdc92405eaeb4434e6 Mon Sep 17 00:00:00 2001 From: Nicholas Chaimov Date: Thu, 9 Jul 2026 14:50:40 -0700 Subject: [PATCH 7/9] Bug fixes in Spindle plugin - Check handleStart failure in addition to prepApp - Only use step ID env var in job script context - Check realize failure in fillInArgs --- src/slurm_plugin/plugin_utils.c | 4 ++++ src/slurm_plugin/slurm_plugin.c | 31 ++++++++++++++++++++++--------- 2 files changed, 26 insertions(+), 9 deletions(-) diff --git a/src/slurm_plugin/plugin_utils.c b/src/slurm_plugin/plugin_utils.c index ef3bc996..3fa85179 100644 --- a/src/slurm_plugin/plugin_utils.c +++ b/src/slurm_plugin/plugin_utils.c @@ -560,6 +560,10 @@ int isBEProc(spindle_args_t *params, unsigned int exit_phase) int fd = -1, error; realized_dir = locSpecificDir(params); + if (!realized_dir) { + sdprintf(1, "ERROR: Could not resolve location directory in isBEProc\n"); + goto done; + } gethostname(hostname, sizeof(hostname)); hostname[sizeof(hostname)-1] = '\0'; diff --git a/src/slurm_plugin/slurm_plugin.c b/src/slurm_plugin/slurm_plugin.c index 318f1afa..3d697c1d 100644 --- a/src/slurm_plugin/slurm_plugin.c +++ b/src/slurm_plugin/slurm_plugin.c @@ -568,6 +568,10 @@ int slurm_spank_task_init(spank_t spank, int site_argc, char *site_argv[]) start_params.site_argv = site_argv; result = handleStart(&start_params, &result_str); + if (result == -1) { + sdprintf(1, "Error launching spindle. Aborting spindle\n"); + goto done; + } } result = prepApp(spank, ¶ms); @@ -696,18 +700,21 @@ static spank_err_t get_stepid(spank_t spank, uint32_t *stepid) { char *slurm_step_id_s; spank_err_t err; - uint64_t combined; - - slurm_step_id_s = getenv("SLURM_STEP_ID"); - if (slurm_step_id_s) { - *stepid = (uint32_t) atol(slurm_step_id_s); - } else { - err = spank_get_item(spank, S_JOB_STEPID, stepid); - if (err != ESPANK_SUCCESS) { - return err; + + /* Only get step ID from env var in job script context */ + if (spank_context() == S_CTX_JOB_SCRIPT) { + slurm_step_id_s = getenv("SLURM_STEP_ID"); + if (slurm_step_id_s) { + *stepid = (uint32_t) atol(slurm_step_id_s); + return ESPANK_SUCCESS; } } + err = spank_get_item(spank, S_JOB_STEPID, stepid); + if (err != ESPANK_SUCCESS) { + return err; + } + return ESPANK_SUCCESS; } @@ -796,6 +803,12 @@ static int fillInArgs(spank_t spank, spindle_args_t *args, int argc, char **argv return -1; } args->commpath = realize(orig_commpath); + free(orig_commpath); + if (!args->commpath) { + slurm_error("Spindle Options Error: Could not resolve commpath location\n"); + sdprintf(1, "ERROR: Could not realize commpath from '%s'\n", symbolic_commpath); + return -1; + } current_spank = spank; From b804aca682400fef9a4eb83e13d7c9408d4f7ae9 Mon Sep 17 00:00:00 2001 From: Nicholas Chaimov Date: Thu, 9 Jul 2026 15:19:21 -0700 Subject: [PATCH 8/9] Bug fixes in Slurm plugin - Set current_slurm first so we can look up job control env vars - Don't leak orig_commpath if realize created a new one - Use env var in commpath in Slurm plugin container to test that branch of realize --- .../testing-plugin/scripts/build_spindle.sh | 2 +- src/slurm_plugin/plugin_utils.c | 2 +- src/slurm_plugin/slurm_plugin.c | 7 ++++--- 3 files changed, 6 insertions(+), 5 deletions(-) diff --git a/containers/spindle-slurm-ubuntu/testing-plugin/scripts/build_spindle.sh b/containers/spindle-slurm-ubuntu/testing-plugin/scripts/build_spindle.sh index 8b759415..a52917ba 100755 --- a/containers/spindle-slurm-ubuntu/testing-plugin/scripts/build_spindle.sh +++ b/containers/spindle-slurm-ubuntu/testing-plugin/scripts/build_spindle.sh @@ -3,7 +3,7 @@ set -euxo pipefail mkdir -p /home/${USER}/Spindle-build cd /home/${USER}/Spindle-build -/home/${USER}/Spindle/configure --prefix=/home/${USER}/Spindle-inst --enable-sec-munge --with-rm=slurm-plugin --enable-slurm-plugin --with-cachepaths=/tmp/commpath/cachepath --with-commpath=/tmp/commpath --enable-crash-dedup CFLAGS="-O2 -g" CXXFLAGS="-O2 -g" +/home/${USER}/Spindle/configure --prefix=/home/${USER}/Spindle-inst --enable-sec-munge --with-rm=slurm-plugin --enable-slurm-plugin --with-cachepaths=/tmp/commpath/cachepath --with-commpath='$TMPDIR/commpath' --enable-crash-dedup CFLAGS="-O2 -g" CXXFLAGS="-O2 -g" make -j$(nproc) make install diff --git a/src/slurm_plugin/plugin_utils.c b/src/slurm_plugin/plugin_utils.c index 3fa85179..3a9b8639 100644 --- a/src/slurm_plugin/plugin_utils.c +++ b/src/slurm_plugin/plugin_utils.c @@ -1326,7 +1326,7 @@ char *readSpankEnv(spank_t spank, const char *envname) free(buffer); buffer = (char *) malloc(buffer_size); } - if (err == ESPANK_ENV_NOEXIST) { + if (err == ESPANK_ENV_NOEXIST || err == ESPANK_NOT_REMOTE || err == ESPANK_BAD_ARG) { free(buffer); buffer = getenv(envname); return buffer ? strdup(buffer) : NULL; diff --git a/src/slurm_plugin/slurm_plugin.c b/src/slurm_plugin/slurm_plugin.c index 3d697c1d..cd102db5 100644 --- a/src/slurm_plugin/slurm_plugin.c +++ b/src/slurm_plugin/slurm_plugin.c @@ -772,6 +772,8 @@ static int fillInArgs(spank_t spank, spindle_args_t *args, int argc, char **argv char *symbolic_commpath, *orig_commpath; char *err_string; + current_spank = spank; + args->unique_id = unique_id; args->number = (number_t) args->unique_id; result = fillInSpindleArgsCmdlineFE(args, SPINDLE_FILLARGS_NOUNIQUEID | SPINDLE_FILLARGS_NONUMBER, @@ -803,15 +805,14 @@ static int fillInArgs(spank_t spank, spindle_args_t *args, int argc, char **argv return -1; } args->commpath = realize(orig_commpath); - free(orig_commpath); + if (args->commpath != orig_commpath) + free(orig_commpath); if (!args->commpath) { slurm_error("Spindle Options Error: Could not resolve commpath location\n"); sdprintf(1, "ERROR: Could not realize commpath from '%s'\n", symbolic_commpath); return -1; } - current_spank = spank; - return 0; } From 8f41962b50ab0a74d0dc9110ec929f7f0b70f016 Mon Sep 17 00:00:00 2001 From: Nicholas Chaimov Date: Wed, 5 Aug 2026 16:22:48 -0700 Subject: [PATCH 9/9] Forward env vars to Slurm job control in every launch mode --- src/slurm_plugin/slurm_plugin.c | 7 ++----- 1 file changed, 2 insertions(+), 5 deletions(-) diff --git a/src/slurm_plugin/slurm_plugin.c b/src/slurm_plugin/slurm_plugin.c index cd102db5..468e2f36 100644 --- a/src/slurm_plugin/slurm_plugin.c +++ b/src/slurm_plugin/slurm_plugin.c @@ -213,10 +213,7 @@ int slurm_spank_init_post_opt(spank_t spank, int ac, char *argv[]) { setenv(SPANK_SPINDLE_USE_SESSION, "1", 1); } - /* With PrologFlags=Alloc, forward env vars to job control here; - them. Without PrologFlags=Alloc, this forwarding happens later - in local context (srun). */ - if (prolog_alloc_mode && start_session) { + if (start_session) { int result = forward_environment_to_job_control(spank); if (result == -1) { slurm_error("ERROR: Spindle plugin error. Unable to forward environment variables to job control.\n"); @@ -355,7 +352,7 @@ int slurm_spank_local_user_init(spank_t spank, int ac, char *argv[]) if (!use_session) goto done; - if (prolog_alloc_mode) + if (prolog_alloc_mode) goto done; result = process_spindle_args(spank, ac, argv, ¶ms, NULL, NULL, use_session);