Skip to content

Commit

Permalink
winpthreads/cond.c: Remove waits for sema_b from wait functions.
Browse files Browse the repository at this point in the history
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 <lh_mouse@126.com>
  • Loading branch information
lhmouse committed May 5, 2019
1 parent 2a439e1 commit b2302cd
Showing 1 changed file with 0 additions and 12 deletions.
12 changes: 0 additions & 12 deletions mingw-w64-libraries/winpthreads/src/cond.c
Original file line number Diff line number Diff line change
Expand Up @@ -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;
Expand Down Expand Up @@ -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;
Expand Down

0 comments on commit b2302cd

Please sign in to comment.