[cpp-threads] Re: Comments on N2094 MT API proposal

[Anthony Williams]

Howard Hinnant <hinnant at twcny.rr.com> writes:

> On Sep 15, 2006, at 4:21 AM, Anthony Williams wrote:
>
>> To: C++ extensions mailing list
>> Message c++std-ext-7971
>>
>> Hi all,
>>
>> I've been reading the threading-related papers in the pre-Portland  mailing
>> with interest.
>>
>> One part of Howards paper N2094 that concerns me is the concept of
>> "convertible shared ownership" of a mutex. Allowing an upgrade from  shared
>> ownership to exclusive ownership without releasing the shared lock is
>> providing a nice easy way for people to write code that deadlocks.  I know that
>> the blocking operations are not provided precisely for this reason,  but even
>> the non-blocking variants can cause a problem. Consider two threads  running
>> the following function:
>>
>> void f(mutex& m)
>> {
>>     m.lock_sharable();
>>     std::thread_util::sleep(100); // or some other time-consuming op
>>     while(!m.try_unlock_sharable_and_lock())
>>     {
>>         std::thread_util::yield();
>>     }
>> }
>>
>> If both threads get the sharable lock, neither will be able to  upgrade to a
>> full lock, so they will both loop forever.
>>
>> I propose that the "convertible shared ownership" concept is  dropped. We
>> already have the "upgradable ownership" concept, which allows one  thread to be
>> upgradable, but share with others that have just "sharable"  ownership, until
>> it actually needs to upgrade to exclusive ownership.
>
> Yes, the code you show is dead-lock-bait.  But I'm not yet convinced that
> try_unlock_sharable_and_lock() is too dangerous to offer.
>
> When analyzing code for deadlocks, as we all know, you have to make sure you
> are holding zero locks when wanting to obtain another lock, unless you can
> prove that either you won't block on obtaining that other lock, or that all
> other threads can make forward progress without needing the first lock you
> hold while blocking on the second.

Sometimes people might think they've done such a proof, but actually they've
overlooked a case. The fewer ways we provide for them to get into trouble, the
better.

> In such analysis, upgrading a mutex from sharable to upgradable, from
> upgradable to exclusive, or from sharable to exclusive must all be viewed as
> obtaining another lock.  I.e. your code snipped will deadlock with any of
> the following variations on the same theme:
>
> void f(mutex& m)
> {
>     m.lock_sharable();
>     while(!m.try_unlock_sharable_and_lock_upgradable())
>         std::thread_util::yield();
> }

Yes, I would propose removing that one for precisely the same reason.

Your other examples are for locking a second mutex. Acquiring locks on
separate mutexes is a different kettle of fish, in my opinion, and there's not
much we can do to limit the dangers.

> So if we get rid of try_unlock_sharable_and_lock(), I'm not sure that we've
> really made the world a safer place.  People just really shouldn't spin like
> that without careful analysis that the loop will end.  

Agreed that one shouldn't write code like that, but I can see it happening,
especially if the loop is more complicated.

> And at the same time,
> we would have gotten rid of some functionality that does have some valid use
> cases which are easy and safe to use:
>
> void f(mutex& m)
> {
>     m.lock_sharable();
>     ...
>     if (m.try_unlock_sharable_and_lock())
>     {
>         ...
>     }
>     else
>     {
>        m.unlock_sharable();
>        m.lock();
>        ...
>     }
>     m.unlock();
> }

For the function as written, I would use upgradable instead of sharable, and
the problem goes away, since there can only be one upgradable.

The issue is where there are multiple threads that *may* want to upgrade
depending on some condition. I'm inclined to say that in that case they just
have to use the unlock_sharable/lock pairing, and take the hit of reloading
the data --- you have to wait for the other threads with sharable access to
relinquish their locks anyway.

> Going the other way (unlock_and_lock_sharable()) also seems very useful, and
> is absolutely safe (it doesn't block):

Agreed here.

> There's a diagram I have of the mutex/lock conversions that perhaps I should
> have put in the paper.  I did have it at the bottom of my pre- standard
> draft:
>
> http://home.twcny.rr.com/hinnant/cpp_extensions/threads_move.html

I saw the draft and made similar comments to these to the UK C++ panel. Sorry
I didn't get my comments to you earlier.

> Getting rid of some of those arrows seems to leave an incomplete model.

I disagree. It makes sense to me that you can freely downgrade *to* sharable
from exclusive or upgradable, but not upgrade *from* sharable, since that is
not safe.

I know that C++ has a history of giving programmers "enough rope to shoot
themselves in the foot", where there is utility in doing so, but I'm not
convinced there is enough value in this usecase to outweigh the
danger.

Anthony
-- 
Anthony Williams
Software Developer
Just Software Solutions Ltd
http://www.justsoftwaresolutions.co.uk