Boost logo

Boost :

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


Boost list run by bdawes at acm.org, gregod at cs.rpi.edu, cpdaniel at pacbell.net, john at johnmaddock.co.uk