diff --git a/NEWS.adoc b/NEWS.adoc index 8275b373a8..4728f271ac 100644 --- a/NEWS.adoc +++ b/NEWS.adoc @@ -266,6 +266,7 @@ https://github.com/networkupstools/nut/milestone/13 - `upssched` client/tool updates: * Fixed handling of `NOTIFYMSG` from command line if other arguments are present (e.g. debugging with `-DDDDDD`). [issues #3105, #3525, PR #3527] + * Rewrote the WIN32 code path to spawn the timer daemon. [issue #3525] - `NUT-Monitor` Python GUI client: * Fixed Qt tray tooltips to render in plain text, not rich text which diff --git a/clients/upssched.c b/clients/upssched.c index 5eea36c015..fe77c8d544 100644 --- a/clients/upssched.c +++ b/clients/upssched.c @@ -93,6 +93,46 @@ static size_t cmdscript_argc = 0; static int nut_debug_level_args = 0, nut_debug_level_env = 0, nut_debug_level_conf = 0; static int list_timers = 0; +#ifdef WIN32 +/* WIN32-only: set when THIS process instance is itself the freshly + * spawned timer-daemon child (see WIN32_DAEMON_MARKER below), as + * opposed to being the short-lived CLI process invoked by upsmon. + * + * This flag makes conf_arg() treat the config file the same way it + * already does for the "-l" (list_timers) CLI switch: parse the + * global upssched.conf directives (PIPEFN, LOCKFN, CMDSCRIPT, + * DEBUG_MIN), but skip acting on "AT" lines (i.e. skip parse_at() + * and, transitively, sendcmd()) -- a daemon child must never re-send + * the notification that originally caused upsmon to invoke this + * program; that is precisely the bug this patch addresses. */ +static int is_win32_timer_daemon = 0; + +/* Hidden/internal argv[1] marker, used only for the WIN32 replacement + * of fork(). CreateProcess() cannot split one already-running process + * into a "parent continues here" / "child takes over from here" pair + * the way fork() does on POSIX (see the "if (pid != 0) { ...; return; }" + * branch in start_daemon() below) -- it always starts a brand-new + * process at main(). We therefore spawn a new process and tell it, + * via this marker (checked at the very top of main(), before normal + * option/config parsing), that it must go straight to being the timer + * daemon instead of re-running the normal CLI codepath (which would + * re-parse the config and potentially re-send the very notification + * that caused upsmon to invoke this program -- the original cause of + * the runaway process cascade this patch fixes). */ +#define WIN32_DAEMON_MARKER "--nut-upssched-win32-timer-daemon" + +/* HANDLE of a manual-reset Win32 event object, inherited from the + * spawning process, used as a one-shot "I am listening on the named + * pipe now" readiness signal from the daemon child back to the + * process that spawned it (see start_daemon() and + * win32_run_timer_daemon() below). This is the WIN32 counterpart of + * us_serialize(SERIALIZE_WAIT/SET), which is implemented below with a + * plain POSIX pipe(2) and has no WIN32 implementation. + * Populated from argv[2] in main() when is_win32_timer_daemon is set; + * remains NULL otherwise. */ +static HANDLE win32_daemon_ready_event = NULL; +#endif /* WIN32 */ + /* ups name and notify type (string) as received from upsmon */ static const char *ups_name = NULL, *notify_type = NULL, *notify_msg = NULL, *prog = NULL; @@ -1334,12 +1374,11 @@ static int sock_read(conn_t *conn) static void start_daemon(TYPE_FD lockfd) { +#ifndef WIN32 int maxfd = 0; /* Unidiomatic use vs. "pipefd" below, which is "int" on non-WIN32 */ TYPE_FD pipefd; struct timeval tv; conn_t *tmp; - -#ifndef WIN32 int pid, ret; fd_set rfds; conn_t *tmpnext; @@ -1522,37 +1561,246 @@ static void start_daemon(TYPE_FD lockfd) #else /* WIN32 */ - DWORD timeout_ms; - HANDLE rfds[32]; + /* NOTE ON THIS REWRITE: + * + * This function is only ever called by the short-lived CLI process + * (the one invoked by upsmon with UPSNAME/NOTIFYTYPE set) when it + * could not connect to an already-running timer daemon. + * + * Historically (see version control history for the code this + * replaces), this WIN32 branch did NOT fork(): it called + * CreateProcess(module, NULL, ...) -- i.e. spawned a brand-new, + * unmarked copy of this very executable, which (since + * lpEnvironment==NULL) inherited this process' current + * environment, INCLUDING UPSNAME/NOTIFYTYPE -- and then THIS + * process itself (not the spawned one!) called open_sock() and + * ran the server for(;;) loop forever, i.e. becoming the daemon. + * + * That spawned copy had no way of knowing it was supposed to + * "become the daemon": unlike fork(), CreateProcess() cannot + * split this already-initialized process into a parent/child + * pair -- it always starts a brand-new process at main(). So the + * spawned copy simply re-ran the entire normal CLI codepath + * (re-parsing upssched.conf, calling parse_at() and sendcmd() + * again) as if upsmon had invoked upssched a second time for the + * very same event. Combined with a lock-file race in + * check_parent() (see the unconditional DeleteFile(lockfn) a few + * lines above in that function), this could make the spawned + * copy conclude that IT also needed to start a daemon, spawning + * yet another copy, and so on -- a runaway cascade of processes + * instead of the intended single client + single daemon pair. + * + * The rewrite below fixes this by: + * 1. Explicitly marking the spawned process (via + * WIN32_DAEMON_MARKER on its command line, checked at the + * very top of main()) so it goes straight to + * win32_run_timer_daemon() instead of the normal CLI path. + * 2. Clearing UPSNAME/NOTIFYTYPE/NOTIFYMSG from our own + * environment before spawning, since CreateProcess() with + * lpEnvironment==NULL takes a snapshot of it for the child. + * 3. Adding an explicit readiness handshake (a manual-reset + * Win32 event, inherited by the child) so THIS process waits + * until the child is actually listening on the named pipe, + * then RETURNS -- exactly like the non-WIN32 parent branch + * above returns after us_serialize(SERIALIZE_WAIT). This + * process is NOT the daemon: control goes back to + * check_parent(), which retries try_connect() and delivers + * the ORIGINAL command to the now-running daemon, and this + * process then exits normally, same as on non-WIN32. + */ char module[NUT_PATH_MAX + 1]; + char cmdline[NUT_PATH_MAX + 64]; + char ready_event_str[32]; STARTUPINFO sinfo; PROCESS_INFORMATION pinfo; + SECURITY_ATTRIBUTES sa; + HANDLE hReady; + DWORD wait_ret; + if (!GetModuleFileName(NULL, module, sizeof(module))) { fatal_with_errno(EXIT_FAILURE, "Can't retrieve module name"); } - memset(&sinfo,0,sizeof(sinfo)); - if (!CreateProcess(module, NULL, NULL, NULL, FALSE, 0, NULL, NULL, &sinfo, &pinfo)) { - fatal_with_errno(EXIT_FAILURE, "Can't create child process"); + + /* Manual-reset event, created inheritable, so the about-to-be + * spawned child can SetEvent() it once its named pipe is up. This + * is the WIN32 counterpart of us_serialize(SERIALIZE_INIT), which + * is implemented above with a plain POSIX pipe(2) and has no + * WIN32 equivalent. */ + memset(&sa, 0, sizeof(sa)); + sa.nLength = sizeof(sa); + sa.lpSecurityDescriptor = NULL; + sa.bInheritHandle = TRUE; + + hReady = CreateEvent(&sa, TRUE /* manual-reset */, FALSE /* initially non-signaled */, NULL); + if (hReady == NULL) { + fatal_with_errno(EXIT_FAILURE, "Can't create timer daemon readiness event"); + } + + /* Hand the readiness-event HANDLE down to the child as a hex + * string on its command line, alongside WIN32_DAEMON_MARKER (see + * main() for the receiving side). This relies on the handle + * actually being inherited by the child process, which requires + * BOTH bInheritHandle=TRUE on the handle itself (set above) AND + * bInheritHandles=TRUE in the CreateProcess() call below. + * + * NOTE for the developers reading this years down the road: + * passing a raw HANDLE value across a process boundary via argv, + * encoded as text (with hex value), is somewhat unusual (because a + * named/well-known event, or the newer PROC_THREAD_ATTRIBUTE_HANDLE_LIST + * mechanism, are the more modern alternatives), but it is simple, + * requires no extra Windows SDK version checks, and is a + * well-documented pattern for inherited handle hand-off predating + * Vista-era API additions. + * strtoull()/"%llx" are used here for portability across the + * MinGW-w64 and MSVC toolchains this project supports; please + * double check this against the actual minimum supported + * toolchain versions in this codebase's build files, should + * any target OS level ever get officially deprecated by the + * NUT project. */ + snprintf(ready_event_str, sizeof(ready_event_str), "%llx", + (unsigned long long)(uintptr_t)hReady); + snprintf(cmdline, sizeof(cmdline), "\"%s\" %s %s", + module, WIN32_DAEMON_MARKER, ready_event_str); + + /* Whatever upsmon envvars are currently set (UPSNAME, NOTIFYTYPE, + * NOTIFYMSG) must NOT leak into the spawned child: unlike fork(), + * CreateProcess() with lpEnvironment==NULL makes the child inherit + * a snapshot of THIS process' current environment block -- and it + * was exactly that inheritance which historically let the (back + * then, unmarked) spawned copy believe it was handling a fresh + * notification from upsmon. The child is not expected to consult + * these variables at all now (is_win32_timer_daemon short-circuits + * that in main()/conf_arg()), but we clear them here too, out of + * caution. */ + unsetenv("UPSNAME"); + unsetenv("NOTIFYTYPE"); + unsetenv("NOTIFYMSG"); + + memset(&sinfo, 0, sizeof(sinfo)); + sinfo.cb = sizeof(sinfo); + + /* bInheritHandles=TRUE is required for the child to receive a + * usable duplicate of hReady. */ + if (!CreateProcess(module, cmdline, NULL, NULL, TRUE, 0, NULL, NULL, &sinfo, &pinfo)) { + CloseHandle(hReady); + fatal_with_errno(EXIT_FAILURE, "Can't create timer daemon child process"); + } + + /* We don't need to track or wait on the child process/thread + * handles beyond this point -- only the readiness event matters + * here. (The code this replaces never closed these two handles, + * leaking a HANDLE pair on every daemon start; fixed here.) */ + CloseHandle(pinfo.hThread); + CloseHandle(pinfo.hProcess); + + /* Wait for the child to open its named pipe and call SetEvent(); + * this is the WIN32 equivalent of us_serialize(SERIALIZE_WAIT). + * A finite timeout avoids hanging forever if the child fails to + * start for some unrelated reason -- we fail loudly instead of + * silently proceeding as if the daemon were already up. */ + wait_ret = WaitForSingleObject(hReady, 10000 /* ms; NOTE: adjust if too tight/loose for the target environment */); + CloseHandle(hReady); + + if (wait_ret != WAIT_OBJECT_0) { + fatal_with_errno(EXIT_FAILURE, + "Timer daemon child did not signal readiness in time"); } - pipefd = open_sock(); if (nut_debug_level) - upslogx(LOG_INFO, "Timer daemon started"); + upslogx(LOG_INFO, "Timer daemon child confirmed ready"); - /* drop the lock now that the background is running */ + /* We (the CLI process) are NOT the daemon. Release our lock and + * return to the caller (check_parent()), which will retry + * try_connect() and deliver the ORIGINAL command via the named + * pipe, then exit -- mirroring the non-WIN32 parent branch above. */ CloseHandle(lockfd); DeleteFile(lockfn); + + return; +#endif /* WIN32 */ + + /* Should not get here */ +/* + if (nut_debug_level) + upslogx(LOG_INFO, "Timer daemon ending"); +*/ +} + +#ifdef WIN32 +/* WIN32-only: body of the persistent timer-daemon process. + * + * Entered directly from main() when WIN32_DAEMON_MARKER is found on + * the command line (see main() below), i.e. this is the freshly + * spawned child created by start_daemon() above via CreateProcess(). + * This process never goes through the normal CLI codepath: by the + * time main() calls us, checkconf() has already run (gated by + * is_win32_timer_daemon so that only PIPEFN/LOCKFN/CMDSCRIPT/ + * DEBUG_MIN are parsed and no "AT" line is acted upon -- see + * conf_arg()). + * + * This is the WIN32 counterpart of the "child" branch of the + * non-WIN32 start_daemon() above (the code path taken when + * "pid != 0" is false, i.e. after the "if (pid != 0) { ...; return; }" + * block) -- except it necessarily lives in its own function, since + * unlike fork() it cannot simply "fall through" from inside + * start_daemon(): it is a distinct OS process running its own, + * separate main(). + * + * Below this point, down to the closing brace, the loop body is + * relocated VERBATIM from the previous (buggy) WIN32 branch of + * start_daemon() -- no functional change was made to the connection/ + * timer-handling logic itself, only to how and by which process this + * code gets reached. */ +static void win32_run_timer_daemon(void) +{ + int maxfd = 0; + TYPE_FD pipefd; + struct timeval tv; + conn_t *tmp; + DWORD timeout_ms; + HANDLE rfds[32]; + + pipefd = open_sock(); + + if (nut_debug_level) + upslogx(LOG_INFO, "Timer daemon started"); + + /* Tell the process that spawned us (still waiting inside its own + * call to start_daemon(), see above) that our named pipe is now + * up and accepting connections. This is the WIN32 counterpart of + * us_serialize(SERIALIZE_SET). */ + if (win32_daemon_ready_event != NULL) { + SetEvent(win32_daemon_ready_event); + CloseHandle(win32_daemon_ready_event); + win32_daemon_ready_event = NULL; + } else { + /* Defensive: should not happen in normal operation, since + * start_daemon() always passes a valid inherited handle via + * WIN32_DAEMON_MARKER's argv[2]. Log and carry on: the + * spawning process will simply hit its own + * WaitForSingleObject() timeout and report an error, rather + * than this (already perfectly usable) daemon exiting. */ + upslogx(LOG_WARNING, + "%s: no readiness event handle was received from the spawning process", + __func__); + } + writepid(prog); - /* Whatever upsmon envvars were set when this daemon started, would be - * irrelevant and only confusing at the moment a particular timer causes - * CMDSCRIPT to run */ + /* Whatever upsmon envvars were set for the CLI process that + * spawned us would be irrelevant and only confusing at the moment + * a particular timer causes CMDSCRIPT to run. Under normal + * operation none of these should be inherited any more anyway, + * since start_daemon() above clears them from its own environment + * before calling CreateProcess() -- this is kept as a defensive, + * now-redundant fallback matching the non-WIN32 daemon child. */ unsetenv("NOTIFYTYPE"); unsetenv("UPSNAME"); unsetenv("NOTIFYMSG"); /* now watch for activity */ + upsdebugx(2, "Timer daemon waiting for connections"); for (;;) { /* wait at most 1s so we can check our timers regularly */ @@ -1571,54 +1819,52 @@ static void start_daemon(TYPE_FD lockfd) /* Add the connect event */ rfds[maxfd] = connect_overlapped.hEvent; maxfd++; - DWORD ret_val; - ret_val = WaitForMultipleObjects( - maxfd, /* number of objects in array */ - rfds, /* array of objects */ - FALSE, /* wait for any object */ - timeout_ms); /* timeout in millisecond */ - - if (ret_val == WAIT_FAILED) { - upslog_with_errno(LOG_ERR, "waitfor failed"); - return; - } - /* timer has not expired */ - if (ret_val != WAIT_TIMEOUT) { - /* Retrieve the signaled connection */ - for(tmp = connhead; tmp != NULL; tmp = tmp->next) { - if( tmp->read_overlapped.hEvent == rfds[ret_val-WAIT_OBJECT_0]) { - break; - } - } + { + DWORD ret_val; + ret_val = WaitForMultipleObjects( + maxfd, /* number of objects in array */ + rfds, /* array of objects */ + FALSE, /* wait for any object */ + timeout_ms); /* timeout in millisecond */ - /* the connection event handle has been signaled */ - if (rfds[ret_val] == connect_overlapped.hEvent) { - pipefd = conn_add(pipefd); + if (ret_val == WAIT_FAILED) { + upslog_with_errno(LOG_ERR, "waitfor failed"); + return; } - /* one of the read event handle has been signaled */ - else { - if( tmp != NULL) { - if (sock_read(tmp) < 0) { - upsdebugx(3, "closing connection on handle %p", tmp->fd); - CloseHandle(tmp->fd); - conn_del(tmp); + + /* timer has not expired */ + if (ret_val != WAIT_TIMEOUT) { + /* Retrieve the signaled connection */ + for (tmp = connhead; tmp != NULL; tmp = tmp->next) { + if (tmp->read_overlapped.hEvent == rfds[ret_val - WAIT_OBJECT_0]) { + break; } } - } + /* the connection event handle has been signaled */ + if (rfds[ret_val] == connect_overlapped.hEvent) { + pipefd = conn_add(pipefd); + } + /* one of the read event handle has been signaled */ + else { + if (tmp != NULL) { + if (sock_read(tmp) < 0) { + upsdebugx(3, "closing connection on handle %p", tmp->fd); + CloseHandle(tmp->fd); + conn_del(tmp); + } + } + } + } } checktimers(); } -#endif /* WIN32 */ - /* Should not get here */ -/* - if (nut_debug_level) - upslogx(LOG_INFO, "Timer daemon ending"); -*/ + /* Should not get here (see the WAIT_FAILED "return" above) */ } +#endif /* WIN32 */ /* --- 'client' functions --- */ @@ -1915,65 +2161,158 @@ static void sendcmd(const char *cmd, const char *arg1, const char *arg2) } } - OVERLAPPED read_overlapped; - DWORD ret; - - memset(&read_overlapped,0,sizeof(read_overlapped)); - memset(buf,0,sizeof(buf)); - read_overlapped.hEvent = CreateEvent(NULL, /*Security*/ - FALSE, /* auto-reset*/ - FALSE, /* inital state = non signaled*/ - NULL /* no name*/); - if (read_overlapped.hEvent == NULL) { - fatal_with_errno(EXIT_FAILURE, "Can't create event"); - } - - /* Accommodate reading of longer replies like the loop above; - * async (overlapped) read so not tracking bytesRead here, per - * https://learn.microsoft.com/en-us/windows/win32/api/fileapi/nf-fileapi-readfile + /* NOTE ON THIS REWRITE: + * + * The code this replaces used the return value of ReadFile() + * itself as a "while (ReadFile(...))" loop condition. For a + * handle opened with FILE_FLAG_OVERLAPPED (see try_connect()), + * ReadFile() returning FALSE with GetLastError()== + * ERROR_IO_PENDING is the NORMAL, expected outcome while the + * read is still in flight -- it is NOT a failure, and it is + * by far the common case here, since the daemon practically + * never has its reply ready at the exact instant ReadFile() + * is called. That meant the loop's body (containing the + * WaitForSingleObject()/parsing logic) was, in practice, + * almost never entered, leaving "buf" at the all-zeroes state + * left by the memset() below -- which is the direct cause of + * the empty "read confirmation got []" messages observed in + * the logs. + * + * Separately, on the rare occasions that inner loop WAS + * entered, the "continue"/"break" statements inside it (meant, + * by the surrounding comments and by symmetry with the + * non-WIN32 branch above, to retry/give up on the OUTER + * "for (i = 0; i < MAX_TRIES; i++)" loop) actually only + * affected that inner while-loop -- and did so right after + * CloseHandle(pipefd) had already been called, meaning the + * loop's own condition would then call ReadFile() again on an + * already-closed HANDLE. This rewrite removes that inner loop + * entirely (folding its one legitimate use -- accumulating a + * possibly multi-chunk list_timers response -- into a small, + * clearly-scoped do/while used only for that purpose), and + * uses a "win32_outer_action" flag to route give-up/retry + * decisions to plain "break"/"continue" statements that sit + * directly in the outer for-loop's body, so they unambiguously + * target the outer loop, exactly like every other + * "break"/"continue" in this function already does. + * + * Also fixed in passing: pipefd (a HANDLE) was never closed + * on the success path (a leak on every successful sendcmd() + * call), and read_overlapped.hEvent was never closed at all + * (a leak on every single call, success or failure). Both are + * now closed exactly once, right before returning control to + * the outer loop. */ - while (ReadFile(pipefd, buf, sizeof(buf)-1, NULL, &(read_overlapped))) { - ret = WaitForSingleObject(read_overlapped.hEvent, 2000); - - if (ret == WAIT_TIMEOUT || ret == WAIT_FAILED) { - if (list_timers) { - upslogx(LOG_ERR, "read confirmation failed, daemon must have ended"); - CloseHandle(pipefd); - break; - } else { - upslogx(LOG_ERR, "read confirmation failed, trying again"); - CloseHandle(pipefd); - continue; - } + { + OVERLAPPED read_overlapped; + DWORD bytesRead = 0; + int win32_outer_action = 0; /* 0=fall through and check buf; 1=continue; 2=break */ + int keep_reading; + + memset(&read_overlapped, 0, sizeof(read_overlapped)); + memset(buf, 0, sizeof(buf)); + + read_overlapped.hEvent = CreateEvent(NULL, /* Security */ + FALSE, /* auto-reset */ + FALSE, /* initial state = non-signaled */ + NULL /* no name */); + if (read_overlapped.hEvent == NULL) { + fatal_with_errno(EXIT_FAILURE, "Can't create event"); } - if (buf[0] == '\0') - break; + do { + BOOL read_ok; - if (list_timers) { - /* ASSUME we see whole starting and/or ending lines - * of the response within one read() operation - */ - char *end = strstr(buf, "END LIST TIMERS\n"), - *ok = strstr(buf, "OK\n"); + keep_reading = 0; - if (end) - *end = '\0'; + read_ok = ReadFile(pipefd, buf, sizeof(buf) - 1, NULL, &read_overlapped); - if (ok) - *ok = '\0'; + if (!read_ok && GetLastError() != ERROR_IO_PENDING) { + /* A real, immediate failure -- as opposed to + * the read simply being queued (see the long + * comment above). */ + upslogx(LOG_ERR, "ReadFile failed (error %lu)", + (unsigned long)GetLastError()); + win32_outer_action = list_timers ? 2 : 1; + break; + } - if (!strncmp(buf, "BEGIN LIST TIMERS\n", 18)) { - printf("%s", buf + 18); - } else { - printf("%s", buf); + { + /* Whether read_ok==TRUE (completed synchronously + * -- rare but possible for a pipe) or the read + * was queued (ERROR_IO_PENDING), wait on the + * event to find out when it actually completes. */ + DWORD wait_ret = WaitForSingleObject(read_overlapped.hEvent, 2000); + + if (wait_ret != WAIT_OBJECT_0) { + if (list_timers) { + upslogx(LOG_ERR, "read confirmation failed, daemon must have ended"); + win32_outer_action = 2; + } else { + upslogx(LOG_ERR, "read confirmation failed, trying again"); + win32_outer_action = 1; + } + break; + } } - if (ok) { - snprintf(buf, sizeof(buf), "OK\n"); + /* The code this replaces never called + * GetOverlappedResult() at all, and instead relied + * on the leftover zeroed tail of the memset() above + * to behave like a NUL terminator -- fragile, and + * unable to distinguish "0 bytes read" (e.g. a + * graceful pipe close by the peer) from "read + * failed". */ + if (!GetOverlappedResult(pipefd, &read_overlapped, &bytesRead, FALSE)) { + upslogx(LOG_ERR, "GetOverlappedResult failed (error %lu)", + (unsigned long)GetLastError()); + win32_outer_action = list_timers ? 2 : 1; break; } - } + + if (bytesRead >= sizeof(buf)) + bytesRead = sizeof(buf) - 1; + buf[bytesRead] = '\0'; + + if (bytesRead == 0) + break; + + if (list_timers) { + /* ASSUME we see whole starting and/or ending lines + * of the response within one read() operation + */ + char *end = strstr(buf, "END LIST TIMERS\n"), + *ok = strstr(buf, "OK\n"); + + if (end) + *end = '\0'; + + if (ok) + *ok = '\0'; + + if (!strncmp(buf, "BEGIN LIST TIMERS\n", 18)) { + printf("%s", buf + 18); + } else { + printf("%s", buf); + } + + if (ok) { + snprintf(buf, sizeof(buf), "OK\n"); + } else { + /* More of the multi-line response may + * still be coming: read again. */ + keep_reading = 1; + } + } + } while (keep_reading); + + CloseHandle(read_overlapped.hEvent); + CloseHandle(pipefd); + + if (win32_outer_action == 2) + break; + if (win32_outer_action == 1) + continue; } #endif /* WIN32 */ @@ -2171,8 +2510,23 @@ static int conf_arg(size_t numargs, char **arg) return 1; } + /* In list_timers mode ("-l") and, on WIN32, in the timer-daemon + * child process (is_win32_timer_daemon -- see WIN32_DAEMON_MARKER + * and win32_run_timer_daemon()), we only care about the global + * directives handled above (PIPEFN, LOCKFN, CMDSCRIPT, DEBUG_MIN). + * "AT" lines describe notification-triggered actions and must NOT + * be acted upon (parse_at() -> sendcmd()) by either of these two + * modes: list_timers is a read-only query, and the WIN32 daemon + * child is not forwarding any notification of its own -- doing so + * would resend a stale/foreign command and, on WIN32, is exactly + * what previously caused a runaway cascade of spawned processes. */ +#ifdef WIN32 + if (list_timers || is_win32_timer_daemon) + return 2; +#else if (list_timers) return 2; +#endif /* WIN32 */ if (numargs < 5) return 0; @@ -2256,6 +2610,25 @@ static void checkconf(void) sendcmd("LIST-TIMERS", NULL, NULL); } +#ifdef WIN32 + if (is_win32_timer_daemon) { + if (!pipefn || !(*pipefn)) { + fatalx(EXIT_FAILURE, "upssched.conf: invalid configuration for WIN32 timer daemon: lacks PIPEFN"); + } + if (!lockfn || !(*lockfn)) { + fatalx(EXIT_FAILURE, "upssched.conf: invalid configuration for WIN32 timer daemon: lacks LOCKFN"); + } + + upsdebugx(1, "%s: config parsed for WIN32 timer daemon child", __func__); + + /* NOTE: unlike list_timers mode above, we do NOT sendcmd() + * here: this process IS the daemon being brought up, not a + * client asking an already-running daemon to list its timers. + * Control returns to main(), which calls + * win32_run_timer_daemon() next. */ + } +#endif /* WIN32 */ + /* FIXME: Per legacy behavior, we silently went on. * Maybe should abort on unusable configs? */ @@ -2374,6 +2747,68 @@ int main(int argc, char **argv) /* Here this is a global variable, used also in start_daemon() */ prog = getprogname_argv0_default(argc > 0 ? argv[0] : NULL, "upssched"); +#ifdef WIN32 + /* WIN32 replacement for fork(): see the rewritten WIN32 branch of + * start_daemon() for the process-spawning side of this handshake. + * CreateProcess() always starts a brand-new process at main() -- + * there is no equivalent of the "if (pid == 0) { ...child + * continues here... }" branch fork() gives us on POSIX. So we + * recognize our own hidden marker in argv[1] here, at the very + * top of main() (before getopt() and before the UPSNAME/ + * NOTIFYTYPE environment check further below), to take that + * "we are the daemon" role explicitly -- instead of falling + * through into the normal CLI path, which would re-parse the + * config and re-send the very notification that caused upsmon to + * invoke this program in the first place. That re-sending, plus + * a lock-file race further down the call chain, is what used to + * cause a runaway cascade of spawned processes; this patch fixes + * it by giving the spawned process an explicit, unambiguous + * signal of its role, mirroring fork()'s pid==0 branch. + * + * argv[2], if present, carries the hex-encoded value of a HANDLE + * to a manual-reset Win32 event object created by the spawning + * process (see start_daemon()) and marked inheritable; we + * SetEvent() it, in win32_run_timer_daemon(), once our named pipe + * is up and listening, so the spawning process (still waiting + * inside its own start_daemon() call) knows it is safe to + * proceed. */ + if (argc > 1 && !strcmp(argv[1], WIN32_DAEMON_MARKER)) { + is_win32_timer_daemon = 1; + + if (argc > 2 && argv[2] && *argv[2]) { + win32_daemon_ready_event = (HANDLE)(uintptr_t)strtoull(argv[2], NULL, 16); + } + + /* Do NOT touch UPSNAME/NOTIFYTYPE/NOTIFYMSG here: this is + * not a CLI invocation forwarding an upsmon notification, so + * those variables -- even if somehow still present in our + * environment -- are simply irrelevant on this codepath. + * (start_daemon() already clears them from its own + * environment before spawning us, so in practice they won't + * be inherited at all any more.) */ + + open_syslog(prog); + syslogbit_set(); + atexit(clean_exit); + setproctag("timer"); + + upsdebugx(1, "%s: running as the WIN32 timer-daemon child process", __func__); + + /* Parses PIPEFN/LOCKFN/CMDSCRIPT/DEBUG_MIN from + * upssched.conf via conf_arg(), gated by + * is_win32_timer_daemon (see conf_arg() and checkconf() + * above) to skip acting on any "AT" line. */ + checkconf(); + + win32_run_timer_daemon(); + + /* win32_run_timer_daemon() only returns after a fatal wait + * error (see WAIT_FAILED handling within it); nothing + * sensible left to do here. */ + exit(EXIT_FAILURE); + } +#endif /* WIN32 */ + while ((opt_ret = getopt(argc, argv, optstring)) != -1) { switch (opt_ret) { case 'D':