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 this.
Addresses: https://cygwin.com/pipermail/cygwin/2024-May/255987.html
Reported-by: Bruno Haible
Fixes: 2c5433e5da82 ("Cygwin: pthread: Fix handle leak in pthread_once.")
Reviewed-by:
Signed-off-by: Takashi Yano
---
winsup/cygwin/local_includes/thread.h | 2 +-
winsup/cygwin/thread.cc | 34 ++-
2 files changed, 19 insertions(+), 17 deletions(-)
diff --git a/winsup/cygwin/local_includes/thread.h
b/winsup/cygwin/local_includes/thread.h
index 9939c4224..b3496281e 100644
--- a/winsup/cygwin/local_includes/thread.h
+++ b/winsup/cygwin/local_includes/thread.h
@@ -674,7 +674,7 @@ class pthread_once
{
public:
pthread_mutex_t mutex;
- int state;
+ volatile int state;
};
/* shouldn't be here */
diff --git a/winsup/cygwin/thread.cc b/winsup/cygwin/thread.cc
index 0f8327831..e060af084 100644
--- a/winsup/cygwin/thread.cc
+++ b/winsup/cygwin/thread.cc
@@ -2045,27 +2045,29 @@ 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.
+ Similary, the next significant bit is used as destroyed flag. */
+ const int done = INT_MIN;/* 0b1000 */
+ const int destroyed = INT_MIN >> 1; /* 0b1100 */
+ if (once_control->state & done)
return 0;
- 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)
+ /* The type of &once_control->state is int *, which is compatible with
+ LONG * (the type of the pointer argument of InterlockedXxx()). */
+ if ((InterlockedIncrement (&once_control->state) & done) == 0)
{
- init_routine ();
- once_control->state = 1;
+ pthread_mutex_lock (&once_control->mutex);
+ if (!(once_control->state & done))
+ {
+ init_routine ();
+ InterlockedOr (&once_control->state, done);
+ }
pthread_mutex_unlock (&once_control->mutex);
- while (pthread_mutex_destroy (&once_control->mutex) == EBUSY);
- return 0;
}
- /* Here we must remove our cancellation handler */
- pthread_mutex_unlock (&once_control->mutex);
+ InterlockedDecrement (&once_control->state);
+ if (InterlockedCompareExchange (&once_control->state,
+ destroyed, done) == done)
+pthread_mutex_destroy (&once_control->mutex);
return 0;
}
--
2.45.1
