CODEWITHSUNDEEP
c++multithreadingc++11
David
David

Reputation: 28188

How can I lock a mutex in an initializer list?

I have a ConcurrentQueue class that is based around a user provided container with a constructor like this...

ConcurrentQueue(const ConcurrentQueue& other) : m_Queue(other.m_Queue) {}

But, I need to lock other's mutex while it's being copied.

Option 1:

So I could not use the copy constructor at all, and do...

ConcurrentQueue(const ConcurrentQueue& other) : m_Queue(other.m_Queue)
{
    std::lock_guard<std::mutex> lock(other.m_Mutex);
    m_Queue = other.m_Queue;
}

But I can't guarantee that copy assignment and copy construction are equivalent functionality.

Option 2:

I could have a private method...

std::queue<T, Container> GetQueue() const
{
    std::lock_guard<std::mutex> lock(other.m_Mutex);
    return m_Queue;
}

And then in the constructor do this...

ConcurrentQueue(const ConcurrentQueue& other) : m_Queue(other.GetQueue()) {}

But this potentially (depending on optimizations) uses m_Queue's copy constructor once and it's move constructor once. And I also can't guarantee that a copy and a move is equivalent to just a copy. Additionally the user provided container could be bizarre and be copyable but unmoveable, which would also cause this approach to have problems.

So, what do I do?

Upvotes: 8

Views: 1719

Answers (2)

Ulrich Eckhardt
Ulrich Eckhardt

Reputation: 17424

Lock, create a copy of the content and then swap it with the member. At least that's the easiest and IMHO cleanest way. Another less clean way is to use the comma operator: (a, b) yields b, but if a is a scoped lock, the temporary will live until the next sequence point, i.e. until you have used b to initialize your local copy.

That said, there are two things to consider:

  • Maybe copying is not such a smart idea anyway and your design works as well if you just disable copying.
  • If you have access to the queue and you can read it for copying, doesn't that imply that the mutex must already be locked? If not, how are you sure that you really want to copy the queue? I don't question that there are answers that justify the design, but it's unusual.

Upvotes: 1

James Kanze
James Kanze

Reputation: 153977

ConcurrrentQueue::ConcurrrentQueue(
        ConcurrrentQueue const& other )
    : m_Queue( (std::lock_guard<std::mutex>( other.m_Mutex ),
               other.m_Queue ) )
{
}

should work.

Upvotes: 9

Related Questions