[PATCH v2] Cygwin: pthread: Fix a race issue introduced by the commit 2c5433e5da82

Takashi Yano takashi.yano@nifty.ne.jp
Thu May 30 05:05:29 GMT 2024


To avoid race issues, pthread::once() uses pthread_mutex. This caused
the handle leak which was fixed by the commit 2c5433e5da82. However,
this fix introduced another race issue, i.e., the mutex may be used
after it is destroyed. This patch fixes the issue. Special thanks to
Bruno Haible for discussing how to fix the issue.

Addresses: https://cygwin.com/pipermail/cygwin/2024-May/255987.html
Reported-by: Bruno Haible <bruno@clisp.org>
Fixes: 2c5433e5da82 ("Cygwin: pthread: Fix handle leak in pthread_once.")
Reviewed-by:
Signed-off-by: Takashi Yano <takashi.yano@nifty.ne.jp>
---
 winsup/cygwin/thread.cc | 23 +++++++++--------------
 1 file changed, 9 insertions(+), 14 deletions(-)

diff --git a/winsup/cygwin/thread.cc b/winsup/cygwin/thread.cc
index 0f8327831..cd57a6ac1 100644
--- a/winsup/cygwin/thread.cc
+++ b/winsup/cygwin/thread.cc
@@ -2045,27 +2045,22 @@ pthread::create (pthread_t *thread, const pthread_attr_t *attr,
 int
 pthread::once (pthread_once_t *once_control, void (*init_routine) (void))
 {
-  // already done ?
-  if (once_control->state)
+  /* Sign bit of once_control->state is used as done flag */
+  if (once_control->state & INT_MIN)
     return 0;
 
+  /* The type of &once_control->state is int *, which is compatible with
+     LONG * (the type of the first argument of InterlockedIncrement()). */
+  InterlockedIncrement (&once_control->state);
   pthread_mutex_lock (&once_control->mutex);
-  /* Here we must set a cancellation handler to unlock the mutex if needed */
-  /* but a cancellation handler is not the right thing. We need this in the thread
-   *cleanup routine. Assumption: a thread can only be in one pthread_once routine
-   *at a time. Stote a mutex_t *in the pthread_structure. if that's non null unlock
-   *on pthread_exit ();
-   */
-  if (!once_control->state)
+  if (!(once_control->state & INT_MIN))
     {
       init_routine ();
-      once_control->state = 1;
-      pthread_mutex_unlock (&once_control->mutex);
-      while (pthread_mutex_destroy (&once_control->mutex) == EBUSY);
-      return 0;
+      once_control->state |= INT_MIN;
     }
-  /* Here we must remove our cancellation handler */
   pthread_mutex_unlock (&once_control->mutex);
+  if (InterlockedDecrement (&once_control->state) == INT_MIN)
+    pthread_mutex_destroy (&once_control->mutex);
   return 0;
 }
 
-- 
2.45.1



More information about the Cygwin-patches mailing list