2

What's wrong with the following code? I just try to set up a quite simple thread-safe stack, when I run several threads to concurrently push and pop on the stack, then it sometimes reports 0xC0000005 exception.

#include <stack>
#include <list>
#include <memory>
#include <mutex>


template<class T> class Stack{
    typedef stack < shared_ptr<T>, list<shared_ptr<T>>> Stack_;
public:
    Stack(){}
    Stack(const Stack& other){
        lock_guard<mutex>(other.mtx_);
        stack_ = other.stack_;
    }
    void push(shared_ptr<T> value){
        lock_guard<mutex>(this->mtx_);
        stack_.push(value);
    }
    shared_ptr<T> pop(){
        lock_guard<mutex>(this->mtx_);
        if (stack_.empty()) return nullptr;
        auto res = stack_.top();
        stack_.pop();
        return res;
    }
private:
    mutex mtx_;
    Stack_ stack_;
};
Lightness Races in Orbit
  • 378,754
  • 76
  • 643
  • 1,055
yangwenjin
  • 51
  • 1
  • 3

1 Answers1

7

The main problem I see is you are not locking your resources properly.

This:

lock_guard<mutex>(this->mtx_); // temporary

That will unlock immediately after you locked it because it is a temporary and only lives until the ;.

You should create a named variable like this:

lock_guard<mutex> lock(this->mtx_); // lives till end of scope

See: This Post for information on temporary objects.

Galik
  • 47,303
  • 4
  • 80
  • 117