On Mar 24 14:53, Takashi Yano wrote: > Previously, the fast_mutex defined in thread.h could not be aquired > multiple times, i.e., the thread causes deadlock if it attempted to > acquire a lock already acquired by the thread. For example, a deadlock > occurs if another pthread_key_create() is called in the destructor > specified in the previous pthread_key_create(). This is because the > run_all_destructors() calls the desructor via keys.for_each() where > both for_each() and pthread_key_create() (that calls List_insert()) > attempt to acquire the lock. With this patch, the fast_mutex can be > acquired multiple times by the same thread similar to the behaviour > of a Windows mutex. In this implementation, the mutex is released > only when the number of unlock() calls matches the number of lock() > calls.
Doesn't that mean fast_mutex is now the same thing as muto? The muto type was recursive from the beginning. It's kind of weird to maintain two lock types which are equivalent. I wonder if we shouldn't drop the keys list structure entirely, and convert "keys" to a simple sequence number + destructor array, as in GLibc. This allows lockless key operations and drop the entire list and mutex overhead. The code would become dirt-easy, see https://sourceware.org/cgit/glibc/tree/nptl/pthread_key_create.c https://sourceware.org/cgit/glibc/tree/nptl/pthread_key_delete.c What do you think? However, for 3.6.1, the below patch should be ok. Corinna > > Addresses: https://cygwin.com/pipermail/cygwin/2025-March/257705.html > Fixes: 1a821390d11d ("fix race condition in List_insert") > Reported-by: Yuyi Wang <[email protected]> > Reviewed-by: > Signed-off-by: Takashi Yano <[email protected]> > --- > winsup/cygwin/local_includes/thread.h | 16 ++++++++++++++-- > 1 file changed, 14 insertions(+), 2 deletions(-) > > diff --git a/winsup/cygwin/local_includes/thread.h > b/winsup/cygwin/local_includes/thread.h > index b3496281e..025bfa2fc 100644 > --- a/winsup/cygwin/local_includes/thread.h > +++ b/winsup/cygwin/local_includes/thread.h > @@ -31,7 +31,7 @@ class fast_mutex > { > public: > fast_mutex () : > - lock_counter (0), win32_obj_id (0) > + tid (0), counter_self (0), lock_counter (0), win32_obj_id (0) > { > } > > @@ -55,17 +55,29 @@ public: > > void lock () > { > - if (InterlockedIncrement (&lock_counter) != 1) > + if (!locked () && InterlockedIncrement (&lock_counter) != 1) > cygwait (win32_obj_id, cw_infinite, cw_sig | cw_sig_restart); > + tid = GetCurrentThreadId (); > + counter_self++; > } > > void unlock () > { > + if (!locked () || --counter_self > 0) > + return; > + tid = 0; > if (InterlockedDecrement (&lock_counter)) > ::SetEvent (win32_obj_id); > } > > + bool locked () > + { > + return tid == GetCurrentThreadId (); > + } > + > private: > + DWORD tid; > + int counter_self; > LONG lock_counter; > HANDLE win32_obj_id; > }; > -- > 2.45.1
