CODEWITHSUNDEEP
c++multithreadingc++11mutexswap
Daniel
Daniel

Reputation: 8451

Implementing swap for class with std::mutex

Suppose we have a class with a std::mutex:

class Foo
{
    std::mutex mutex_;
    std::string str_;
    // other members etc
public:
    friend void swap(Foo& lhs, Foo& rhs) noexcept;
}

What is the appropriate way to implement the swap method here? Is it required/safe to lock each mutex separately and then swap everything? e.g.

void swap(Foo& lhs, Foo& rhs) noexcept
{
    using std::swap;
    std::lock_guard<std::mutex> lock_lhs {lhs.mutex_}, lock_rhs {rhs.mutex_};
    swap(ls.str_, rhs.str_);
    // swap everything else
}

I've seen that in C++17, std::lock_guard will have a constructor taking multiple mutexes for avoiding deadlock, but I'm not sure if that's an issue here?

Upvotes: 14

Views: 2148

Answers (2)

Lingxi
Lingxi

Reputation: 14987

I don't think your swap implementation is safe. If another algorithm tries to lock rhs.mutex_ first and then lhs.mutex_, you may end up with a deadlock. Try std::lock() instead.

Upvotes: 2

malchemist
malchemist

Reputation: 444

You can use std::lock() to acquire the locks in a non-deadlocking way.

If you want to use std::lock_guard, have them adopt the locks once taken:

std::lock(lhs.mutex_, rhs.mutex_);
std::lock_guard<std::mutex> lock_a(lhs.mutex_, std::adopt_lock);
std::lock_guard<std::mutex> lock_b(rhs.mutex_, std::adopt_lock);
//swap actions
swap(ls.str_, rhs.str_);

If you prefer std::unique_lock, then construct them without locking, then call std::lock() to lock them both (this also works with std::lock_guard):

std::unique_lock<std::mutex> lock_a(lhs.mutex_, std::defer_lock);
std::unique_lock<std::mutex> lock_b(rhs.mutex_, std::defer_lock);
std::lock(lock_a, lock_b);
//swap actions
swap(ls.str_, rhs.str_);

In both cases, you should first test for lhs and rhs being the same object, because using std::lock with one mutex twice is undefined behavior:

if (&lhs == &rhs)
    return;

Upvotes: 12

Related Questions