Pthreads over Win32 / Condition Variables (fwd)

[Ross Johnson <rpj at ise dot canberra dot edu dot au>]

Hi all,

Graham Dumpleton (email included below) has discovered a bug in
condvar.c, and provided the fix. I think the fix is correct and will
include it, but in looking at the code I've found another bug
nearby.

The internal routine Cond_timedwait, called by pthread_cond_wait and
pthread_cond_timedwait, is a cancelation point. In case the thread
is canceled while it's waiting, the code pushes a cleanup handler
(actually just pthread_mutex_lock) to re-lock the mutex associated
with the CV. At the moment this is all it does.

The canceled waiting thread doesn't decrement the waiting threads
count (cv->waiters) and so the count will never get back to zero,
and the true last waiter will never notify the broadcasting thread.

I suspect the cleanup handler also needs to check if a broadcast is
in effect and if it's the last waiting thread, and finish off the
broadcast if it is.

If this is correct I'll fix it for the next snapshot.

Ross Johnson

---------- Forwarded message ----------
Date: Wed, 13 Oct 1999 14:51:52 +1000 (EST)
From: Graham Dumpleton <Graham.Dumpleton@ra.pad.otc.telstra.com.au>
Reply-To: grahamd@dscpl.com.au
To: rpj@ise.canberra.edu.au
Subject: Pthreads over Win32 / Condition Variables

Note sure if yours was the best email address to send to, but couldn't see
a single specific email address for bugs related to pthreads-win32. I don't
really want to be subscribing to the mailing list as I don't intend using
the package, just tracking down various ways people implement condition
variables.

Anyway, was looking at how you implement condition variables on Win32 and
I see a problem in the code.

In pthread_cond_init you set wasBroadcast to FALSE. In pthread_cond_broadcast
you set wasBroadcast to TRUE. In cond_timedwait you check what wasBroadcast
was set to.

The problem is that nothing resets wasBroadcast back to FALSE after the
broadcast case has finished. The only place it is being set FALSE is in
the init routine which only gets called once at the start.

What is perhaps needed is in cond_timedwait to say:

      lastWaiter = cv->wasBroadcast && (cv->waiters == 0);

      if (lastWaiter)
	cv->wasBroadcast = FALSE;

      internal_result = pthread_mutex_unlock (&(cv->waitersLock));

Ie., if lastWaiter also reset wasBroadcast.

Still analysing whether this will work, but can't see where else it can be
done as would have to be done before waitersLock is released.

-- 
Graham Dumpleton (grahamd@dscpl.com.au)


+----------------------+---+
| Ross Johnson         |   | E-Mail: rpj@ise.canberra.edu.au
| Info Sciences and Eng|___|
| University of Canberra   | FAX:    +61 6 2015227
| PO Box 1                 |
| Belconnen  ACT    2616   | WWW:    http://willow.canberra.edu.au/~rpj/
| AUSTRALIA                |
+--------------------------+