Re: thread lock/unlock example

From: William Kempf (sirwillard_at_[hidden])
Date: 2000-09-14 15:37:56

--- In boost_at_[hidden], Csaba Szepesvari <szepes_at_m...> wrote:
> William Kempf wrote:
> > --- In boost_at_[hidden], Csaba Szepesvari <szepes_at_m...> wrote:
> > > You might have missed the point that I also used another type
> > `alock' to
> > > avoid this.
> >
> > No, I didn't.  I explained it better in an early post.
> 
> Sure you did:)

No, I didn't.
 
> > The lock
> > class (alock or what ever name you choose to use) should not add
> > recursive locking to a non-recursive mutex, and a recursive lock 
is
> > pointless when you have a recursive mutex.
> 
> That's a separate issue, is not it?

No, it's not.

> I was talking about exception safety
> here..
> First, you could use interfaces/compile-time checks to ensure that a
> recursive lock will not work with a non-recursive mutex.

This muddies the waters between our concepts a little, so I very much 
dislike it.  Especially since there's no purpose for a 
recursive 'alock'.  You only use recursive locks in recursive 
functions, which means you create new locks.  There's no reason to 
recursively lock an 'alock' or unsafe_lock<>, and in fact attempts to 
do so should be prohibited.

> I do not agree that having a recursive lock that works with a 
recursive
> mutex is pointless.
> Consider
> 
> struct A
> {
>   boost::recursive_lock lock;
>   A( boost::recursive_mutex& m ) : lock( m, notlocked ) {}; // no 
locking
> here
>   <various fns using lock>
> };
> 
> The point is that A will clean up the locks upon destruction - I do 
not
> see any problems here.

Why would you choose to design a class this way?  Why create a lock 
at construction time, especially since the lock is constructed as not-
locked (an extension not part of our current concept, btw)?  Your 'A' 
would be better defined as:

struct A
{
   boost::recursive_mutex m_mutex;
   A() { }
   foo() { boost::recursive_mutex::lock lk(m_mutex); bar(); }
   bar() { boost::recursive_mutex::lock lk(m_mutex); }
};

foo() makes a call to bar, which requires a seperate lock on the 
mutex (a recursive lock, since we're in the same thread).  Your 
approach requires foo() and bar() to explicitly call lock.lock() and 
lock.unlock(), which is unsafe in the face of exceptions.

No, a recursive "lock" class is simply not appropriate here.

> The alternate design is
> struct A
> {
>   boost::mutex& m;
>   A( boost::mutex& m ) {}; // this issues
>   <fns usings auto-locks>
> };
> and if you are lucky then the following postulate will hold:
> *** the syntactical scopes in the program coincide with the lock 
scopes
> ***
> I think that the example of Levente was showing the point that this 
is not
> 
> always the case in a valid way.

I don't agree.  You simply use unsafe_lock<> in his example (or use 
it on the heap) instead of the normal auto-lock.  Beyond this, the 
code is identical to what I had above.  A recursive "lock" class is 
redundant here (i.e. it duplicates what's already available).
 
> > > Recursive locks: With conditions, of course, you do not want
> > recursive
> > > locks. That's solved by having two types.
> >
> > You need only one type.
> 
> How did you derive that?? You also have unsafe_lock and lock..??
> Why am I not allowed two types?

Sorry, I meant that you only needed one type for condition.  As 
written the condition works fine with any mutex/lock type.  It's 
completely appropriate for recursive_mutex lock types (the 
implementation specifically deals with recursive locks on a mutex).

> > > Having the lock() exposed on the interface of a lock type yields
> > > exception safety provided that the lock knows how many times to
> > unlock
> > > (could be zero or one, better a bool, for a non-recursive lock).
> >
> > It should be a non-recursive lock, which is precisely how
> > unsafe_lock<> was implemented.
> 
> Ok, then we are talking about the same things.

No, because you wanted a recursive "lock". (The quotes are there 
because I don't want anyone to think that we don't support 
recursively locking a recursive_mutex, I only mean that the 
unsafe_lock itself should not allow multiple calls to lock in a 
recursive fashion.)

> > How is the implementation currently in the Win32 code failing you
> > here?  Why do you want to complicate things with a lock_impl<>?
> >
> > Seriously, I can't comprehend why we still have an issue here.
> >
> 
> I have the same feelings:)
> 
> Summary
> 1) I don't see this template kind of thing too complicated
> 2) It's complexity is such that people will tend to use the simpler
> (auto)lock - your goal and as well as mine
> 3) I don't like the name unsafe_lock.

Why not?

> 4) having different types solves the compile-time check problem (no 
issue
> with condition::wait, nor with non-recursive mutexes used with 
recursive
> locks)

Again, there is no such thing as a "recursive_lock".  The recursive 
locking is a trait of the mutex, not the lock.

> 5) having a recursive mutex with a recursive lock is not evil, but 
on the
> contrary, can be useful.

Recursive mutexes are necessary.  In fact, I'd prefer that we not 
have non-recursive mutexes, but that's a debate for another time.  
But you need to not mix-up the concepts of recursive_mutex and 
recursive_lock.  They are different concepts entirely, and our 
concepts support recursive_mutex not recursive_lock, which I think is 
the more appropriate place to put the trait or recursivity.

> 6) having a non-recursive lock with an explicit lock() is in.

I'll agree, so long as my two requirements are retained:  the lock 
has a name specifying that it's less safe and it's a completely 
seperate concept (i.e. it's not a nested type like the other locks 
are).
 
> I felt you were stressing the idea that a lexical scope must 
(ideally)
> correspond to lock scope.

Ideally it will.  Any time it doesn't seem to fit you need to 
consider the design carefully, since you've found a good indication 
that you've got a problem.  However, I agree that there are some 
corner cases in which lexical scope won't work (at least 
efficiently), so we need a way to program these corner cases.  I've 
dealt with this issue in real code several times with similar 
solutions to our unsafe_lock<>.

> I just don't want to let people write tricky codes like unsafe_lock 
and we
> will push them doing that if the interface is too thin if the 
lexical
> scope hypothesis does not work for them (for whatever reason).

unsafe_lock<> isn't that tricky.  You're not the first to imply that 
placement new is a "hack", but I really don't see it that way.  In 
any event, *if* we provide unsafe_lock<> it no longer matters if you 
think the implementation is tricky/hackish, so long as the lock is 
portable, simple to use and efficient.  Placement new fits the bill.
 
> (I believed that having unsafe_lock in was still undecided, sorry 
of my
> ignorance.)

Well, it is.  I don't have any real objections to including it, but 
I'm just the pushy individual that's stirring up talk... I'm not the 
final arbitrator on this decision.

Bill Kempf