[boost] Re: Lock unification [move]

From: Howard Hinnant (hinnant_at_[hidden])
Date: 2004-07-22 19:54:41

On Jul 22, 2004, at 4:52 PM, Bronek Kozicki wrote:

> Howard Hinnant wrote:
>
>>> * I do not quite like
>>> "write_lock(try_rvalue_ref<upgradable_read_lock<mutex_type> > r);"
>>> (neither
>>> "exclusive_lock(try_rvalue_ref<upgradable_shared_lock<mutex_type> >
>>> r)" ). It seems inconsistent with rest of interface. What about
>>> "write_lock (rvalue_ref<upgradable_read_lock<mutex_type> > r,
>>> blocking_enum block_it);" ?
>> My problem is that this syntax does not generalize to the 
>> try-move-from-upgradable_lock-assignment:
>> upgradable_lock ul(m);
>> scoped_lock sl  (try_move(ul));
>>             sl = try_move(ul);
>> I really want the try-to-upgrade syntax to be consistent between copy 
>> ctor and assignment forms.  The consistency makes the interface 
>> easier to learn.
>
> I'm little nervous seeing "try_move". If move semantics (the one you 
> proposed in N1377) gets into language, "move" can be replaced with 
> simple move-constructor, while "try_move" cannot.

Actually N1377 suggests a move helper function:

template <class T>
inline
T&&
move(T&& x)
{
     return static_cast<T&&>(x);
}

And N1377 got it slightly wrong.  It should really look like:

template <class T>
inline
typename remove_reference<T>::type&&
move(T&& x)
{
     return x;
}

If we introduce "try_move" it could be a similarly simple helper which 
just returns an auto_ptr-ref-like object tagged with the "try" 
information.  Not that I'm 100% dead set on try_move, I'm not.

>  Moreover, there is already constructor of scoped_lock) taking two 
> parameters, second of them being enumeration (or bool - I'm not in 
> favour of either) meaning "this operation may block". Thus, under my 
> proposal we would have:
>
> upgradable_lock ul(m1); // blocking
> scoped_lock s2(m2); // blocking
> scoped_lock sl(move(ul)); // blocking
> scoped_lock s3(m3, try_lock); // non-blocking (A)
>
> upgradable_lock u4(m4); // blocking
> scoped_lock s4(move(u4), try_lock); // non-blocking (B)
>
> I think that you will agree that A and B are similar and consistent?

Yes, but my concern is what does the syntax look like when s4 already 
exists and you try to upgrade to it by assigning u4 to it:

s4 = u4;  // ???  Try to upgrade (non-blocking) u4 to s4

This isn't a hypothetical scenario, I have a use case for it.  I've 
suggested

s4 = try_move(u4);

and am open to other syntax suggestions.

>> I'm not completely against the free functions.  But I do feel that 
>> they are syntax sugar for the constructors that must be there anyway 
>> (discussed more below).  And if you have them for one lock, you need 
>> them for all 3 locks for consistency.
>
> users of all locks could use the same templated functions:
>
> template <template <typename _Mutex> class Lock, typename Mutex>
> Lock<Mutex> lock(Mutex&);
>
> template ....
> Lock<Mutex> try_lock(Mutex&);
>
> template ....
> Lock<Mutex> try_lock(Mutex&, const elapsed_time&);

I believe this still requires Lock to be explicitly supplied in each 
case.  That's doable.  But is it sufficiently desirable?

>>> * I'd like to have common base class over all lock classes. I do not
>>> want polymorphic behaviour (all member functions might be protected,
>>> including destructor, ctor and cctor), but to allow following very
>>> simple and I believe typical usage scenarios:
>> The common base class stuff makes me nervous.  I don't want 
>> polymorphic behavior either.  But if it isn't polymorphic behavior 
>> what would ~lock() do?  Choices are m_.unlock(), m_.unlock_sharable() 
>> and m_.unlock_upgradable().
>
> destructor would be protected (as constructor, cctor and assignment) 
> and do nothing. There's no special need for any virtual functions, 
> including destructor. This base class would be useful to accept and 
> use reference to lock value returned by helper functions:
>
>   // we do not know type of some_synchronization_primitive here
>   lock_base& l = try_lock<scoped_lock>(some_synchronization_primitive);
>   if (l) {
>     // ...
>   }
>
> It would provide very limited set of non-virtual member functions:
>   bool locked() const;
>   operator safe_bool() const;
>
> and maybe some way to test identity of mutex with other lock. We could 
> use it when using generic helper functions to create lock from unknown 
> (at the time of writing code) type of mutex, as demonstrated above.

Could you flesh this out with a little more complete code.  Sorry, I'm 
just not following.  For instance the first line appears to bind an 
rvalue to a non-const reference.  Are we just missing a const on the 
lock_base& or is there something else going on?

>> I don't dislike these helper functions.  But I do dislike the idea 
>> that they should be the only way to construct-and-try or 
>> construct-with-time.
>
> The whole thing with helper functions is that we minimize number of 
> constructors, instead providing functions. Take a look at following 
> code:
>
> {
>   exclusive_lock some_lock(some_mutex);
>   // some_mutex is not locked yet; some_lock merely points to it
>   some_lock.lock();
>   // some_mutex is locked now
> } // after destruction of some_lock, some_mutex is unlocked again
>
>
> Here exclusive_lock would have only one, nonblocking constructor. All 
> blocking operations would be performed "by hand" using member 
> functions,  or in template helper functions (like try_lock above).
>
> I know this is very different design. Just one nonblocking 
> constructor, number of simple functions (mostly forwarding requests to 
> synchronization primitive and remembering returned state of it when 
> necessary) and destructor required to free lock. Move-constructors and 
> move-assignment (as well as regular cctor and assignment in case of 
> shared lock) are great ideas, also upgradable_lock has its place in 
> such design.

I think this will have an uphill battle.  This code:

{
   exclusive_lock some_lock(some_mutex);
   // assume some_mutex is locked here
}

is just too easy to write (and be wrong).

-Howard