Pthreads over Win32 / Condition Variables (fwd)

Pthreads over Win32 / Condition Variables (fwd)

[Ross Johnson]

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                |
+--------------------------+

Re: Pthreads over Win32 / Condition Variables (fwd)

[Peter Slacik]

Ross Johnson wrote:

> [...] 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.

Lorin Hochstein <lmh@xiphos.ca> already informed about this problem (Wed, 30 Jun
1999) and submitted a bugfix. Ross, please check my reply (Thu, 01 Jul 1999)
too.

--
Peter Slacik

Re: Pthreads over Win32 / Condition Variables (fwd)

[Ross Johnson]

On Fri, 15 Oct 1999, Peter Slacik wrote:

> Ross Johnson wrote:
> 
> > [...] looking at the code I've found another bug nearby.
> >
> Lorin Hochstein <lmh@xiphos.ca> already informed about this problem (Wed, 30 Jun
> 1999) and submitted a bugfix. Ross, please check my reply (Thu, 01 Jul 1999)
> too.
> 

This is embarrassingly true, I have the messages. At the time I
intended to go and check Douglas Schmidt's paper on implementing CVs
under Win32 but obviously never finished the job. My sincere
apologies, especially to Lorin and Peter, and I'll be managing bug
reports differently in the future.

The bug is now fixed and tested, so I'll be producing a new snapshot
very soon.

Regards.
Ross

+----------------------+---+
| 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                |
+--------------------------+