std::lock and std::lock_guard for SharedLockable's lock_shared
575 views
Skip to first unread message
Bernd
Jul 12, 2016, 3:09:42 PM7/12/16
to ISO C++ Standard - Future Proposals
In discussions on Andrei's NDC talk about folly/Synchronized [1] the question came up why his deadlock prevention [2] did not just use std::lock(). It was pointed out that this is due to using SharedLockable::lock_shared(), making std::lock() the wrong thing. There is the workaround of
std::shared_mutex a, b;
std::shared_lock<> lock_a (a, std::defer_lock);
std::shared_lock<> lock_b (b, std::defer_lock);
std::shared_lock<> lock_b (b, std::defer_lock);
std::lock (lock_a, lock_b);
using the shared_lock adapter exposing SharedLockable as Lockable. Note that the inverse order of std::lock and std::shared_lock (using std::adopt_lock) requires manual deadlock prevention. Note that the locks also have to be non-const which is undesirable.
I propose adding std::lock_shared (SharedLockable&...) to allow said inverse order, for consistency:
std::shared_mutex a, b;
std::lock_shared (a, b); std::shared_lock<> const lock_a (a, std::adopt_lock);
std::shared_lock<> const lock_b (b, std::adopt_lock);
On the same note, C++17 added std::lock_guard<Lockable...>, which has no SharedLockable variant either, which would make the above code even easier:
std::shared_lock_guard<> const lock_both (a, b);
The biggest issue I can see with it is that combining locking a Lockable and a SharedLockable at once, which still requires the defer_lock approach or manual deadlock prevention. It feels like this was an oversight when adding SharedLockable and an easy addition of basically "do as std::lock, but use _shared methods".
-- Bernd
Howard Hinnant
Jul 12, 2016, 3:34:38 PM7/12/16
to std-pr...@isocpp.org
On Jul 12, 2016, at 3:09 PM, 'Bernd' via ISO C++ Standard - Future Proposals <std-pr...@isocpp.org> wrote:
>
> In discussions on Andrei's NDC talk about folly/Synchronized [1] the question came up why his deadlock prevention [2] did not just use std::lock(). It was pointed out that this is due to using SharedLockable::lock_shared(), making std::lock() the wrong thing. There is the workaround of
>
> std::shared_mutex a, b;
> std::shared_lock<> lock_a (a, std::defer_lock);
> std::shared_lock<> lock_b (b, std::defer_lock);
> std::lock (lock_a, lock_b);
>
> using the shared_lock adapter exposing SharedLockable as Lockable. Note that the inverse order of std::lock and std::shared_lock (using std::adopt_lock) requires manual deadlock prevention. Note that the locks also have to be non-const which is undesirable.
>
> I propose adding std::lock_shared (SharedLockable&...) to allow said inverse order, for consistency:
>
> std::shared_mutex a, b;
> std::lock_shared (a, b);
> std::shared_lock<> const lock_a (a, std::adopt_lock);
> std::shared_lock<> const lock_b (b, std::adopt_lock);
>
> On the same note, C++17 added std::lock_guard<Lockable...>, which has no SharedLockable variant either, which would make the above code even easier:
>
> std::shared_lock_guard<> const lock_both (a, b);
>
> The biggest issue I can see with it is that combining locking a Lockable and a SharedLockable at once, which still requires the defer_lock approach or manual deadlock prevention. It feels like this was an oversight when adding SharedLockable and an easy addition of basically "do as std::lock, but use _shared methods”.
I can assure you it wasn’t an oversight. And your first “workaround” is the intended use.
>
> In discussions on Andrei's NDC talk about folly/Synchronized [1] the question came up why his deadlock prevention [2] did not just use std::lock(). It was pointed out that this is due to using SharedLockable::lock_shared(), making std::lock() the wrong thing. There is the workaround of
>
> std::shared_mutex a, b;
> std::shared_lock<> lock_a (a, std::defer_lock);
> std::shared_lock<> lock_b (b, std::defer_lock);
> std::lock (lock_a, lock_b);
>
> using the shared_lock adapter exposing SharedLockable as Lockable. Note that the inverse order of std::lock and std::shared_lock (using std::adopt_lock) requires manual deadlock prevention. Note that the locks also have to be non-const which is undesirable.
>
> I propose adding std::lock_shared (SharedLockable&...) to allow said inverse order, for consistency:
>
> std::shared_mutex a, b;
> std::lock_shared (a, b);
> std::shared_lock<> const lock_a (a, std::adopt_lock);
> std::shared_lock<> const lock_b (b, std::adopt_lock);
>
> On the same note, C++17 added std::lock_guard<Lockable...>, which has no SharedLockable variant either, which would make the above code even easier:
>
> std::shared_lock_guard<> const lock_both (a, b);
>
> The biggest issue I can see with it is that combining locking a Lockable and a SharedLockable at once, which still requires the defer_lock approach or manual deadlock prevention. It feels like this was an oversight when adding SharedLockable and an easy addition of basically "do as std::lock, but use _shared methods”.
We don’t need or want a “kitchen sink” API where there are 3 three different ways to do everything. We already have that with std::string, and we don’t need to repeat that design mistake.
The “mixed case” you speak of at the end is actually a very prominent use case:
class A
{
using ReadLock std::shared_lock<std::shared_mutex>;
using WriteLock std::unique_lock<std::shared_mutex>;
mutable std::shared_mutex mut_;
public:
// ...
A& operator=(const A& a)
{
if (this != &a)
{
WriteLock this_lock(mut_, std::defer_lock);
ReadLock that_lock(a.mut_, std::defer_lock);
std::lock(this_lock, that_lock);
// mut_ is now unique-locked and a.mut_ is now share-locked
// ... now safe to assign data ...
} // mut_ and a.mut_ now unlocked, even if there was an exception
return *this;
}
// ...
};
Howard
Arthur O'Dwyer
Jul 13, 2016, 1:55:49 AM7/13/16
to ISO C++ Standard - Future Proposals
On Tuesday, July 12, 2016 at 12:34:38 PM UTC-7, Howard Hinnant wrote:
On Jul 12, 2016, at 3:09 PM, 'Bernd' via ISO C++ Standard - Future Proposals <std-pr...@isocpp.org> wrote:
>
> In discussions on Andrei's NDC talk about folly/Synchronized [1] the question came up why his deadlock prevention [2] did not just use std::lock(). It was pointed out that this is due to using SharedLockable::lock_shared(), making std::lock() the wrong thing. [...]
> I propose adding std::lock_shared (SharedLockable&...)
[...]
> On the same note, C++17 added std::lock_guard<Lockable...>, which has no SharedLockable variant either [...]
I can assure you it wasn’t an oversight. And your first “workaround” is the intended use.
We don’t need or want a “kitchen sink” API where there are 3 three different ways to do everything. We already have that with std::string, and we don’t need to repeat that design mistake.
As someone who wasn't involved at all in the design process for lock/shared_lock/lock_guard, let me add another reason NOT to add more lock variants than we already have: massive confusion for newbies.
When I first learned about std::mutex and std::lock_guard, I thought it was the neatest thing ever.
Then I learned about std::unique_lock, which was even neater — it was like lock_guard, but you could actually pass it around from place to place. Mind you, you couldn't copy it — it was a move-only type — this was obviously why they called it unique_lock, just like C++11's other move-only type, unique_ptr.
Then came shared_mutex. Which kind of makes sense as a name... kind of. I mean, anyone who's taken Operating Systems in university knows that what it really is is a reader-writer lock, a.k.a. rwlock, but okay, the Committee hates acronyms or something, so yeah, it does behave kind of like a mutex that you can "share". And so it has a method lock_shared(). Makes sense if you think about it.
And then... wait a minute. Was my intuition about unique_lock wrong? Did it actually get its name not because it's move-only, but because it takes an exclusive lock on the mutex it controls? (Seeing the Committee fumble the noun "rwlock" leads to the horrible thought that they might equally have fumbled the adjective "exclusive".) So then what's a shared_lock? ...which showed up in C++14, as a move-only type! This way lies madness. Best to just sweep it under the bed and try to forget about it.
I welcome anything the Committee can do at this point to limit the confusion and preserve the general principle of "unique_ things are move-only, shared_ things are copyable, and things pretty much do what they say on the tin (except for shared_mutex which is really rwlock in disguise, oops)" — which means I would attempt to oppose the addition of any new shared_ stuff to the library unless it really clearly did what it appeared to say on the tin.
–Arthur
Andrey Semashev
Jul 13, 2016, 4:24:22 AM7/13/16
to std-pr...@isocpp.org
On 07/13/16 08:55, Arthur O'Dwyer wrote:
>
> As someone who wasn't involved at all in the design process for
> lock/shared_lock/lock_guard, let me add another reason NOT to add more
> lock variants than we already have: massive confusion for newbies.
>
> When I first learned about std::mutex and std::lock_guard, I thought it
> was the neatest thing ever.
> Then I learned about std::unique_lock, which was even neater — it was
> like lock_guard, but you could actually pass it around from place to
> place. Mind you, you couldn't /*copy*/ it — it was a move-only type —
>
> As someone who wasn't involved at all in the design process for
> lock/shared_lock/lock_guard, let me add another reason NOT to add more
> lock variants than we already have: massive confusion for newbies.
>
> When I first learned about std::mutex and std::lock_guard, I thought it
> was the neatest thing ever.
> Then I learned about std::unique_lock, which was even neater — it was
> like lock_guard, but you could actually pass it around from place to
> this was obviously why they called it /*unique*/_lock, just like C++11's
> other move-only type, /*unique*/_ptr.
>
> Then came shared_mutex. Which kind of makes sense as a name... kind of.
> I mean, anyone who's taken Operating Systems in university knows that
> what it really is is a reader-writer lock
> <https://en.wikipedia.org/wiki/Readers–writer_lock>, a.k.a. r
> Then came shared_mutex. Which kind of makes sense as a name... kind of.
> I mean, anyone who's taken Operating Systems in university knows that
> what it really is is a reader-writer lock
> <https://doc.rust-lang.org/std/sync/struct.RwLock.html>w
> <https://www.npmjs.com/package/rwlock>lo
> <http://linux.die.net/man/3/pthread_rwlock_init>c
> <http://www.freebsd.org/cgi/man.cgi?query=rwlock&sektion=9>k
> <http://docs.libuv.org/en/v1.x/threading.html#read-write-locks>, but
> okay, the Committee hates acronyms or something, so yeah, it does behave
> kind of like a mutex that you can "share". And so it has a method
> lock_shared(). Makes sense if you think about it.
>
> And then... wait a minute. Was my intuition about unique_lock wrong? Did
> it actually get its name not because it's move-only, but because it
> takes an /*exclusive*/ lock on the mutex it controls?
> kind of like a mutex that you can "share". And so it has a method
> lock_shared(). Makes sense if you think about it.
>
> And then... wait a minute. Was my intuition about unique_lock wrong? Did
> it actually get its name not because it's move-only, but because it
Yes. You can also say the lock offers a unique access to the resource
protected by the mutex.
> (Seeing the
> Committee fumble the noun "rwlock" leads to the horrible thought that
> they might equally have fumbled the adjective "exclusive".) So then
> type! This way lies madness. Best to just sweep it under the bed and try
> to forget about it.
I don't see the problem. The shared lock offers a shared access to the
> to forget about it.
resource, so the name is fitting. The lock could potentially be made
copyable but this would require unnecessary overhead because normally
you don't want to copy locks, only move.
> I welcome anything the Committee can do at this point to limit the
> move-only, /shared_/ things are copyable, and things pretty much do what
> they say on the tin (except for shared_mutex which is really rwlock in
> disguise, oops)" — which means I would attempt to oppose the addition of
> any new /shared_/ stuff to the library unless it really clearly did what
> disguise, oops)" — which means I would attempt to oppose the addition of
> it appeared to say on the tin.
The tail doesn't wag the dog. Objects representing unique resources are
movable-only because when you allow copying there appears two objects
owning the same resource. Objects representing shared resources don't
have that restriction but they still can be movable-only for other
reasons. In either case, the types are not named unique or shared
because they are movable-only or not - copyability and movability are
the consequence and not the cause.
As for the shared_lock_guard, I do think it is needed, even if not the
variadic version. Most of the time I use lock_guard instead of
unique_lock because I don't need neither movability nor elaborate
constructors or lock methods - I just want to have a locked scope and
nothing more. The most frequent use of unique_lock for me is with a
condition variable. I barely ever use std::lock and frankly if you need
to have multiple resources locked then chances are high that you have a
design problem. I believe, with shared locks the situation will be even
more emphasized because you can't use shared locks with condition variables.
Bernd Lörwald
Jul 13, 2016, 6:17:49 AM7/13/16
to std-pr...@isocpp.org
I understand the goal of not cluttering the API with a thousand ways, as well as not trying to confuse users. The unique_lock vs lock_guard naming issue has infact also confused me the first few times I used it, and I keep checking back if I'm actually using the right one from time to time.
I want to highlight Andrey's point on the shared_lock_guard part: Most uses of locks are not unlocking, but using pure RAII style locking, and expressing this intent when locking multiple mutexes is currently not possible:
* Due to defer_lock the shared_lock has to be non-const, only the lock_guard referencing the shared_locks can be const,
* if using std::lock like in Howard's example means nothing is const.
* Having const shared_locks via adopt_lock requires external deadlock protection which the library can probably do better.
* Due to defer_lock the shared_lock has to be non-const, only the lock_guard referencing the shared_locks can be const,
* if using std::lock like in Howard's example means nothing is const.
* Having const shared_locks via adopt_lock requires external deadlock protection which the library can probably do better.
Note that boost already provides a shared_lock_guard (Mutex&) as extension [1], but like the STL does not provide a std::lock equivalent. I see the that mixed-locks are common, so I understand that decision. Best I can come up with for that is on overload
std::lock (LockableOrSharedLockable1&, Policy1, …);
to be used as
std::lock (other.mutex_, std::lock_shared, mutex_, std::lock_exclusive);
which is somewhat ugly but would do the job. (And yes, I know this doesn't make the API thinner.)
--
You received this message because you are subscribed to the Google Groups "ISO C++ Standard - Future Proposals" group.
To unsubscribe from this group and stop receiving emails from it, send an email to std-proposal...@isocpp.org.
To post to this group, send email to std-pr...@isocpp.org.
To view this discussion on the web visit https://groups.google.com/a/isocpp.org/d/msgid/std-proposals/5785FAB2.2000203%40gmail.com.
Howard Hinnant
Jul 13, 2016, 10:43:54 AM7/13/16
to std-pr...@isocpp.org
On Jul 13, 2016, at 4:24 AM, Andrey Semashev <andrey....@gmail.com> wrote:
>
> As for the shared_lock_guard, I do think it is needed, even if not the variadic version.
Just do this:
>
> As for the shared_lock_guard, I do think it is needed, even if not the variadic version.
using shared_lock_guard = const std::shared_lock<std::shared_mutex>;
The standard should concentrate on supplying those things that are not trivial for anyone to do themselves, or is needed in virtually every program.
On Jul 13, 2016, at 6:17 AM, 'Bernd Lörwald' via ISO C++ Standard - Future Proposals <std-pr...@isocpp.org> wrote:
> std::lock (LockableOrSharedLockable1&, Policy1, …);
>
> to be used as
>
> std::lock (other.mutex_, std::lock_shared, mutex_, std::lock_exclusive);
Howard
Andrey Semashev
Jul 13, 2016, 11:14:19 AM7/13/16
to std-pr...@isocpp.org
On 07/13/16 17:43, Howard Hinnant wrote:
> On Jul 13, 2016, at 4:24 AM, Andrey Semashev <andrey....@gmail.com> wrote:
>>
>> As for the shared_lock_guard, I do think it is needed, even if not the variadic version.
>
> Just do this:
>
> using shared_lock_guard = const std::shared_lock<std::shared_mutex>;
That is not the same as shared_lock_guard from Boost. Similar to
> On Jul 13, 2016, at 4:24 AM, Andrey Semashev <andrey....@gmail.com> wrote:
>>
>> As for the shared_lock_guard, I do think it is needed, even if not the variadic version.
>
> Just do this:
>
> using shared_lock_guard = const std::shared_lock<std::shared_mutex>;
lock_guard, shared_lock_guard is not movable and does not support
special constructors or methods. Consequently it does not have the
overhead of shared_lock, like lock_guard doesn't have that of unique_lock.
> The standard should concentrate on supplying those things that are not trivial for anyone to do themselves, or is needed in virtually every program.
program involving shared_mutex.
Reply all
Reply to author
Forward
0 new messages