[Bug libstdc++/89417] helgrind detects a lock order violation inside std::scoped_lock

[redi at gcc dot gnu.org]

https://gcc.gnu.org/bugzilla/show_bug.cgi?id=89417

--- Comment #2 from Jonathan Wakely <redi at gcc dot gnu.org> ---
(In reply to Federico Kircheis from comment #0)
> In case helgrind is correct, it seems that there are some issues behind
> std::scoped_lock, since it was explicitly designed for solving issues with
> lock order.

Our implementation matches the specification in the standard, and the reference
implementation in
http://www.open-std.org/jtc1/sc22/wg21/docs/papers/2017/p0156r2.html

It was designed to avoid deadlock when locking, not necessarily by considering
lock order.

> In case helgrind (and possibly for the same reason other tools) is wrong,
> this is would be a feature request.
> 
> 
> A possible fix (or improvement) would be for `std::scoped_lock` to sort its
> arguments by address (since std::mutex are not copyable or movable, and thus
> their address should remain constant):

The scoped_lock type just uses the std::lock algorithm for locking multiple
mutexes, and  that is specified to avoid deadlock even if another thread is
locking the same mutexes in a different order. Ordering them in
std::scoped_lock doesn't help, you would need to do it in std::lock.

But there is no requirement that the same set of mutexes are always locked
using std::scoped_lock or std::lock, they can be manually locked using
individual std::lock_guard locks or bare calls to the mutex's lock() member
functions, in arbitrary orders. So ordering them in std::lock still doesn't
solve anything.

Also, I'm not even sure what the helgrind error really means. Just because they
were locked in one order at one time doesn't establish an invariant that they
must always be locked in that order elsewhere in the program. So I don't see
anything that needs to be fixed. If you specific program has an invariant that
the order is preserved, then don't pass them to the scoped_lock constructor in
different orders.