From 9ea48811fed455d4a869eb100b2216f039f8677a Mon Sep 17 00:00:00 2001 From: Viktor Szakats Date: Wed, 5 Aug 2026 09:43:57 +0200 Subject: [PATCH] servers: drop duplicate (and interacting) ctrl handlers on Windows, add exit message On Windows, the init code calls `SetConsoleCtrlHandler()`, and before this patch also set handlers for all Unixy signals. Of these, `SIGBREAK` (used on Windows-only), `SIGINT`, `SIGABRT` and `SIGTERM` were also setting up a `SetConsoleCtrlHandler()`, in addition to the call made directly. (The rest, `SIGHUP`, `SIGPIPE`, `SIGALRM` are either missing the macros, or ignored by `signal()` on Windows.) As per WINE sources, `SetConsolCtrlHandler(, TRUE)` calls are additive, which means the test server set up two console ctrl handlers. Then the ctrl handler set directly (`ctrl_event_handler()`), was triggering the other signal handler via `raise()`, for the 'initiate exit' logic, which in turn triggered exiting a wait within `select_ws()` and other loops. The Windows window handler also made use of the `SIGTERM` event to initiate exit via `raise()` and the second signal handler. To simplify, de-duplicate the ctrl handlers by dropping `signal()` calls and keeping the direct Win32 call with `ctrl_event_handler()` doing all the signal handling on Windows. Break out the 'initiate exit' logic into a function and call it from both Unix and Windows signal/ctrl/window handlers. Also drop calling `raise()` on exit, because it's a no-op without a `signal()` pair. Also: - drop logging the actual ctrl type number, replace with just logging whether we handled the event, in `ctrl_event_handler()`. To avoid using non-signal-safe functions (e.g. `fprintf()`) from the handler. - also replace `logmsg()` with `WriteFile()` to prevent regressions. Ref: #22045 - replace `logmsg()` with `WriteFile()` in `main_window_proc()`. - fix to forward ctrl handling to the OS in the rare case of failed `exit_event` initialization on startup. To swap a possible hang (within `WaitForMultipleObjectsEx()`) with an ungraceful shutdown. - add support for an 'exit message' string, set by signal/ctrl handlers, and log it on app exit. To avoid the need to deal with logging within the handlers, yet have a static trace message about the event. Complementing the already logged signal number. - drop stderr trace message from `exit_signal_handler()` in favor of an exit message. runtests triggers it frequantly, which added much noise to stderr. As a bonus, this also allows dropping the compiler warning suppression. Reported-by: Stefan Eissing Bug: https://github.com/curl/curl/pull/22487#issuecomment-5204092974 Follow-up to 3aae64e4fbee7c1fa408c54df18d3f631781c283 #22507 Refs: https://learn.microsoft.com/windows/console/setconsolectrlhandler https://learn.microsoft.com/windows/console/registering-a-control-handler-function https://learn.microsoft.com/cpp/c-runtime-library/reference/raise https://learn.microsoft.com/cpp/c-runtime-library/reference/signal https://gitlab.winehq.org/wine/wine/-/blob/wine-11.14/dlls/kernelbase/console.c#L1517-1526 https://github.com/huangqinjin/ucrt/blob/d6e817a4cc90f6f1fe54f8a0aa4af4fff0bb647d/misc/signal.cpp#L286-L348 Follow-up to fe28fcf04cdfe7c6e1ab4499a33f9b8479839f14 7dc8a981fa043b9dbbae3a632229b74dcd868bd7 0e058776c02cf8ddc753a36f9cde98cc87899d51 #5260 Closes #22487 --- tests/server/first.c | 6 ++ tests/server/first.h | 1 + tests/server/util.c | 207 ++++++++++++++++++++----------------------- 3 files changed, 101 insertions(+), 113 deletions(-) diff --git a/tests/server/first.c b/tests/server/first.c index a3fc4308f7..c19018bc0e 100644 --- a/tests/server/first.c +++ b/tests/server/first.c @@ -58,6 +58,9 @@ int main(int argc, const char **argv) result = entry_func(argc - 1, argv + 1); + if(serverlogfile && exit_msg) + logmsg("========> exit message: %s", exit_msg); + if(got_exit_signal) { char port_str[11]; const char *location_str = port_str; @@ -72,12 +75,15 @@ int main(int argc, const char **argv) logmsg("========> %s %s (%s pid: %ld) exits with signal (%d)", socket_type, entry_name, location_str, (long)our_getpid(), exit_signal); + +#ifndef _WIN32 /* * To properly set the return status of the process we * must raise the same signal SIGINT or SIGTERM that we * caught and let the old handler take care of it. */ raise(exit_signal); +#endif } if(serverlogfile) diff --git a/tests/server/first.h b/tests/server/first.h index 73657a86ba..aa81a5798e 100644 --- a/tests/server/first.h +++ b/tests/server/first.h @@ -148,6 +148,7 @@ static volatile int exit_signal = 0; #ifdef _WIN32 static HANDLE exit_event = NULL; #endif +static volatile const char *exit_msg = NULL; extern void install_signal_handlers(bool keep_sigalrm); extern void restore_signal_handlers(bool keep_sigalrm); #ifdef USE_UNIX_SOCKETS diff --git a/tests/server/util.c b/tests/server/util.c index 6782c0f1e9..ee8d401d5c 100644 --- a/tests/server/util.c +++ b/tests/server/util.c @@ -322,17 +322,26 @@ storerequest_cleanup: errno, curlx_strerror(errno, errbuf, sizeof(errbuf))); } +static bool initiate_exit(int signum) /* stay signal-safe */ +{ + if(got_exit_signal == 0) { + got_exit_signal = 1; + exit_signal = signum; + } +#ifdef _WIN32 + if(!exit_event) + return FALSE; + (void)SetEvent(exit_event); +#endif + return TRUE; +} + +#ifndef _WIN32 + /* vars used to keep around previous signal handlers */ typedef void (*SIGHANDLER_T)(int); -#if defined(_MSC_VER) && (_MSC_VER <= 1700) -/* Workaround for warning C4306: - 'type cast' : conversion from 'int' to 'void (__cdecl *)(int)' */ -#undef SIG_ERR -#define SIG_ERR ((SIGHANDLER_T)(size_t)-1) -#endif - #ifdef SIGHUP static SIGHANDLER_T old_sighup_handler = SIG_ERR; #endif @@ -348,15 +357,6 @@ static SIGHANDLER_T old_sigint_handler = SIG_ERR; #ifdef SIGTERM static SIGHANDLER_T old_sigterm_handler = SIG_ERR; #endif -#ifdef _WIN32 -static SIGHANDLER_T old_sigbreak_handler = SIG_ERR; -#endif - -#if defined(_WIN32) && !defined(CURL_WINDOWS_UWP) -static DWORD thread_main_id = 0; -static HANDLE thread_main_window = NULL; -static HWND hidden_main_window = NULL; -#endif /* signal handler that is triggered to indicate that the program * should finish its execution in a controlled manner as soon as possible. @@ -367,55 +367,64 @@ static HWND hidden_main_window = NULL; * the POSIX specification: * https://pubs.opengroup.org/onlinepubs/009695399/functions/xsh_chap02_04.html#tag_02_04_03 */ -static void exit_signal_handler(int signum) +static void exit_signal_handler(int signum) /* stay signal-safe */ { int old_errno = errno; - static const char msg[] = "exit_signal_handler(): triggered\n"; - /* suppress warning seen in configurations where 'write()' has the attribute - 'warn_unused_result', which is not silenced by casting to '(void)'. */ -#if defined(CURL_HAVE_DIAG) && !defined(__clang__) -#pragma GCC diagnostic push -#pragma GCC diagnostic ignored "-Wunused-result" /* GCC 4.5+ */ -#endif - (void)write(STDERR_FILENO, msg, CURL_CSTRLEN(msg)); -#if defined(CURL_HAVE_DIAG) && !defined(__clang__) -#pragma GCC diagnostic pop -#endif - if(got_exit_signal == 0) { - got_exit_signal = 1; - exit_signal = signum; -#ifdef _WIN32 - if(exit_event) - (void)SetEvent(exit_event); -#endif - } + exit_msg = "exit_signal_handler(): triggered"; + (void)initiate_exit(signum); #if !(defined(HAVE_SIGACTION) && defined(SA_RESTART)) (void)signal(signum, exit_signal_handler); #endif errno = old_errno; } -#ifdef _WIN32 -/* CTRL event handler for Windows Console applications to simulate - * SIGINT, SIGTERM and SIGBREAK on CTRL events and trigger signal handler. +static SIGHANDLER_T set_signal(int signum, SIGHANDLER_T handler, int norestart) +{ +#if defined(HAVE_SIGACTION) && defined(SA_RESTART) + struct sigaction sa, oldsa; + + memset(&sa, 0, sizeof(sa)); + sa.sa_handler = handler; + sigemptyset(&sa.sa_mask); + sigaddset(&sa.sa_mask, signum); + sa.sa_flags = norestart ? 0 : SA_RESTART; + + if(sigaction(signum, &sa, &oldsa)) + return SIG_ERR; + + return oldsa.sa_handler; +#else + SIGHANDLER_T oldhdlr = signal(signum, handler); + +#ifdef HAVE_SIGINTERRUPT + if(oldhdlr != SIG_ERR) + siginterrupt(signum, norestart); +#else + (void)norestart; +#endif + + return oldhdlr; +#endif +} + +#else /* _WIN32 */ + +/* CTRL event handler for Windows Console applications to handle exit events. * * Background information from MSDN: - * SIGINT is not supported for any Win32 application. When a CTRL+C - * interrupt occurs, Win32 operating systems generate a new thread - * to specifically handle that interrupt. This can cause a single-thread + * When a CTRL+C interrupt occurs, Win32 operating systems generate a new + * thread to specifically handle that interrupt. This can cause a single-thread * application, such as one in UNIX, to become multi-threaded and cause * unexpected behavior. - * [...] - * The SIGKILL and SIGTERM signals are not generated under Windows. - * They are included for ANSI compatibility. Therefore, you can set - * signal handlers for these signals by using signal, and you can also - * explicitly generate these signals by calling raise. Source: - * https://learn.microsoft.com/cpp/c-runtime-library/reference/signal */ -static BOOL WINAPI ctrl_event_handler(DWORD dwCtrlType) +static BOOL WINAPI ctrl_event_handler(DWORD dwCtrlType) /* stay signal-safe */ { + static const char msgU[] = "ctrl_event_handler(): unhandled\n"; + static const char msgH[] = "ctrl_event_handler(): handled\n"; + static const char msgF[] = "ctrl_event_handler(): failed to handle\n"; + HANDLE out = GetStdHandle(STD_ERROR_HANDLE); + DWORD dwWritten; int signum = 0; - logmsg("ctrl_event_handler: %lu", dwCtrlType); switch(dwCtrlType) { case CTRL_C_EVENT: signum = SIGINT; @@ -427,17 +436,25 @@ static BOOL WINAPI ctrl_event_handler(DWORD dwCtrlType) signum = SIGBREAK; break; default: + WriteFile(out, msgU, CURL_CSTRLEN(msgU), &dwWritten, NULL); + exit_msg = msgU; return FALSE; } - if(signum) { - logmsg("ctrl_event_handler: %lu -> %d", dwCtrlType, signum); - raise(signum); + if(!initiate_exit(signum)) { + WriteFile(out, msgF, CURL_CSTRLEN(msgF), &dwWritten, NULL); + exit_msg = msgF; + return FALSE; } + WriteFile(out, msgH, CURL_CSTRLEN(msgH), &dwWritten, NULL); + exit_msg = msgH; return TRUE; } -#endif -#if defined(_WIN32) && !defined(CURL_WINDOWS_UWP) +#ifndef CURL_WINDOWS_UWP +static DWORD thread_main_id = 0; +static HANDLE thread_main_window = NULL; +static HWND hidden_main_window = NULL; + /* Window message handler for Windows applications to add support * for graceful process termination via taskkill (without /f) which * sends WM_CLOSE to all Windows of a process (even hidden ones). @@ -448,20 +465,21 @@ static BOOL WINAPI ctrl_event_handler(DWORD dwCtrlType) static LRESULT CALLBACK main_window_proc(HWND hwnd, UINT uMsg, WPARAM wParam, LPARAM lParam) { - int signum = 0; if(hwnd == hidden_main_window) { switch(uMsg) { - case WM_CLOSE: - signum = SIGTERM; + case WM_CLOSE: { + static const char msg[] = "main_window_proc(): WM_CLOSE -> SIGTERM\n"; + DWORD dwWritten; + WriteFile(GetStdHandle(STD_ERROR_HANDLE), msg, CURL_CSTRLEN(msg), + &dwWritten, NULL); + exit_msg = msg; + initiate_exit(SIGTERM); break; + } case WM_DESTROY: PostQuitMessage(0); break; } - if(signum) { - logmsg("main_window_proc: %u -> %d", uMsg, signum); - raise(signum); - } } return DefWindowProc(hwnd, uMsg, wParam, lParam); } @@ -521,47 +539,15 @@ static DWORD WINAPI main_window_loop(void *lpParameter) hidden_main_window = NULL; return (DWORD)msg.wParam; } -#endif - -static SIGHANDLER_T set_signal(int signum, SIGHANDLER_T handler, int norestart) -{ -#if defined(HAVE_SIGACTION) && defined(SA_RESTART) - struct sigaction sa, oldsa; - - memset(&sa, 0, sizeof(sa)); - sa.sa_handler = handler; - sigemptyset(&sa.sa_mask); - sigaddset(&sa.sa_mask, signum); - sa.sa_flags = norestart ? 0 : SA_RESTART; - - if(sigaction(signum, &sa, &oldsa)) - return SIG_ERR; - - return oldsa.sa_handler; -#else - SIGHANDLER_T oldhdlr = signal(signum, handler); - -#ifdef HAVE_SIGINTERRUPT - if(oldhdlr != SIG_ERR) - siginterrupt(signum, norestart); -#else - (void)norestart; -#endif - - return oldhdlr; -#endif -} +#endif /* CURL_WINDOWS_UWP */ +#endif /* !_WIN32 */ void install_signal_handlers(bool keep_sigalrm) { char errbuf[STRERROR_LEN]; (void)errbuf; -#ifdef _WIN32 - /* setup Windows exit event before any signal can trigger */ - exit_event = CreateEvent(NULL, TRUE, FALSE, NULL); - if(!exit_event) - logmsg("cannot create exit event"); -#endif + (void)keep_sigalrm; +#ifndef _WIN32 #ifdef SIGHUP /* ignore SIGHUP signal */ old_sighup_handler = set_signal(SIGHUP, SIG_IGN, 0); @@ -584,8 +570,6 @@ void install_signal_handlers(bool keep_sigalrm) logmsg("cannot install SIGALRM handler: (%d) %s", errno, curlx_strerror(errno, errbuf, sizeof(errbuf))); } -#else - (void)keep_sigalrm; #endif #ifdef SIGINT /* handle SIGINT signal with our exit_signal_handler */ @@ -601,12 +585,11 @@ void install_signal_handlers(bool keep_sigalrm) logmsg("cannot install SIGTERM handler: (%d) %s", errno, curlx_strerror(errno, errbuf, sizeof(errbuf))); #endif -#ifdef _WIN32 - /* handle SIGBREAK signal with our exit_signal_handler */ - old_sigbreak_handler = set_signal(SIGBREAK, exit_signal_handler, 1); - if(old_sigbreak_handler == SIG_ERR) - logmsg("cannot install SIGBREAK handler: (%d) %s", - errno, curlx_strerror(errno, errbuf, sizeof(errbuf))); +#else /* _WIN32 */ + /* setup Windows exit event before any signal can trigger */ + exit_event = CreateEvent(NULL, TRUE, FALSE, NULL); + if(!exit_event) + logmsg("cannot create exit event"); if(!SetConsoleCtrlHandler(ctrl_event_handler, TRUE)) logmsg("cannot install CTRL event handler"); @@ -616,11 +599,13 @@ void install_signal_handlers(bool keep_sigalrm) if(!thread_main_window || !thread_main_id) logmsg("cannot start main window loop"); #endif -#endif +#endif /* !_WIN32 */ } void restore_signal_handlers(bool keep_sigalrm) { + (void)keep_sigalrm; +#ifndef _WIN32 #ifdef SIGHUP if(old_sighup_handler != SIG_ERR) (void)set_signal(SIGHUP, old_sighup_handler, 0); @@ -634,8 +619,6 @@ void restore_signal_handlers(bool keep_sigalrm) if(old_sigalrm_handler != SIG_ERR) (void)set_signal(SIGALRM, old_sigalrm_handler, 0); } -#else - (void)keep_sigalrm; #endif #ifdef SIGINT if(old_sigint_handler != SIG_ERR) @@ -645,9 +628,7 @@ void restore_signal_handlers(bool keep_sigalrm) if(old_sigterm_handler != SIG_ERR) (void)set_signal(SIGTERM, old_sigterm_handler, 0); #endif -#ifdef _WIN32 - if(old_sigbreak_handler != SIG_ERR) - (void)set_signal(SIGBREAK, old_sigbreak_handler, 0); +#else /* _WIN32 */ (void)SetConsoleCtrlHandler(ctrl_event_handler, FALSE); #ifndef CURL_WINDOWS_UWP if(thread_main_window && thread_main_id) { @@ -663,7 +644,7 @@ void restore_signal_handlers(bool keep_sigalrm) #endif if(exit_event && CloseHandle(exit_event)) exit_event = NULL; -#endif +#endif /* !_WIN32 */ } #ifdef USE_UNIX_SOCKETS