libs/newlib-cygwin
libs/newlib-cygwin
4
0
Fork 0
mirror of git://sourceware.org/git/newlib-cygwin.git synced 2025-02-13 04:29:09 +08:00
Code Issues Projects Releases Wiki Activity

Cygwin: pthread: Fix a race issue introduced by the commit 2c5433e5da82

Browse Source 
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 <bruno@clisp.org>
Fixes: 2c5433e5da82 ("Cygwin: pthread: Fix handle leak in pthread_once.")
Reviewed-by: Ken Brown <kbrown@cornell.edu>
Signed-off-by: Takashi Yano <takashi.yano@nifty.ne.jp>
This commit is contained in:
Takashi Yano 2024-05-29 19:12:09 +09:00
parent e567e9f7c3
commit d49c6a7065
3 changed files with 25 additions and 17 deletions
Show Stats Download Patch File Download Diff File Expand all files Collapse all files

2
winsup/cygwin/local_includes/thread.h
View File

@ -674,7 +674,7 @@ class pthread_once
{
public:
pthread_mutex_t mutex;
int state;
volatile int state;
};
/* shouldn't be here */

4
winsup/cygwin/release/3.5.4
View File

@ -8,3 +8,7 @@ Fixes:
- Fix regression introduced in 3.5.0 when reading surrogate pairs (i.e.,
unicode chars >= 0x10000) from the DOS command line. Addresses:
https://cygwin.com/pipermail/cygwin/2024-April/255807.html
- Fix regression of pthread::once() introduced in 3.5.0 (i.e., the race
issue regarding destroying mutex).
Addresses: https://cygwin.com/pipermail/cygwin/2024-May/255987.html

36
winsup/cygwin/thread.cc
View File

@ -2045,27 +2045,31 @@ 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.
Similarly, the next significant bit is used as destroyed flag. */
const int32_t DONE = 0x80000000;
const int32_t DESTROYED = 0x40000000;
static_assert (sizeof (once_control->state) >= sizeof (int32_t));
static_assert (sizeof (once_control->state) == sizeof (LONG));
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 * (that is 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,
DONE | DESTROYED, DONE) == DONE)
pthread_mutex_destroy (&once_control->mutex);
return 0;
}