CODEWITHSUNDEEP
c++multithreadingconcurrency
meriam
meriam

Reputation: 393

Potential deadlock in thread-safe stack C++

In the book "Concurrency in Action", there's an implementation of thread-safe stack where mutex is acquired/locked upon entering pop() and empty() functions like shown below:

class threadsafe_stack {
   private:
      std::stack<T> data;
      mutable std::mutex m;
   public:
      //...
      void pop(T& value) {
         std::lock_guard<std::mutex> lock(m);
         if(data.empty()) throw empty_stack();
         value = std::move(data.top());
         data.pop();
      }

      bool empty() const {
         std::lock_guard<std::mutex> lock(m);
         return data.empty();
      }
};

My question is, how does this code not run into a deadlock, when a thread, that has acquired the lock upon entering pop() is calling empty() which is protected by mutex as well? If lock() is called by the thread that already owns the mutex, isn't that undefined behavior?

Upvotes: 4

Views: 192

Answers (3)

abhiarora
abhiarora

Reputation: 10460

how does this code not run into a deadlock, when a thread, that has acquired the lock upon entering pop() is calling empty() which is protected by mutex as well?

Because you are not calling empty member function of threadsafe_stack but you are calling empty() of class std::stack<T>. If the code would be:

void pop(T& value) 
{
    std::lock_guard<std::mutex> lock(m);
    if(empty()) // instead of data.empty()
        throw empty_stack();
    value = std::move(data.top());
    data.pop();
}

Then, it would be undefined behavior:

If lock is called by a thread that already owns the mutex, the behavior is undefined: for example, the program may deadlock. An implementation that can detect the invalid usage is encouraged to throw a std::system_error with error condition resource_deadlock_would_occur instead of deadlocking.

Learn about recursive and shared mutex.

Upvotes: 2

463035818_is_not_an_ai
463035818_is_not_an_ai

Reputation: 123431

You would be correct if pop would call this->empty. Locking the same mutex twice via a std::lock_guard is undefined behavior unless the locked mutex is a recursive one.

From cppreference on the constructor (the one that is used in the example code):

Effectively calls m.lock(). The behavior is undefined if m is not a recursive mutex and the current thread already owns m.

For the sake of completeness, there is a second constructor:

lock_guard( mutex_type& m, std::adopt_lock_t t );

which

Acquires ownership of the mutex m without attempting to lock it. The behavior is undefined if the current thread does not own m.

However, pop calls data.empty and this is the method of the private member, not the member function empty of threadsafe_stack. There is no problem in the code.

Upvotes: 1

Lukas-T
Lukas-T

Reputation: 11370

Not 100% sure what you mean, I guess you mean calling pop and empty sequentially in the same thread? Like in 

while(!x.empty()) x.pop();

std::lock_guard follows RAII. This means the constructor 

std::lock_guard<std::mutex> lock(m);

will acquire/lock the mutex and the destructor (when lock goes out of scope) will release/unlock the mutex again. So it's unlocked at the next function call.

Inside pop only data.empty() is called, which is not protected by a mutex. Calling this->empty() inside pop would indeed result in undefined behaviour.

Upvotes: 1

Related Questions