C++ Logo


Advanced search

Re: [std-proposals] Lack of preconditions on std::lock/std::try_lock

From: Jason McKesson <jmckesson_at_[hidden]>
Date: Wed, 13 Sep 2023 11:48:07 -0400
On Wed, Sep 13, 2023 at 11:20 AM Nikl Kelbon via Std-Proposals
<std-proposals_at_[hidden]> wrote:
> Postcondition of std::lock and std::try_lock:
> All of Mutexes... are locked, no deadlock occurs.
> Effects: All arguments are locked via a sequence of calls to lock(), try_lock(), or unlock() on each argument. The sequence of calls does not result in deadlock, but is otherwise unspecified.
> But in an common case, when there is a type with a mutex and you need to swap it
> void swap(Type& other) {
> std::scoped_lock l(this->mutex, other.mutex);
> // ... do swap ...
> }
> If addressof(other) == this same mutex will be locked, and there are no precondition which blocks this usage.
> But in fact its undefined behavior (implementation uses try_lock on already locked mutex) (deadlock on msvc-stl, just ub on others)
> There are several solutions to this, either we explicitly add precondition, or the implementation must handle this (then there could potentially be an overhead when unlocking)

No, it does not. It is UB, but it shouldn't be in `scoped_lock`'s preconditions.

`scoped_lock`'s constructor is stated to use `std::lock` to lock the
sequences of mutexes. And `std::lock` will lock each given mutex in
turn, in an unspecified order. If the same mutex is in the list twice,
then one *will* be locked before the other.

And therefore, you will get the behavior expected if you try to lock
the mutex twice. For `std::mutex`, this is UB. For
`std::recursive_mutex`, this is well-defined. As such, the validity of
`scoped_lock` in this circumstance depends on the lock types. And it
is ultimately well-specified based on those lock types.

Also, any user-provided swapping behavior *must* take into account
self-swapping. This code did not, and therefore suffers the
consequences of not doing so. A precondition wouldn't fix it.

Received on 2023-09-13 15:48:19