From 91b5ffbdd95d1c573932856f1c2e27ddaca148ae Mon Sep 17 00:00:00 2001 From: Bruno Haible Date: Fri, 10 Apr 2026 17:22:07 +0200 Subject: [PATCH] sigprocmask: Support multithreaded applications on native Windows. * lib/signal.in.h (WIN_PTHREADS_SIGNAL_H): New macro. * lib/sigprocmask.c: Include windows-spin.h. (thread_local): New macro. (blocked_set, pending_array): Mark as thread-local. (blocked_handler): Remove function. (struct override): New type. (overrides, overrides_lock): New variables. (override_handler): New function. (pthread_sigmask): New function, borrowing from the previous sigprocmask definition. (sigprocmask): Now a wrapper around pthread_sigmask. (rpl_signal): Use the overrides_lock to make it multithread-safe. (_gl_raise_SIGPIPE): Add comments. * modules/sigprocmask (Depends-on): Add windows-spin. * lib/pthread_sigmask.c: Revert last change. On native Windows, don't define pthread_sigmask here. * modules/pthread_sigmask (Depends-on): Remove lock. * NEWS: Remove the last entry. --- ChangeLog | 22 +++++ NEWS | 6 -- lib/pthread_sigmask.c | 55 +++++------- lib/signal.in.h | 6 ++ lib/sigprocmask.c | 189 ++++++++++++++++++++++++++++++---------- modules/pthread_sigmask | 1 - modules/sigprocmask | 1 + 7 files changed, 192 insertions(+), 88 deletions(-) diff --git a/ChangeLog b/ChangeLog index d526ad8b0a..9b08fc4a54 100644 --- a/ChangeLog +++ b/ChangeLog @@ -1,3 +1,25 @@ +2026-04-10 Bruno Haible + + sigprocmask: Support multithreaded applications on native Windows. + * lib/signal.in.h (WIN_PTHREADS_SIGNAL_H): New macro. + * lib/sigprocmask.c: Include windows-spin.h. + (thread_local): New macro. + (blocked_set, pending_array): Mark as thread-local. + (blocked_handler): Remove function. + (struct override): New type. + (overrides, overrides_lock): New variables. + (override_handler): New function. + (pthread_sigmask): New function, borrowing from the previous sigprocmask + definition. + (sigprocmask): Now a wrapper around pthread_sigmask. + (rpl_signal): Use the overrides_lock to make it multithread-safe. + (_gl_raise_SIGPIPE): Add comments. + * modules/sigprocmask (Depends-on): Add windows-spin. + * lib/pthread_sigmask.c: Revert last change. On native Windows, don't + define pthread_sigmask here. + * modules/pthread_sigmask (Depends-on): Remove lock. + * NEWS: Remove the last entry. + 2026-04-06 Bruno Haible stat: Update doc regarding mingw bug. diff --git a/NEWS b/NEWS index 55dc317ebf..faaece85b2 100644 --- a/NEWS +++ b/NEWS @@ -78,12 +78,6 @@ User visible incompatible changes Date Modules Changes -2026-04-05 sigprocmask The native MS-Windows implementation no longer - tries to be thread-safe. (As per POSIX, - sigprocmask's behavior has never been - well-defined in multithreaded programs, - which should use pthread_sigmask instead.) - 2026-03-31 lock This module no longer defines the once-only execution primitives. For these, request the 'once' module and #include "glthread/once.h". diff --git a/lib/pthread_sigmask.c b/lib/pthread_sigmask.c index 242664c104..ee2f8c0f5c 100644 --- a/lib/pthread_sigmask.c +++ b/lib/pthread_sigmask.c @@ -19,39 +19,22 @@ /* Specification. */ #include -#include -#include +/* The native Windows implementation is defined in sigprocmask.c. */ +#if !(defined _WIN32 && !defined __CYGWIN__) -#if PTHREAD_SIGMASK_INEFFECTIVE -# include -#endif - -#if !HAVE_SIGPROCMASK && !GNULIB_PTHREAD_SIGMASK_SINGLE_THREAD -# include "glthread/lock.h" - -gl_lock_define_initialized (static, sig_lock) +# include +# include -/* A thread-safe variant of the Gnulib sigprocmask substitute. - Unfortunately it still has undesirable effects in multithreaded processes, - as it can stomp on other threads' signal masks. */ -static int -sigprocmask_r (int how, sigset_t const *new_mask, sigset_t *old_mask) -{ - gl_lock_lock (sig_lock); - int ret = sigprocmask (how, new_mask, old_mask); - gl_lock_unlock (sig_lock); - return ret; -} -# undef sigprocmask -# define sigprocmask sigprocmask_r -#endif +# if PTHREAD_SIGMASK_INEFFECTIVE +# include +# endif int pthread_sigmask (int how, const sigset_t *new_mask, sigset_t *old_mask) -#undef pthread_sigmask +# undef pthread_sigmask { -#if HAVE_PTHREAD_SIGMASK && !PTHREAD_SIGMASK_NOT_IN_LIBC -# if PTHREAD_SIGMASK_INEFFECTIVE +# if HAVE_PTHREAD_SIGMASK && !PTHREAD_SIGMASK_NOT_IN_LIBC +# if PTHREAD_SIGMASK_INEFFECTIVE sigset_t omask; sigset_t *old_mask_ptr = &omask; sigemptyset (&omask); @@ -60,13 +43,13 @@ pthread_sigmask (int how, const sigset_t *new_mask, sigset_t *old_mask) sigaddset (&omask, SIGILL); sigset_t omask_copy; memcpy (&omask_copy, &omask, sizeof omask); -# else +# else sigset_t *old_mask_ptr = old_mask; -# endif +# endif int ret = pthread_sigmask (how, new_mask, old_mask_ptr); -# if PTHREAD_SIGMASK_INEFFECTIVE +# if PTHREAD_SIGMASK_INEFFECTIVE if (ret == 0) { /* Detect whether pthread_sigmask is currently ineffective. @@ -84,14 +67,16 @@ pthread_sigmask (int how, const sigset_t *new_mask, sigset_t *old_mask) if (old_mask) memcpy (old_mask, &omask, sizeof omask); } -# endif -# if PTHREAD_SIGMASK_FAILS_WITH_ERRNO +# endif +# if PTHREAD_SIGMASK_FAILS_WITH_ERRNO if (ret == -1) return errno; -# endif +# endif return ret; -#else +# else int ret = sigprocmask (how, new_mask, old_mask); return (ret < 0 ? errno : 0); -#endif +# endif } + +#endif diff --git a/lib/signal.in.h b/lib/signal.in.h index ce844b1a9c..e4fa3fbaf2 100644 --- a/lib/signal.in.h +++ b/lib/signal.in.h @@ -20,6 +20,12 @@ #endif @PRAGMA_COLUMNS@ +/* Deactivate the mingw , that provides an unusable definition + of pthread_sigmask(). We need to do this before including . */ +#ifndef WIN_PTHREADS_SIGNAL_H +#define WIN_PTHREADS_SIGNAL_H +#endif + #if defined __need_sig_atomic_t || defined __need_sigset_t || defined _@GUARD_PREFIX@_ALREADY_INCLUDING_SIGNAL_H || (defined _SIGNAL_H && !defined __SIZEOF_PTHREAD_MUTEX_T) /* Special invocation convention: - Inside glibc header files. diff --git a/lib/sigprocmask.c b/lib/sigprocmask.c index 6f24fe4f39..63705a5ce5 100644 --- a/lib/sigprocmask.c +++ b/lib/sigprocmask.c @@ -28,11 +28,14 @@ # include "msvc-inval.h" #endif +#include "windows-spin.h" + /* We assume that a platform without POSIX signal blocking functions also does not have the POSIX sigaction() function, only the signal() function. We also assume signal() has SysV semantics, where any handler is uninstalled prior to being invoked. This is - true for native Windows platforms. */ + true for native Windows platforms. Documentation: + */ /* We use raw signal(), but also provide a wrapper rpl_signal() so that applications can query or change a blocked signal. */ @@ -181,25 +184,21 @@ sigfillset (sigset_t *set) return 0; } +/* Storage class specifier for thread-local storage. */ +#undef thread_local +#if defined _MSC_VER +/* */ +# define thread_local __declspec(thread) +#else +/* */ +# define thread_local __thread +#endif + /* Set of currently blocked signals. */ -static sigset_t blocked_set /* = 0 */; +static thread_local sigset_t blocked_set /* = 0 */; /* Set of currently blocked and pending signals. */ -static volatile sig_atomic_t pending_array[NSIG] /* = { 0 } */; - -/* Signal handler that is installed for blocked signals. */ -static void -blocked_handler (int sig) -{ - /* Reinstall the handler, in case the signal occurs multiple times - while blocked. There is an inherent race where an asynchronous - signal in between when the kernel uninstalled the handler and - when we reinstall it will trigger the default handler; oh - well. */ - signal (sig, blocked_handler); - if (sig >= 0 && sig < NSIG) - pending_array[sig] = 1; -} +static thread_local volatile sig_atomic_t pending_array[NSIG] /* = { 0 } */; int sigpending (sigset_t *set) @@ -213,12 +212,61 @@ sigpending (sigset_t *set) return 0; } -/* The previous signal handlers. - Only the array elements corresponding to blocked signals are relevant. */ -static volatile handler_t old_handlers[NSIG]; +/* A registry which signal handlers are overridden. */ +struct override +{ + /* True if the signal is or was blocked in some thread. */ + volatile int overridden; + /* The original signal handler, if overridden. */ + volatile handler_t original_handler; +}; +static struct override overrides[NSIG] /* = { { 0, NULL }, ... } */; + +/* A spin lock that protects overrides against simultaneous use from + different threads. */ +static glwthread_spinlock_t overrides_lock = GLWTHREAD_SPIN_INIT; + +/* Signal handler that overrides an original one. */ +static void +override_handler (int sig) +{ + /* Reinstall the handler, in case the signal occurs multiple times + while blocked. There is an inherent race where an asynchronous + signal in between when the kernel uninstalled the handler and + when we reinstall it will trigger the default handler; oh well. */ + signal (sig, override_handler); + + if ((blocked_set >> sig) & 1) + { + /* The signal is blocked in the current thread. */ + pending_array[sig] = 1; + } + else + { + if (!overrides[sig].overridden) + { + /* Another thread has already installed the override_handler but + is not yet done. Wait until it has finished the operation. */ + glwthread_spin_lock (&overrides_lock); + glwthread_spin_unlock (&overrides_lock); + /* Now overrides[sig].overridden must be true. */ + if (!overrides[sig].overridden) + abort (); + } + handler_t handler = overrides[sig].original_handler; + if (handler == SIG_DFL) + { + /* Restore the original handler. Then raise the signal again. */ + signal (sig, SIG_DFL); + raise (sig); + } + else if (handler != SIG_IGN) + (*handler) (sig); + } +} int -sigprocmask (int operation, const sigset_t *set, sigset_t *old_set) +pthread_sigmask (int operation, const sigset_t *set, sigset_t *old_set) { if (old_set != NULL) *old_set = blocked_set; @@ -238,8 +286,7 @@ sigprocmask (int operation, const sigset_t *set, sigset_t *old_set) new_blocked_set = blocked_set & ~*set; break; default: - errno = EINVAL; - return -1; + return EINVAL; } sigset_t to_unblock = blocked_set & ~new_blocked_set; @@ -249,9 +296,35 @@ sigprocmask (int operation, const sigset_t *set, sigset_t *old_set) for (int sig = 0; sig < NSIG; sig++) if ((to_block >> sig) & 1) { - pending_array[sig] = 0; - if ((old_handlers[sig] = signal (sig, blocked_handler)) != SIG_ERR) - blocked_set |= 1U << sig; + /* All pending_array[sig] remain zero. */ + blocked_set |= 1U << sig; + if (!overrides[sig].overridden) + { + glwthread_spin_lock (&overrides_lock); + if (!overrides[sig].overridden) + { + /* Now it's OK to install the override_handler: + - If it gets invoked in this thread, there is no race + condition since blocked_set already has the sig bit + set. + - If it gets invoked in another thread, the + overrides_lock protects it from proceeding until we + have stored the old_handler. */ + handler_t old_handler = signal (sig, override_handler); + if (old_handler != SIG_ERR) + { + if (old_handler == override_handler) + /* Different threads calling pthread_sigmask at the + same time (race condition). This shouldn't + happen, thanks to the second test of + overrides[sig].overridden, above. */ + abort (); + overrides[sig].original_handler = old_handler; + overrides[sig].overridden = 1; + } + } + glwthread_spin_unlock (&overrides_lock); + } } if (to_unblock != 0) @@ -261,11 +334,6 @@ sigprocmask (int operation, const sigset_t *set, sigset_t *old_set) for (int sig = 0; sig < NSIG; sig++) if ((to_unblock >> sig) & 1) { - if (signal (sig, old_handlers[sig]) != blocked_handler) - /* The application changed a signal handler while the signal - was blocked, bypassing our rpl_signal replacement. - We don't support this. */ - abort (); received[sig] = pending_array[sig]; blocked_set &= ~(1U << sig); pending_array[sig] = 0; @@ -282,8 +350,24 @@ sigprocmask (int operation, const sigset_t *set, sigset_t *old_set) return 0; } +/* sigprocmask is like pthread_sigmask, except that it has undefined behaviour + in multithreaded applications and a different return value convention. */ +int +sigprocmask (int operation, const sigset_t *set, sigset_t *old_set) +{ + int ret = pthread_sigmask (operation, set, old_set); + if (ret == 0) + return 0; + else + { + errno = ret; + return -1; + } +} + /* Install the handler FUNC for signal SIG, and return the previous - handler. */ + handler. + This override transparently hides the override_handler. */ handler_t rpl_signal (int sig, handler_t handler) { @@ -297,23 +381,31 @@ rpl_signal (int sig, handler_t handler) sig = SIGABRT; #endif - if (blocked_set & (1U << sig)) + handler_t result; + if (overrides[sig].overridden) { - /* POSIX states that sigprocmask and signal are both - async-signal-safe. This is not true of our - implementation - there is a slight data race where an - asynchronous interrupt on signal A can occur after we - install blocked_handler but before we have updated - old_handlers for signal B, such that handler A can see - stale information if it calls signal(B). Oh well - - signal handlers really shouldn't try to manipulate the - installed handlers of unrelated signals. */ - handler_t result = old_handlers[sig]; - old_handlers[sig] = handler; - return result; + /* There is an inherent race condition, when one thread calls + rpl_signal to install a different signal handler, while another + thread invokes the signal handler (via override_handler). + It is unavoidable. Nothing we can do about it. */ + result = overrides[sig].original_handler; + overrides[sig].original_handler = handler; } else - return signal (sig, handler); + { + /* Lock, in case of another thread calling pthread_sigmask, with the + effect of installing the override_handler (race condition). */ + glwthread_spin_lock (&overrides_lock); + if (overrides[sig].overridden) + { + result = overrides[sig].original_handler; + overrides[sig].original_handler = handler; + } + else + result = signal (sig, handler); + glwthread_spin_unlock (&overrides_lock); + } + return result; } else { @@ -327,7 +419,12 @@ rpl_signal (int sig, handler_t handler) int _gl_raise_SIGPIPE (void) { + /* On POSIX platforms, SIGPIPE is generated by the kernel and delivered to + any thread of the current process. In the SIGPIPE emulation here, we do + it slightly differently: we deliver it to the current thread always, + like raise (SIGPIPE) would do. */ if (blocked_set & (1U << SIGPIPE)) + /* The signal is blocked in the current thread. */ pending_array[SIGPIPE] = 1; else { diff --git a/modules/pthread_sigmask b/modules/pthread_sigmask index 27ba7031d5..e014553005 100644 --- a/modules/pthread_sigmask +++ b/modules/pthread_sigmask @@ -9,7 +9,6 @@ Depends-on: signal-h threadlib sigprocmask [test $HAVE_PTHREAD_SIGMASK = 0 || test $REPLACE_PTHREAD_SIGMASK = 1] -lock [test $HAVE_PTHREAD_SIGMASK = 0 || test $REPLACE_PTHREAD_SIGMASK = 1] memeq [test $HAVE_PTHREAD_SIGMASK = 0 || test $REPLACE_PTHREAD_SIGMASK = 1] configure.ac: diff --git a/modules/sigprocmask b/modules/sigprocmask index 7f2e083803..f229eeedd8 100644 --- a/modules/sigprocmask +++ b/modules/sigprocmask @@ -10,6 +10,7 @@ signal-h stdint-h [test $HAVE_POSIX_SIGNALBLOCKING = 0] raise [test $HAVE_POSIX_SIGNALBLOCKING = 0] msvc-inval [test $HAVE_POSIX_SIGNALBLOCKING = 0] +windows-spin [test $HAVE_POSIX_SIGNALBLOCKING = 0] configure.ac: gl_SIGNALBLOCKING -- 2.47.3