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 25cced87c65be06e22f6a8e6b506924f630d041c..e628b7169f096cf0f38c2db2b3768fb4af8eab5e 100644 (file)
--- a/ChangeLog
+++ b/ChangeLog
@@ -1,3 +1,17 @@
+2025-05-16  Bruno Haible  <bruno@clisp.org>
+
+       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  <bruno@clisp.org>
 
        sigprocmask: Make multithread-safe on native Windows.

diff --git a/lib/asyncsafe-spin.c b/lib/asyncsafe-spin.c
index 5dbefa1c608e713119c14fb2dede6c50371760b0..38740e817cd97a93484b1881fdd06604d6fc453a 100644 (file)
--- 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"
+     <https://pubs.opengroup.org/onlinepubs/9799919799/functions/V2_chap02.html#tag_16_04_03>.
+     (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 1bbf2f47027d4d45b96b00413b887799cf50095a..c2a4b6bdfe617dcab41ba8097fa2e64d4b38797c 100644 (file)
--- 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 727ff559f2a3248a318467d6a2a866baa65f377c..95103b8ac0f33e264f4641ff68f2961ce03a9ba6 100644 (file)
--- 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 f477e7719ebdd344a77b7c83dc3e8328e2e8617e..e8d0cf31138a2072a1e8552085adea539dcef35b 100644 (file)
--- 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 d918096163e6ee1221d4ca009e5f75556cfffc91..9c3d0097f1ca0b02a11bcca79b3ae5b88a17861c 100644 (file)
--- 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 dd528f422e9e7599c4459374636c0cabab208359..9662bb255b46cfe08dea28fa558431a1213cfa11 100644 (file)
--- 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 106aca24b977f6c747a79fa0cfa3891b596539f5..b6e05637974e1fd1a8cd67aa1a5dfa1137bdaf62 100644 (file)
--- 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 ();