Re: [boost] Re: More on Boost.Threads

From: David Abrahams (dave_at_[hidden])
Date: 2002-10-16 20:35:47

Jonathan Biggar <jon_at_[hidden]> writes:

> David Abrahams wrote:

> > > >    b. What is the rationale for condition::wait() throwing
> > > >    lock_error if !lock.locked()? Would it not be cleaner to
> > > >    simply lock the lock if it arrives in an unlocked
> > > >    condition?
> > >
> > > It is almost certainly an error to call condition::wait()
> > > without having the lock, since the application code is
> > > responsible for testing the application data that the condition
> > > and lock protect.  If you aren't holding the lock, you either
> > > have a race condition or a deadlock lurking.
> > 
> > I also realize that, given the way condition::wait() is
> > implemented. Did you read my 2nd sentence? If I don't have the
> > lock, and I want to wait on the condition, why shouldn't it just
> > acquire the lock for me? Use case: the condition protects some
> > input queue. I want to block until some data is available in the
> > queue. I don't need to touch the queue until there's data ready,
> > so I have no reason to lock the mutex ahead of time other than
> > that's what the condition interface asks me to do.
> 
> That use case will fail as described.  You must test the queue to
> see if it already has data in it before calling wait or you might
> never get woken up, and that test must be done with the lock held to
> avoid a race.

I'm talking about the version of wait which takes a predicate. I
presume the predicate is what you'd use to test the queue anyway?

> I've never seen a workable use case for calling wait() on a
> condition without performing some test on the data that the
> condition is protectiong, and that means the lock has to be acquired
> first.  That's the basic design pattern for using locks & condition
> variables:
> 
>     lock->acquire();
>     while (!predicate())
> 	cond.wait();
>     // modify protected data here
>     lock->release();

Right. But I'm suggesting that everything up to the comment can be
encapsultated in the version of wait that takes the predicate.

> Of course to be exception safe, you use a lock guard rather than
> just acquiring and releasing the lock.

'course.

> > It's always more pleasant to use an interface which "always works"
> > than one which throws at you to let you know it doesn't like the
> > state of your inputs.
> 
> It would be nice, but that's not the way to make this "always work".

Maybe I'm still missing something, but I don't see it yet. The version
of wait that takes a predicate already contains code for testing the
predicate and in a loop. I don't see a reason to make the user test it
and branch around the wait. But then, maybe I'm describing a slightly
higher-level construct that just shouldn't be called a condition wait.

> > > This is only a race condition if the code violates the
> > > pre-condition that the lock must be locked before testing the
> > > predicate and calling wait().
> > 
> > It's not a precondition. The behavior in case the lock is not lock
> > is well-defined by the documentation, and lock.locked() is not in
> > the Requires: clause. It doesn't make sense to specify behavior in
> > the case of precondition violations.
> 
> It is a precondition in an informal sense, in that wait() throws
> rather than doing what it is expected to do.

That informal sense just confuses things. We're talking about formal
documentation. There's a race condition in the way the wait() with a
predicate is described -- the same race condition you're concerned
about above, in fact. It should be fixed.

> > > >   f. The non-predicate form of timed_wait Effects: clause ends
> > > >   with "...and then reacquires the lock". This appears to mean
> > > >   the lock is reacquired regardless of whether timeout arrives
> > > >   before notification. I think that can't be intentional. Can
> > > >   it?
> > >
> > > Yes, that is intentional.  The required condition variable pre- and
> > > post-condition logic for wait() is that the associated lock is locked.
> > > Without that, you get race conditions.
> > 
> > Only if you access the associated data, right? Well, it wasn't
> > completely clear to me that that was supposed to be allowed in the
> > case of timeout. It means that even with a timeout, you can't come
> > back from wait() if some other thread has the mutex locked. I
> > think it would be good to spell that out somewhere in the
> > condition docs.
> 
> Right, but you have to access the associated data to see if you are
> going to call cond.timed_wait() in the first place.

No, I mean /after/ the return from timed_wait. At that point, if you
aren't going to touch the data, you don't need to have the mutex. If
you're going to touch it regardless of whether notify() occurred. You
need it.

-- 
                    David Abrahams
dave_at_[hidden] * http://www.boost-consulting.com
Building C/C++ Extensions for Python: Dec 9-11, Austin, TX
http://www.enthought.com/training/building_extensions.html