upssched: run expired timer commands asynchronously

NEWS.adoc

@@ -48,6 +48,9 @@ https://github.com/networkupstools/nut/milestone/13
     * `libnutconf`: character reads from a TCP socket now use the Windows
       socket API, matching string reads. Previously they failed with a bad
       file descriptor error on Windows. [issue #2293]
+    * `upssched` runs commands from expired timers asynchronously, so a slow
+      command no longer delays other timers or timer requests. The daemon
+      waits for these commands before exiting. [issue #3115, PR #3769]
     * Now `upssched -l` preserves timer expiry timestamps beyond January 2038
       on systems with 64-bit `time_t` and 32-bit `long`. [issue #3096, PR #3669]
     * PyNUT: `GetUPSCommands()` now requests command descriptions with the

UPGRADING.adoc

@@ -31,6 +31,13 @@ command line, in order to quickly pick up any other removed option flags.
 Changes from 2.8.5 to 2.8.6
 ---------------------------
 
+- `upssched` timer commands now run asynchronously. Commands from different
+  timers, or successive invocations of the same timer name, may overlap.
+  Scripts which relied on serial execution must arrange their own locking.
+  On idle exit, the timer daemon waits for its directly launched commands.
+  Immediate `EXECUTE` and unmatched `CANCEL-TIMER` command handling retain
+  their synchronous behaviour. [issue #3115, PR #3769]
+
 - PLANNED: Keep track of any further API clean-up?
 
 - The `PACKAGE_VERSION` and `PACKAGE_STRING` macros in the generated

clients/upssched.c

@@ -45,7 +45,7 @@
  *
  * timers can be cancelled at any time before they trigger
  *
- * the daemon will shut down automatically when no more timers are active
+ * the daemon shuts down after all timers and their direct commands finish
  *
  */
 
@@ -149,17 +149,30 @@
 #define US_SOCK_BUF_LEN		256
 #define US_MAX_READ		128
 
+typedef struct command_s {
+#ifndef WIN32
+	pid_t	pid;
+#else
+	intptr_t	pid;
+#endif
+	char	*name;
+	struct command_s	*next;
+} command_t;
+
+static command_t	*commands = NULL;
+
 /* --- server functions --- */
 
-static void exec_cmd(const char *cmd)
+static void exec_cmd(const char *cmd, int asynchronous)
 {
 #ifndef WIN32
 	int	waitstatus = 0;
 	pid_t	pid, waitret;
 #else
-	int	err = 0;
+	intptr_t	err = 0;
 #endif
 	char	**argv = NULL;
+	command_t	*child;
 
 	if (cmdscript_argc == 0) {
 		upslogx(LOG_ERR, "No CMDSCRIPT defined, cannot execute command: %s", NUT_STRARG(cmd));
@@ -177,6 +190,15 @@
 	}
 
 	if (pid > 0) {
+		if (asynchronous) {
+			child = (command_t *)xcalloc(1, sizeof(*child));
+			child->pid = pid;
+			child->name = xstrdup(cmd);
+			child->next = commands;
+			commands = child;
+			return;
+		}
+
 		/* parent process - wait for child */
 		waitret = waitpid(pid, &waitstatus, 0);
 		if (waitret < 0) {
@@ -214,17 +236,27 @@
 
 #ifndef WIN32
 	/* child process */
 	execvp(cmdscript_argv[0], argv);
 	/* execvp() only returns on error */
 	upslog_with_errno(LOG_ERR, "execvp(%s) failed", NUT_STRARG(cmdscript_concat));
 	free(argv);
 	exit(EXIT_FAILURE);
 #else	/* WIN32 */
 	/* Use _spawnvp for Windows */
-	err = _spawnvp(_P_WAIT, cmdscript_argv[0], (const char * const *)argv);
+	err = _spawnvp(asynchronous ? _P_NOWAIT : _P_WAIT,
+		cmdscript_argv[0], (const char * const *)argv);
+	if (asynchronous && err != -1) {
+		child = (command_t *)xcalloc(1, sizeof(*child));
+		child->pid = err;
+		child->name = xstrdup(cmd);
+		child->next = commands;
+		commands = child;
+		free(argv);
+		return;
+	}
 	if (err != -1) {
 		upslogx(LOG_INFO, "Execute command %s \"%s\" OK", NUT_STRARG(cmdscript_concat), NUT_STRARG(cmd));
-		upsdebugx(3, "%s: returned status %d", __func__, err);
+		upsdebugx(3, "%s: returned status %d", __func__, (int)err);
 	} else {
 		upslog_with_errno(LOG_ERR, "Execute command %s \"%s\" failure", NUT_STRARG(cmdscript_concat), NUT_STRARG(cmd));
 	}
@@ -317,6 +349,57 @@
 	return buf;
 }
 
+/* Poll only commands owned by timer dispatch; do not collect other children. */
+static void reap_commands(void)
+{
+	command_t **link = &commands, *child;
+
+	while ((child = *link) != NULL) {
+#ifndef WIN32
+		int status = 0;
+		pid_t ret = waitpid(child->pid, &status, WNOHANG);
+
+		if (ret == 0 || (ret < 0 && errno == EINTR)) {
+			link = &child->next;
+			continue;
+		}
+		if (ret < 0) {
+			upslog_with_errno(LOG_ERR, "waitpid(%d) failed", (int)child->pid);
+		} else if (WIFEXITED(status)) {
+			if (WEXITSTATUS(status)) {
+				upslogx(LOG_INFO, "exec_cmd(%s '%s') returned %d",
+					NUT_STRARG(cmdscript_concat), child->name, WEXITSTATUS(status));
+			}
+		} else if (WIFSIGNALED(status)) {
+			upslogx(LOG_WARNING, "exec_cmd(%s '%s') terminated with signal %d",
+				NUT_STRARG(cmdscript_concat), child->name, WTERMSIG(status));
+		} else {
+			upslogx(LOG_ERR, "Execute command failure: %s '%s'",
+				NUT_STRARG(cmdscript_concat), child->name);
+		}
+#else
+		DWORD status = 0;
+		DWORD ret = WaitForSingleObject((HANDLE)child->pid, 0);
+
+		if (ret == WAIT_TIMEOUT) {
+			link = &child->next;
+			continue;
+		}
+		if (ret != WAIT_OBJECT_0 || !GetExitCodeProcess((HANDLE)child->pid, &status)) {
+			upslogx(LOG_ERR, "Cannot collect command '%s': Windows error %lu",
+				child->name, (unsigned long)GetLastError());
+		} else if (status) {
+			upslogx(LOG_INFO, "exec_cmd(%s '%s') returned %lu",
+				NUT_STRARG(cmdscript_concat), child->name, (unsigned long)status);
+		}
+		CloseHandle((HANDLE)child->pid);
+#endif
+		*link = child->next;
+		free(child->name);
+		free(child);
+	}
+}
+
 static void exec_cmd_timer(ttype_t *item)
 {
 	char	*upsnames = NULL, *notifytypes = NULL, *notifymsgs = NULL;
@@ -352,7 +435,7 @@
 	if (nut_debug_level)
 		upslogx(LOG_INFO, "Executing command by timer: %s\t[%s]\t[%s]\t[%s]",
 			item->name, NUT_STRARG(notifytypes), NUT_STRARG(upsnames), NUT_STRARG(notifymsgs));
-	exec_cmd(item->name);
+	exec_cmd(item->name, 1);
 	upsdebugx(3, "%s: returned from exec_cmd()", __func__);
 
 	/* Timer process should not retain random envvars */
@@ -436,8 +519,13 @@
 
 	upsdebugx(3, "%s: starting", __func__);
 
-	/* if the queue is empty we might be ready to exit */
-	if (!thead) {
+	reap_commands();
+	if (commands) {
+		emptyctr = 0;
+	}
+
+	/* Running commands keep the daemon alive, even with no queued timers. */
+	if (!thead && !commands) {
 
 		emptyctr++;
 
@@ -726,7 +814,7 @@
 		if (nut_debug_level)
 			upslogx(LOG_INFO, "Cancel %s, event: %s", name, cname);
 
-		exec_cmd(cname);
+		exec_cmd(cname, 0);
 	}
 }
 
@@ -2419,7 +2507,7 @@
 		if (nut_debug_level)
 			upslogx(LOG_INFO, "Executing command: %s", ca1);
 
-		exec_cmd(ca1);
+		exec_cmd(ca1, 0);
 		return;
 	}
 
@@ -2649,6 +2737,16 @@
 	ttype_t	*tcurr, *tnext;
 	conn_t	*ccurr, *cnext;
 	size_t	i;
+	command_t	*child;
+
+	while ((child = commands) != NULL) {
+		commands = child->next;
+#ifdef WIN32
+		CloseHandle((HANDLE)child->pid);
+#endif
+		free(child->name);
+		free(child);
+	}
 
 	/* Flush *our* output before possibly failing in third-party code
 	 * (e.g. SSL libs), so client consumers have a chance to see it */

docs/man/upssched.conf.txt

@@ -18,12 +18,13 @@ IMPORTANT NOTES
 * Contents of this file should be pure ASCII (character codes
   not in range would be ignored with a warning message).
 
-* Command execution is synchronous (with the called tool process in case
-  of `EXECUTE` directive, or with the timer process). Consider using your
-  system shell abilities like `&` to send long-duration handling to the
-  background and let `upssched` timer daemon continue. This should not
-  impact `upsmon` daemon, which handles each notification in a separate
-  sub-process (and so not a problem for immediate `EXECUTE` events).
+* Commands launched by expired timers run asynchronously and may overlap,
+  including invocations of the same timer name. Use locking in scripts
+  which require serial execution. On idle exit, the timer daemon waits until
+  its directly launched commands finish, then follows its idle grace period.
+  Immediate `EXECUTE` commands wait in the calling tool process. The optional
+  command for `CANCEL-TIMER` with no matching timer still waits in the timer
+  daemon. `upsmon` handles notifications in separate sub-processes.
 
 CONFIGURATION DIRECTIVES
 ------------------------

docs/man/upssched.txt

@@ -81,13 +81,15 @@ If you also want to continue writing to the syslog, just add it in:
 
 For a full list of notify flags, see the linkman:upsmon[8] documentation.
 
-Please note that command execution is synchronous (with the called `upssched`
-tool process in case of `EXECUTE` directive, or with the timer process).
-Consider using your system shell abilities like `&` to send long-duration
-handling to the background and let `upssched` timer daemon continue.
-This should not impact `upsmon` daemon, which handles each notification
-in a separate sub-process (and so not a problem for immediate `EXECUTE`
-events).
+Commands launched by expired timers run asynchronously. A long-running command
+therefore does not delay other timers or requests to list, start or cancel
+pending timers. Commands may overlap, including successive invocations of the
+same timer name. Scripts which require serial execution must arrange their
+own locking.
+
+Immediate `EXECUTE` commands wait in the calling tool process. The optional
+command for a `CANCEL-TIMER` with no matching timer still waits in the timer
+daemon. `upsmon` handles each notification in a separate sub-process.
 
 CONFIGURATION
 -------------
@@ -157,12 +159,13 @@ It will come and go from the process list as necessary.  When a new timer
 is started, a process will be forked to actually watch the clock and
 eventually start the CMDSCRIPT.  When a timer triggers, it is removed from
 the queue.  Cancelling a timer will also remove it from the queue.  When
-no timers are present in the queue, the background process exits.
+no timers are present in the queue and all commands launched by timers have
+finished, the background process exits after its idle grace period.
 
 This means that you will only see upssched running when one of two things
 is happening:
 
- - There's a timer of some sort currently running
+ - There's a pending timer or a command launched by a timer still running
  - upsmon just called it, and you managed to catch the brief instance
 
 The final optimization handles the possibility of trying to cancel a timer
@@ -180,7 +183,7 @@ runs in foreground mode.
 NOTE: Unlike some other NUT daemons, `upssched` with enabled debug
 does not stop reporting on `stderr`! It forks a background process
 with the first call as an event handler, which exits soon after all
-tracked timers have elapsed and were handled (if needed).
+tracked timers have elapsed and their directly launched commands have finished.
 
 *UPSNAME* and *NOTIFYTYPE* are required, as detailed above. They are
 set by `upsmon` when it calls `upssched` as its choice of `NOTIFYCMD`.

tests/Makefile.am

@@ -154,6 +154,12 @@ upssched_list_timers_test_CFLAGS = $(AM_CFLAGS) -I$(top_srcdir)/clients
 upssched_list_timers_test_LDADD = $(top_builddir)/common/libcommonversion.la $(NUT_LIBCOMMON) $(NETLIBS)
 upssched_list_timers_test_DEPENDENCIES = $(NUT_LIBCOMMON) $(top_builddir)/common/libcommonversion.la $(top_srcdir)/clients/upssched.c $(top_srcdir)/clients/upssched.h
 
+TESTS += upssched-async-test
+upssched_async_test_SOURCES = upssched-async-test.c
+upssched_async_test_CFLAGS = $(AM_CFLAGS) -I$(top_srcdir)/clients
+upssched_async_test_LDADD = $(top_builddir)/common/libcommonversion.la $(NUT_LIBCOMMON) $(NETLIBS)
+upssched_async_test_DEPENDENCIES = $(NUT_LIBCOMMON) $(top_builddir)/common/libcommonversion.la $(top_srcdir)/clients/upssched.c $(top_srcdir)/clients/upssched.h
+
 TESTS += statetest
 statetest_SOURCES = statetest.c
 statetest_LDADD = $(NUT_LIBCOMMON)

tests/upssched-async-test.c

@@ -0,0 +1,65 @@
+/* upssched-async-test - timer command ownership and collection
+ *
+ * Copyright (C) 2026 Network UPS Tools project
+ * This program is free software; you can redistribute it and/or modify
+ * it under the terms of the GNU General Public License as published by
+ * the Free Software Foundation; either version 2 of the License, or
+ * (at your option) any later version.
+ */
+
+#include "config.h"
+
+/* Include the implementation to test private command ownership. */
+int upssched_main(int argc, char **argv);
+#define main upssched_main
+#include "upssched.c"
+#undef main
+
+int main(int argc, char **argv)
+{
+	command_t *child;
+	int count = 0, attempts;
+	static char *script[3];
+
+	if (argc == 3 && !strcmp(argv[1], "--command")) {
+		if (!strcmp(argv[2], "slow")) {
+			sleep(8);
+		}
+		return 0;
+	}
+
+	script[0] = argv[0];
+	script[1] = (char *)"--command";
+	script[2] = NULL;
+	cmdscript_argv = script;
+	cmdscript_argc = 2;
+	cmdscript_concat = argv[0];
+
+	exec_cmd("slow", 1);
+	exec_cmd("fast", 1);
+	for (attempts = 0; attempts < 3; attempts++) {
+		sleep(1);
+		reap_commands();
+		count = 0;
+		for (child = commands; child; child = child->next) {
+			count++;
+		}
+		if (count == 1) {
+			break;
+		}
+	}
+	if (count != 1 || strcmp(commands->name, "slow")) {
+		fprintf(stderr, "FAIL: fast command must finish while slow command runs\n");
+		return 1;
+	}
+	for (attempts = 0; commands && attempts < 10; attempts++) {
+		sleep(1);
+		reap_commands();
+	}
+	if (commands) {
+		fprintf(stderr, "FAIL: completed commands must be collected\n");
+		return 1;
+	}
+	puts("PASS: independent timer command execution and collection");
+	return 0;
+}