From: Bruno Haible Date: Fri, 16 May 2025 14:37:03 +0000 (+0200) Subject: asyncsafe-spin: Fix race condition on native Windows. X-Git-Url: http://git.ipfire.org/cgi-bin/gitweb.cgi?a=commitdiff_plain;h=9ea3eecee3eafc7cdb08d6167b387374d6dc1378;p=thirdparty%2Fgnulib.git asyncsafe-spin: Fix race condition on native Windows. * lib/asyncsafe-spin.h (asyncsafe_spin_lock, asyncsafe_spin_unlock): Add from_signal_handler parameter. * lib/asyncsafe-spin.c (asyncsafe_spin_lock, asyncsafe_spin_unlock): Likewise. * modules/asyncsafe-spin (Depends-on): Add bool. * tests/test-asyncsafe-spin1.c (main): Update. * tests/test-asyncsafe-spin2.c (lock_mutator_thread, lock_checker_thread): Update. * lib/clean-temp-simple.c (clean_temp_asyncsafe_close): Update. * lib/clean-temp.c (asyncsafe_fclose_variant): Update. --- diff --git a/ChangeLog b/ChangeLog index 25cced87c6..e628b7169f 100644 --- a/ChangeLog +++ b/ChangeLog @@ -1,3 +1,17 @@ +2025-05-16 Bruno Haible + + asyncsafe-spin: Fix race condition on native Windows. + * lib/asyncsafe-spin.h (asyncsafe_spin_lock, asyncsafe_spin_unlock): Add + from_signal_handler parameter. + * lib/asyncsafe-spin.c (asyncsafe_spin_lock, asyncsafe_spin_unlock): + Likewise. + * modules/asyncsafe-spin (Depends-on): Add bool. + * tests/test-asyncsafe-spin1.c (main): Update. + * tests/test-asyncsafe-spin2.c (lock_mutator_thread, + lock_checker_thread): Update. + * lib/clean-temp-simple.c (clean_temp_asyncsafe_close): Update. + * lib/clean-temp.c (asyncsafe_fclose_variant): Update. + 2025-05-16 Bruno Haible sigprocmask: Make multithread-safe on native Windows. diff --git a/lib/asyncsafe-spin.c b/lib/asyncsafe-spin.c index 5dbefa1c60..38740e817c 100644 --- a/lib/asyncsafe-spin.c +++ b/lib/asyncsafe-spin.c @@ -31,18 +31,45 @@ asyncsafe_spin_init (asyncsafe_spinlock_t *lock) void asyncsafe_spin_lock (asyncsafe_spinlock_t *lock, + bool from_signal_handler, const sigset_t *mask, sigset_t *saved_mask) { - sigprocmask (SIG_BLOCK, mask, saved_mask); /* equivalent to pthread_sigmask */ + /* On all platforms, when not running in a signal handler, we need to block + the signals until the corresponding asyncsafe_spin_unlock() invocation. + This is needed because if, during that period, a signal occurred and it + happened to run in the current thread and it were to wait on this spin + lock, it would hang. + On platforms other than native Windows, it is useful to do the same + thing also within a signal handler, since signals may remain enabled + while a signal handler runs. It is possible to do this because + sigprocmask() is safe to call from within a signal handler, see + POSIX section "Signal Actions" + . + (In other words, sigprocmask() is atomic, because it is implemented as a + system call.) + Whereas on native Windows, sigprocmask() is not atomic, because it + manipulates global variables. Therefore in this case, we are *not* + allowed to call it from within a signal handler. */ +#if defined _WIN32 && !defined __CYGWIN__ + if (!from_signal_handler) +#endif + sigprocmask (SIG_BLOCK, mask, saved_mask); /* equivalent to pthread_sigmask */ + glthread_spinlock_lock (lock); } void -asyncsafe_spin_unlock (asyncsafe_spinlock_t *lock, const sigset_t *saved_mask) +asyncsafe_spin_unlock (asyncsafe_spinlock_t *lock, + bool from_signal_handler, + const sigset_t *saved_mask) { if (glthread_spinlock_unlock (lock)) abort (); - sigprocmask (SIG_SETMASK, saved_mask, NULL); /* equivalent to pthread_sigmask */ + +#if defined _WIN32 && !defined __CYGWIN__ + if (!from_signal_handler) +#endif + sigprocmask (SIG_SETMASK, saved_mask, NULL); /* equivalent to pthread_sigmask */ } void diff --git a/lib/asyncsafe-spin.h b/lib/asyncsafe-spin.h index 1bbf2f4702..c2a4b6bdfe 100644 --- a/lib/asyncsafe-spin.h +++ b/lib/asyncsafe-spin.h @@ -28,9 +28,9 @@ both in regular multithreaded code and in signal handlers: sigset_t saved_mask; - asyncsafe_spin_lock (&lock, &mask, &saved_mask); + asyncsafe_spin_lock (&lock, from_signal_handler, &mask, &saved_mask); do_something_contentious (); - asyncsafe_spin_unlock (&lock, &saved_mask); + asyncsafe_spin_unlock (&lock, from_signal_handler, &saved_mask); The mask you specify here is the set of signals whose handlers might want to take the same lock. @@ -53,8 +53,10 @@ extern "C" { extern void asyncsafe_spin_init (asyncsafe_spinlock_t *lock); extern void asyncsafe_spin_lock (asyncsafe_spinlock_t *lock, + bool from_signal_handler, const sigset_t *mask, sigset_t *saved_mask); extern void asyncsafe_spin_unlock (asyncsafe_spinlock_t *lock, + bool from_signal_handler, const sigset_t *saved_mask); extern void asyncsafe_spin_destroy (asyncsafe_spinlock_t *lock); diff --git a/lib/clean-temp-simple.c b/lib/clean-temp-simple.c index 727ff559f2..95103b8ac0 100644 --- a/lib/clean-temp-simple.c +++ b/lib/clean-temp-simple.c @@ -130,7 +130,7 @@ clean_temp_asyncsafe_close (struct closeable_fd *element) int ret; int saved_errno; - asyncsafe_spin_lock (&element->lock, fatal_signal_set, &saved_mask); + asyncsafe_spin_lock (&element->lock, true, fatal_signal_set, &saved_mask); if (!element->closed) { ret = close (element->fd); @@ -142,7 +142,7 @@ clean_temp_asyncsafe_close (struct closeable_fd *element) ret = 0; saved_errno = 0; } - asyncsafe_spin_unlock (&element->lock, &saved_mask); + asyncsafe_spin_unlock (&element->lock, true, &saved_mask); element->done = true; errno = saved_errno; diff --git a/lib/clean-temp.c b/lib/clean-temp.c index f477e7719e..e8d0cf3113 100644 --- a/lib/clean-temp.c +++ b/lib/clean-temp.c @@ -109,7 +109,8 @@ asyncsafe_fclose_variant (struct closeable_fd *element, FILE *fp, int ret; int saved_errno; - asyncsafe_spin_lock (&element->lock, get_fatal_signal_set (), &saved_mask); + asyncsafe_spin_lock (&element->lock, false, + get_fatal_signal_set (), &saved_mask); if (!element->closed) { ret = fclose_variant (fp); /* invokes close (element->fd) */ @@ -121,7 +122,8 @@ asyncsafe_fclose_variant (struct closeable_fd *element, FILE *fp, ret = 0; saved_errno = 0; } - asyncsafe_spin_unlock (&element->lock, &saved_mask); + asyncsafe_spin_unlock (&element->lock, false, + &saved_mask); element->done = true; errno = saved_errno; diff --git a/modules/asyncsafe-spin b/modules/asyncsafe-spin index d918096163..9c3d0097f1 100644 --- a/modules/asyncsafe-spin +++ b/modules/asyncsafe-spin @@ -7,6 +7,7 @@ lib/asyncsafe-spin.c Depends-on: signal-h +bool sigprocmask spin diff --git a/tests/test-asyncsafe-spin1.c b/tests/test-asyncsafe-spin1.c index dd528f422e..9662bb255b 100644 --- a/tests/test-asyncsafe-spin1.c +++ b/tests/test-asyncsafe-spin1.c @@ -36,8 +36,8 @@ main (void) /* Check a spin-lock initialized through the constant initializer. */ { sigset_t saved_set; - asyncsafe_spin_lock (&global_spin_lock, &set, &saved_set); - asyncsafe_spin_unlock (&global_spin_lock, &saved_set); + asyncsafe_spin_lock (&global_spin_lock, false, &set, &saved_set); + asyncsafe_spin_unlock (&global_spin_lock, false, &saved_set); } /* Check a spin-lock initialized through asyncsafe_spin_init. */ @@ -50,8 +50,8 @@ main (void) for (i = 0; i < 10; i++) { sigset_t saved_set; - asyncsafe_spin_lock (&local_spin_lock, &set, &saved_set); - asyncsafe_spin_unlock (&local_spin_lock, &saved_set); + asyncsafe_spin_lock (&local_spin_lock, false, &set, &saved_set); + asyncsafe_spin_unlock (&local_spin_lock, false, &saved_set); } asyncsafe_spin_destroy (&local_spin_lock); diff --git a/tests/test-asyncsafe-spin2.c b/tests/test-asyncsafe-spin2.c index 106aca24b9..b6e0563797 100644 --- a/tests/test-asyncsafe-spin2.c +++ b/tests/test-asyncsafe-spin2.c @@ -56,10 +56,10 @@ #include "asyncsafe-spin.h" #if !ENABLE_LOCKING # define asyncsafe_spin_init(lock) (void)(lock) -# define asyncsafe_spin_lock(lock, mask, saved_mask) \ - ((void)(lock), (void)(mask), (void)(saved_mask)) -# define asyncsafe_spin_unlock(lock, saved_mask) \ - ((void)(lock), (void)(saved_mask)) +# define asyncsafe_spin_lock(lock, from, mask, saved_mask) \ + ((void)(lock), (void)(from), (void)(mask), (void)(saved_mask)) +# define asyncsafe_spin_unlock(lock, from, saved_mask) \ + ((void)(lock), (void)(from), (void)(saved_mask)) # define asyncsafe_spin_destroy(lock) (void)(lock) #endif @@ -130,7 +130,7 @@ lock_mutator_thread (void *arg) int i1, i2, value; dbgprintf ("Mutator %p before lock\n", gl_thread_self_pointer ()); - asyncsafe_spin_lock (&my_lock, &signals_to_block, &saved_signals); + asyncsafe_spin_lock (&my_lock, false, &signals_to_block, &saved_signals); dbgprintf ("Mutator %p after lock\n", gl_thread_self_pointer ()); i1 = random_account (); @@ -140,13 +140,13 @@ lock_mutator_thread (void *arg) account[i2] -= value; dbgprintf ("Mutator %p before unlock\n", gl_thread_self_pointer ()); - asyncsafe_spin_unlock (&my_lock, &saved_signals); + asyncsafe_spin_unlock (&my_lock, false, &saved_signals); dbgprintf ("Mutator %p after unlock\n", gl_thread_self_pointer ()); dbgprintf ("Mutator %p before check lock\n", gl_thread_self_pointer ()); - asyncsafe_spin_lock (&my_lock, &signals_to_block, &saved_signals); + asyncsafe_spin_lock (&my_lock, false, &signals_to_block, &saved_signals); check_accounts (); - asyncsafe_spin_unlock (&my_lock, &saved_signals); + asyncsafe_spin_unlock (&my_lock, false, &saved_signals); dbgprintf ("Mutator %p after check unlock\n", gl_thread_self_pointer ()); yield (); @@ -166,9 +166,9 @@ lock_checker_thread (void *arg) sigset_t saved_signals; dbgprintf ("Checker %p before check lock\n", gl_thread_self_pointer ()); - asyncsafe_spin_lock (&my_lock, &signals_to_block, &saved_signals); + asyncsafe_spin_lock (&my_lock, false, &signals_to_block, &saved_signals); check_accounts (); - asyncsafe_spin_unlock (&my_lock, &saved_signals); + asyncsafe_spin_unlock (&my_lock, false, &saved_signals); dbgprintf ("Checker %p after check unlock\n", gl_thread_self_pointer ()); yield ();