I have tried out this patch and it appears to resolve the main LLD deadlock issues. Some deadlocks still occur when running the LLD test suite, but this is kind of expected.
My only slight concern is that there is a wait for `sema_b` in pthread_cond_destroy which occurs without the `waiters_count_lock_` locked. However, this wait is also an `interruptible` one, which might make a difference. Cheers, Andrew On Fri, 26 Apr 2019 at 09:27, Liu Hao <[email protected]> wrote: > All other functions wait for `sema_b` with `waiters_count_lock_` locked. > The order of acquisition of these two things must happen in the same order > in all functions, otherwise deadlocks may happen. > > The obvious fix is to make wait functions wait for the semaphore after > having locked `waiters_count_lock_`. However, before the wait, the critical > seciton must be unlocked, otherwise all the other threads will be blocked > on signal/broadcast functions. The now consequent wait and signal > operations > on the semaphore have no other effect and can be removed. > > From now on, waiting threads will update `waiters_count_` without > attempting > to lock `sema_b`. If a wait function is called under the protection of the > external mutex, this was unnecessary; otherwise, scheduling behavior > might be > unpredictable, which nevertheless still conforms to POSIX. > > Signed-off-by: Liu Hao <[email protected]> > --- > mingw-w64-libraries/winpthreads/src/cond.c | 12 ------------ > 1 file changed, 12 deletions(-) > > diff --git a/mingw-w64-libraries/winpthreads/src/cond.c > b/mingw-w64-libraries/winpthreads/src/cond.c > index 8df395eb..cd12c8ba 100644 > --- a/mingw-w64-libraries/winpthreads/src/cond.c > +++ b/mingw-w64-libraries/winpthreads/src/cond.c > @@ -431,15 +431,9 @@ pthread_cond_wait (pthread_cond_t *c, > pthread_mutex_t *external_mutex) > } else if (_c->valid != (unsigned int)LIFE_COND) > return EINVAL; > - r = do_sema_b_wait (_c->sema_b, 0, > INFINITE,&_c->waiters_b_lock_,&_c->value_b); > - if (r != 0) > - return r; > EnterCriticalSection (&_c->waiters_count_lock_); > _c->waiters_count_++; > LeaveCriticalSection(&_c->waiters_count_lock_); > - r = do_sema_b_release (_c->sema_b, 1,&_c->waiters_b_lock_,&_c->value_b); > - if (r != 0) > - return r; > ch.c = _c; > ch.r = &r; > @@ -485,15 +479,9 @@ pthread_cond_timedwait_impl (pthread_cond_t *c, > pthread_mutex_t *external_mutex, > dwr = dwMilliSecs(_pthread_time_in_ms_from_timespec(t)); > } > - r = do_sema_b_wait (_c->sema_b, 0, > INFINITE,&_c->waiters_b_lock_,&_c->value_b); > - if (r != 0) > - return r; > EnterCriticalSection (&_c->waiters_count_lock_); > _c->waiters_count_++; > LeaveCriticalSection(&_c->waiters_count_lock_); > - r = do_sema_b_release (_c->sema_b, 1,&_c->waiters_b_lock_,&_c->value_b); > - if (r != 0) > - return r; > ch.c = _c; > ch.r = &r; > -- > 2.21.0 > > > _______________________________________________ > Mingw-w64-public mailing list > [email protected] > https://lists.sourceforge.net/lists/listinfo/mingw-w64-public > _______________________________________________ Mingw-w64-public mailing list [email protected] https://lists.sourceforge.net/lists/listinfo/mingw-w64-public
