Re: Bug in pthread_cond_wait...

[Peter Slacik <Peter.Slacik@tatramed.sk>]

Lorin Hochstein wrote:

> As mentioned in my previous post, I believe there is a bug in the
> pthread_cond_wait code. If the thread is cancelled while blocked in a
> pthread_cond_wait, it does not properly restore the condition variable's
> state, which can cause a subsequent call to pthread_cond_broadcast to
> hang (broadcast will wait forever for the terminated thread to respond).
> This is a big problem if you have one producer and multiple consumers,
> and one of the consumers gets cancelled.
>
> I don't know what the correct protocol is for submitting bugfixes.
> However, for the impatient ones that are encountering the same problem
> that I am attaching a patch with this e-mail.

This is true, but not enough. Cleanup handler _restore_cond() have to
SetEvent(cv->waitersDone) in the thread is the last waiter (as done in
cond_timedwait() after pthread_cleanup_pop()). Missing to do so might cause the
broadcasting thread to check that waiters count is > 0 and then wait for
waitersDone event to be set.

According to my previously posted changes, _restore_cond() must protect it's
work with cv->waitersLock. Additionally, I forgot to do this in
pthread_cond_signal(), something like:

Index: condvar.c
===================================================================
RCS file: pthread/condvar.c,v
retrieving revision 1.5
diff -c -r1.5 condvar.c
*** condvar.c   1999/07/01 07:20:51     1.5
--- condvar.c   1999/07/01 08:22:28
***************
*** 783,792 ****
--- 783,798 ----
    /*
     * If there aren't any waiters, then this is a no-op.
     */
+   if (pthread_mutex_lock(&(cv->waitersLock)) == EINVAL)
+       return EINVAL;
+
    if (cv->waiters > 0)
      {
        result = sem_post (&(cv->sema));
      }
+
+   if (pthread_mutex_unlock(&(cv->waitersLock)) == EINVAL)
+       return EINVAL;

    return (result);

--
Peter