Lock an rvalue argument for thread safe queue's move constructor?

Question

While reading the book c++ concurrency in action,I'm trying to write a thread-safe queue.

The code:

template<typename T>
class ThreadsafeQueue
{
public:
    using Guard = std::lock_guard<std::mutex>;

    //! default Ctor
    ThreadsafeQueue() = default;

    //! copy Ctor
    ThreadsafeQueue(ThreadsafeQueue const& other)
    {
        Guard g{other.mutex_};
        q_ = other.q_;
    }

    //! move Ctor <----my question
    ThreadsafeQueue(ThreadsafeQueue && other)noexcept
    {
        q_ = std::move(other.q_);
    }

    //! other members...

private:
    std::queue<T> q_;
    std::mutex mutex_;
    std::condition_variable cond_;
};

My question is whether should I lock the argument's other.mutex_ in the move constructor? Why?

score 1 · Accepted Answer · edited May 23 '17 at 12:27

Moving a value is a mutating operation.

Mutating requires exclusive access in order to not have a data race.

Therefore the caller should already hold the lock. The caller that passes the object as a rvalue knows it's going to be mutated (or at least promises that it's okay for the receiving function to do so).

REALITY: None of this is likely to happen. Since you're moving the whole queue, you're likely to be in a spot of the application that is still (logically) single-threaded w.r.t the queue.

Locking is only necessary after the point where concurrent access is possible.

It's a bit catch-22 too: how can you lock a mutex that you're gonna move?

Move constructor for std::mutex

my two cents is that you should go ahead an do the technically safe way (or at least default to the safe way) for the same reason it is not likely to happen: it most likely won't be happening in a critical part of the code, so it should hurt anything to do it "correct". — IdeaHat, Nov 21 '14 at 23:18

score 1 · Answer 2 · answered Nov 21 '14 at 23:20

Depends on how you define the moved-out state of your ThreadsafeQueue. I see two reasonable common ways:

Moved-out queue is empty, and it's OK to push new items into it and so on. Then the answer to your question is yes. I personally prefer this in most cases.
Moved-out queue is left in unspecified state and can be destructed or assigned to only. Then the answer is no. And it's user's responsibility to not move from a queue which can be used somewhere else (the same idea about destructor).

Interestingly the "responsibility to not (do _X_) from a _X_ which can be used somewhere else is commonly handled by ... mutual exclusion. — sehe, Nov 21 '14 at 23:32

Lock an rvalue argument for thread safe queue's move constructor?

2 Answers2