C++ Logo

std-proposals

Advanced search

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

From: Nikl Kelbon <kelbonage_at_[hidden]>
Date: Thu, 14 Sep 2023 09:55:09 +0400
> No, it does not. It is UB, but it shouldn't be in `scoped_lock`'s
preconditions.
Im about std::lock / std::try_lock

> `std::lock` will lock each given mutex in
turn, in an unspecified order.

As quoted:
*> Effects*: All arguments are locked via a sequence of calls to lock(),
try_lock(), or unlock() on each argument.
<https://eel.is/c++draft/thread.lock.algorithm#5.sentence-1>

Sequence may be empty and do not contain any call to lock or try lock

> Also, any user-provided swapping behavior *must* take into account
self-swapping.

No, in fact swap in any cases used to not handle self assign in move assign
and this is rare case when it will break something

> A precondition wouldn't fix it.

But handling such case will. Precondition atleast will make it explicit,
now without seeing implementation it is not obvious

If you think it’s must not be handled by std::lock, then try write code
that will handle this correctly for a pack of mutexes or at least just a
few mutexes

This is incredibly error prone

чт, 14 сент. 2023 г. в 02:33, Jason McKesson via Std-Proposals <
std-proposals_at_[hidden]>:

> 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.
> --
> Std-Proposals mailing list
> Std-Proposals_at_[hidden]
> https://lists.isocpp.org/mailman/listinfo.cgi/std-proposals
>

Received on 2023-09-14 05:55:25