23

C++17 introduced both std::shared_mutex and std::scoped_lock. My problem is now, that it seems, that scoped_lock will lock a shared mutex always in exclusive (writer) mode, when it is passed as an argument, and not in shared (reader) mode. In my app, I need to update an object dst with data from an object src. I want to lock src shared and dst exclusive. Unfortunately, this has the potential for deadlock, if a call to another update method with src and dst switched occurs at the same time. So I would like to use the fancy deadlock avoidance mechanisms of std::scoped_lock.

I could use scoped_lock to lock both src and dst in exclusive mode, but that unnecessarily strict lock has performance backdraws elsewhere. However, it seems, that it is possible to wrap src's shared_mutex into a std::shared_lock and use that with the scoped_lock: When the scoped_lock during its locking action calls try_lock() on the shared_lock, the later will actually call try_shared_lock() on src's shared_mutex, and that's what I need.

So my code looks as simple as this:

struct data {
    mutable std::shared_mutex mutex;
    // actual data follows
};

void update(const data& src, data& dst)
{
    std::shared_lock slock(src.mutex, std::defer_lock);
    std::scoped_lock lockall(slock, dst.mutex);
    // now can safely update dst with src???
}

Is it safe to use a (shared) lock guard like this inside another (deadlock avoidance) lock guard?

Kai Petzke
  • 2,150
  • 21
  • 29
  • 2
    Wrapping a `shared_mutex` appears to be [the stated purpose of shared_lock](http://www.open-std.org/jtc1/sc22/wg21/docs/papers/2007/n2406.html#shared_lock_rationale). "For example the standard defined generic locking algorithm which locks multiple locks without deadlock can just as easily work on a `shared_lock` as a `unique_lock`." – Raymond Chen Feb 14 '19 at 12:47
  • 1
    `std::scoped_lock` was meant to work with any Lockable objects, and different Lockables would do; be it `mutex`, `recursive_mutex`, `unique_lock<>`, `shared_lock<>`, or any user defined ones (as long as they operate properly). – ALX23z Feb 14 '19 at 23:16
  • @ALX23z When a `std::scoped_lock` is applied directly on a `std::shared_lock`, the later will be locked in exclusive (write) mode. But I want one of two shared locks to be locked in shared (read) mode. – Kai Petzke Feb 15 '19 at 09:37
  • @ALX23z scoped_lock accepts any of type MutexTypes so it does a cast to get a mutex from shared_lock – Mellester Feb 15 '19 at 16:57
  • @Mellester - you mean that it doesn't use `lock()`, `unlock()` and `try_lock()` on the lock as it should, and instead it takes the mutex itself and applies them to the mutex instead of the lock? This is an error if it does so. – ALX23z Feb 16 '19 at 16:55
  • @ALX32z std::scoped_lock accepts only mutex's not general lockable containers. So the compiler is allowed to cast the shared_lock into a mutex by calling its conversion operator. std::lock however does accept any lockable container so that one needs to be used. `std::scoped_lock lockall(std::defer_lock, slock, dst.mutex);` `std::lock(slock,dst.mutex)` Would also have been a solution – Mellester Feb 16 '19 at 21:06
  • @Mellester - the MutexTypes implies only Lockable requirement which all locks satisfy, even fake locks, and not anything beyond that (in case there is only one element even less is required). The fact that it tries to cast locks to mutexes is a design error. – ALX23z Feb 18 '19 at 10:46
  • 1
    `scoped_lock` does accept objects that satisfy *Lockable*, so your code is fine. See the comments to the answer for some details. The part of the answer that says that `scoped_lock` wants to work with the underlying mutex is incorrect as far as I can tell. – bogdan Feb 20 '19 at 20:59

2 Answers2

9

As pointed out by various commentators, who have read the implementation code of the C++ standard library: Yes, the use of a std::shared_mutex wrapped inside a std::shared_lock() as one of the arguments to std::scoped_lock() is safe.

Basically, a std::shared_lock forwards all calls to lock() to lock_shared() on the mutex.

std::shared_lock::lock -----------> mutex()->lock_shared(). // same for try_lock etc..

Another possible solution

std::shared_lock lk1(src.mutex, std::defer_lock);
std::unique_lock lk2(dst.mutex, std::defer_lock);
std::lock(lk1, lk2);

std::lock is a function that accepts any number of Lockable objects and locks all of them (or aborts with an exception, in which case they will all be unlocked).

std::scoped_lock according to cppreference is a wrapper for std::lock, with the added functionaliy of calling unlock() on each Lockable object in its destructor. That added functionality is not required here, as std::shared_lock lk1 and std::unique_lock lk2 also work as lock guards, that unlock their mutexes, when they go out of scope.

Edit: various clarifications

Kai Petzke
  • 2,150
  • 21
  • 29
Mellester
  • 922
  • 7
  • 9
  • std::scoped_lock does do a bit more than just lock but I hope that is clear – Mellester Feb 15 '19 at 17:15
  • 1
    "mutex_type* shared_lock::mutex() const noexcept; gets called and its underlying Mutex gets exposed instead" - how did you reach this conclusion? I've briefly looked at libstdc++, libc++ and MSVC std lib and none of them does anything like that. `MutexTypes` is just a name for the template parameter pack, it doesn't mean that those types actually need to be `std::mutex` or something derived from it. You have to look at the requirements on the template parameters - they just need to be *Lockable*. – bogdan Feb 18 '19 at 18:29
  • @bogdan I believe it has something to do with the fact that `scoped_lock` tries to take ownership of the mutex - and thus tries to make the casting... which is fine if mutex's `lock()` function coincides with the lock's. But since c++17 there is `shred_lock<>` and one could've written custom locks and mutexes beforehand... I think the way it works is a design error or incorrect implementation. – ALX23z Feb 19 '19 at 06:29
  • 1
    @ALX23z What I'm asking is: where have you (or the author of the answer) seen this incorrect implementation of `scoped_lock`? In what standard library implementation have you seen `scoped_lock` "try to make the casting"? This is the standard specification for `scoped_lock`: http://eel.is/c++draft/thread.lock.scoped . It doesn't mention casting or looking for an underlying mutex anywhere. As mentioned above, I've looked (admittedly very briefly) at three different implementations and found nothing like that. In short, OP's code is fine as far as I can tell. – bogdan Feb 19 '19 at 09:14
  • @bogdan Thank you for your look at the code and your comments here! I have accepted Mellester's answer and rewarded him with the bounty nonetheless, because I think, that using `std::lock()` is a tiny bit better than `std::scoped_lock()` in this case: `std::lock()` does not bring its own lock guard, but `std::scoped_lock()` does. So in my code, two lock guards want to unlock the shared lock on src.mutex, when they go out of scope, while in Mellester's code, there will be only one guard for each mutex. To clarify things, I have heavily edited his answer, though. – Kai Petzke Feb 21 '19 at 18:53
  • 1
    @KaiPetzke Good point that we should always consider the unlocking side as well, but you're still fine: `~scoped_lock` will execute first and call `unlock()` on the `shared_lock`, making it non-owning. When `~shared_lock`'s turn comes, it sees that the lock is no longer owning the mutex and does nothing. All good, so the `scoped_lock` solution is still superior to the naked call to `lock()` in my opinion. – bogdan Feb 21 '19 at 19:41
  • 1
    @KaiPetzke I approved your edit, but I suggest you go further and clarify that the solution using `lock()` is merely an alternative, and the one in your question is perfectly valid as well. After all, that was your question: is that code safe? Answering that question should be the focus of a good answer, I believe. – bogdan Feb 21 '19 at 23:18
1

Mutex: Add thread safety to shared resources
Lock: Add RAII (and possibly extra functionality) to mutex

Different locks let you lock the mutex in different ways:

  • unique_lock: exclusive access to resource (for write)
  • shared_lock: shared access to resource (for simultaneous reads)
  • scoped_lock: same as unique_lock, but with less features

scoped_lock is a bare bone exclusive lock that is locked when constructed and unlocked when destroyed. unique_lock and shared_lock are exclusive and shared locks respectively that are also locked and unlocked with their default constructor and destructor. However, they also provide extra functionality. For example, you can try to luck them, or you can unlock them before they are destroyed.

So a typical use case would be to use a shared_lock for shared access (when multiple threads read the same resource), and use a unique_lock or a scoped_lock for exclusive access (depending on if you need the extra features of unique_lock or not).

pooya13
  • 2,060
  • 2
  • 23
  • 29