RE: [boost] Boost.Threads Locking Delima

From: Darryl Green (green_at_[hidden])
Date: 2002-05-17 04:16:54

> I still lean towards thinking that it's wrong to add public
lock()/unlock()/etc. methods to
> the mutex classes because direct use of such operations is the most common
source of error
> in the code I've had to deal with.

Sounds familiar - but it is at its worst when there isn't anything obviously
better provided. 

I'd like to summarise/interpret the rest of your background info as
background to my comments:

A very common pattern is the existing ScopedLock. This is pretty simple to
understand and use and pretty hard to misuse.

Overlapping lock operations are quite common and the Boost.Threads
ScopedLock deals with this just fine, as you point out. There doesn't seem
to be much scope for misuse here. The dangers associated with overlapping
locks are elsewhere.

The ability to lock and unlock in different scopes is required to implement
a lockingptr<>. The direct approach being the "hack" which makes accessing
the mutex lock/unlock possible but _obscure_. Your concern with this seems
to be that the resulting interface, while obscure, is available for misuse.

This brings us to the RFC:

You propose auto_ptr<> semantics as a solution while noting that there is
ample evidence that auto_ptr<> can be (and is) misused. However, while it is
awfully easily to casually/accidentally misuse autoptr, this wouldn't be so
much the case if pointers were not such a common parameter type in C/C++.
The same can hardly be said for locks, so this mitigates the concerns about
accidental invalidation significantly. Of course, if my assertion is right
and locks are not often passed as parameters, does such a facility belong in
the library? The dangers of providing the facility are that it may encourage
passing locks (it would seem better to discourage it) and that it will hide
errors that would have been detected due to the noncopyable semantics of the
current ScopedLock.

Offering 2 lock types, ScopedLock and MoveOwnershipLock, would (unless the
name was really bad :-) ) just lead to "I'll just use MoveOwnershipLock
because it can be used everywhere". This is worse than changing
ScopedPointer.

Transferring ownership of a lock seems like something that should only be
done after some thought and with care. I don't think the care and though
required to do this using an exposed lock() unlock() interface to a mutex is
significantly higher than that required to do it using the
MoveOwnershipLock. The lockingptr<> example is a good example of where it is
appropriate to lock and unlock in different scopes but (as I see it) doesn't
really make much of a case for the MoveOwnershipLock. The wrapping pattern
is a pattern for precisely the sort of before and after processing that
locking requires. It is blindingly obvious where/when the mutex should be
locked/unlocked when implementing this pattern - auto_ptr semantics seem to
obscure this rather than making it clearer.

I think it is important that boost.threads (and even more important
std.threads) provide a way to efficiently implement whatever higher-level
patterns may be required. A very raw interface provides good flexibility at
the low level (if someone really wants move semantics, let them write it) it
just needs some delineation as to which part of the interface is low level
and which part is "safe". I don't think moving lock ownership around is
"safe" and I wouldn't want to see it suggested by the standard lock types
that it was. Unless of course I am missing something and the ability to do
just that is a great enabler for something good and that outweighs the
accidental evil concerns - anyone have any suggestions as to what that good
thing might be?

Unless there are some good things, I think the best that can be done is to
require a conscious decision and a "statement of intent" in the code before
accessing the lock/unlock interface on a mutex.

Darryl Green.