From: Viktor Szakats Date: Fri, 28 Mar 2025 00:41:28 +0000 (+0100) Subject: tests/server: make the signal handler signal-safe X-Git-Tag: curl-8_13_0~40 X-Git-Url: http://git.ipfire.org/cgi-bin/gitweb.cgi?a=commitdiff_plain;h=e95f509c66abdd88ae02e3243cdc217f19c4a330;p=thirdparty%2Fcurl.git tests/server: make the signal handler signal-safe Before this patch the signal handler called `logmsg()` which in turn called `printf()` variants (internal implementations), and `FILE *` functions, `localtime()`. Some of these called `malloc`/`free`, which isn't supported in s signal handler. Replace them with `write` calls, losing some logging functionality. Also: - De-dupe and move `STD*_FILENO` macros to `lib/curl_setup.h`. Revert the `src` definition to point to `stderr`, instead of `tool_stderr`. Follow-up to e5bb88b8f824ed87620bd923552534c83c2a516e #11958 POSIX specs with list of functions allowed in a signal handler: 2004: https://pubs.opengroup.org/onlinepubs/009695399/functions/xsh_chap02_04.html#tag_02_04_03 2017: https://pubs.opengroup.org/onlinepubs/9699919799/functions/V2_chap02.html#tag_15_04_03 2024: https://pubs.opengroup.org/onlinepubs/9799919799/functions/V2_chap02.html#tag_16_04_03 Linux CI run with the thread sanitizer going crazy when hitting the signal handler in test 1238 and 1242 (TFTP): ``` WARNING: ThreadSanitizer: signal-unsafe call inside of a signal (pid=12582) #0 malloc (servers+0x5ed70) #1 _IO_file_doallocate (libc.so.6+0x851b4) #2 formatf /home/runner/work/curl/curl/bld/tests/server/../../lib/../../lib/mprintf.c:886:9 (servers+0xdff77) [...] WARNING: ThreadSanitizer: signal-unsafe call inside of a signal (pid=12582) #0 free (servers+0x5f453) #1 fclose (libc.so.6+0x8532f) #2 logmsg /home/runner/work/curl/curl/bld/tests/server/../../../tests/server/util.c:134:5 (servers+0xe684d) ``` Ref: https://github.com/curl/curl/actions/runs/14118903372/job/39555309490?pr=16851 Closes #16852 --- diff --git a/lib/curl_setup.h b/lib/curl_setup.h index c8605fd200..ff63d3d72f 100644 --- a/lib/curl_setup.h +++ b/lib/curl_setup.h @@ -862,6 +862,8 @@ #define EINVAL 22 #define ENOSPC 28 #define strerror(x) "?" +#undef STDIN_FILENO +#define STDIN_FILENO 0 #else #define CURL_SETERRNO(x) (errno = (x)) #endif @@ -910,6 +912,17 @@ #define S_ISDIR(m) (((m) & S_IFMT) == S_IFDIR) #endif +/* For MSVC (all versions as of VS2022) */ +#ifndef STDIN_FILENO +#define STDIN_FILENO fileno(stdin) +#endif +#ifndef STDOUT_FILENO +#define STDOUT_FILENO fileno(stdout) +#endif +#ifndef STDERR_FILENO +#define STDERR_FILENO fileno(stderr) +#endif + /* Since O_BINARY is used in bitmasks, setting it to zero makes it usable in source code but yet it does not ruin anything */ #ifdef O_BINARY diff --git a/src/tool_main.h b/src/tool_main.h index c4da177e11..95d603f4a0 100644 --- a/src/tool_main.h +++ b/src/tool_main.h @@ -33,16 +33,4 @@ #define MAX_PARALLEL 300 /* conservative */ #define PARALLEL_DEFAULT 50 -#ifndef STDIN_FILENO -# define STDIN_FILENO fileno(stdin) -#endif - -#ifndef STDOUT_FILENO -# define STDOUT_FILENO fileno(stdout) -#endif - -#ifndef STDERR_FILENO -# define STDERR_FILENO fileno(tool_stderr) -#endif - #endif /* HEADER_CURL_TOOL_MAIN_H */ diff --git a/src/tool_setup.h b/src/tool_setup.h index 404b447a42..b198d89dee 100644 --- a/src/tool_setup.h +++ b/src/tool_setup.h @@ -98,8 +98,6 @@ extern bool tool_term_has_bold; # define _get_osfhandle(fd) (fd) # undef _getch # define _getch() 0 -# undef STDIN_FILENO -# define STDIN_FILENO 0 #endif #ifndef HAVE_FTRUNCATE diff --git a/tests/libtest/lib556.c b/tests/libtest/lib556.c index 03c364164a..1684497340 100644 --- a/tests/libtest/lib556.c +++ b/tests/libtest/lib556.c @@ -26,17 +26,6 @@ #include "warnless.h" #include "memdebug.h" -/* For Windows, mainly (may be moved in a config file?) */ -#ifndef STDIN_FILENO -#define STDIN_FILENO 0 -#endif -#ifndef STDOUT_FILENO -#define STDOUT_FILENO 1 -#endif -#ifndef STDERR_FILENO -#define STDERR_FILENO 2 -#endif - CURLcode test(char *URL) { CURLcode res; diff --git a/tests/server/util.c b/tests/server/util.c index de25857498..6fcf70579b 100644 --- a/tests/server/util.c +++ b/tests/server/util.c @@ -445,10 +445,39 @@ HANDLE exit_event = NULL; * store in exit_signal the signal that triggered its execution. */ #ifndef UNDER_CE +/* + * Only call signal-safe functions from the signal handler, as required by + * the POSIX specification: + * https://pubs.opengroup.org/onlinepubs/9699919799/functions/V2_chap02.html + * Hence, do not call 'logmsg()', and instead use 'open/write/close' to + * log errors. + */ static void exit_signal_handler(int signum) { int old_errno = errno; - logmsg("exit_signal_handler (%d)", signum); + if(!serverlogfile) { + static const char msg[] = "exit_signal_handler: serverlogfile not set\n"; + (void)!write(STDERR_FILENO, msg, sizeof(msg) - 1); + } + else { +#ifdef _WIN32 +#define OPENMODE S_IREAD | S_IWRITE +#else +#define OPENMODE S_IRUSR | S_IWUSR +#endif + int fd = open(serverlogfile, O_WRONLY|O_CREAT|O_APPEND, OPENMODE); + if(fd != -1) { + static const char msg[] = "exit_signal_handler: called\n"; + (void)!write(fd, msg, sizeof(msg) - 1); + close(fd); + } + else { + static const char msg[] = "exit_signal_handler: failed opening "; + (void)!write(STDERR_FILENO, msg, sizeof(msg) - 1); + (void)!write(STDERR_FILENO, serverlogfile, strlen(serverlogfile)); + (void)!write(STDERR_FILENO, "\n", 1); + } + } if(got_exit_signal == 0) { got_exit_signal = 1; exit_signal = signum; @@ -525,9 +554,9 @@ static LRESULT CALLBACK main_window_proc(HWND hwnd, UINT uMsg, if(hwnd == hidden_main_window) { switch(uMsg) { #ifdef SIGTERM - case WM_CLOSE: - signum = SIGTERM; - break; + case WM_CLOSE: + signum = SIGTERM; + break; #endif case WM_DESTROY: PostQuitMessage(0);