CODEWITHSUNDEEP
c++multithreadingclassboostmutex
confusedCoder
confusedCoder

Reputation: 223

class member mutex assertion failed

I'm trying to implement what I think is a fairly simple design. I have a bunch of objects, each containing a std::map and there will be multiple processes accessing them. I want to make sure that there is only one insert/erase to each of these maps at a time.

So I've been reading about boost::thread and class member mutexes and using bind to pass to class member which are all new things to me. I started with a simple example from a Dr. Dobbs article and tried modifying that. I was getting all kinds of compiler errors due to my Threaded object having to be noncopyable. After reading up on that, I decided I can avoid the hassle by keeping a pointer to a mutex instead. So now I have code that compiles but results in the following error:

/usr/include/boost/shared_ptr.hpp:419:
    T* boost::shared_ptr< <template-parameter-1-1> >::operator->() const
    [with T = boost::mutex]: Assertion `px != 0' failed.  Abort

Now I'm really stuck and would really appreciate help with the code as well as comments on where I'm going wrong conceptually. I realize there are some answered questions around these issues here already but I guess I'm still missing something. 

#include <boost/thread/thread.hpp>
#include <boost/thread/mutex.hpp>
#include <boost/bind.hpp>
#include <boost/shared_ptr.hpp>
#include <iostream>
#include <map>

using namespace std;

class Threaded {
public:
  std::map<int,int> _tsMap;

  void count(int id) {
    for (int i = 0; i < 100; ++i) {
      _mx->lock();
      //std::cout << id << ": " << i << std::endl;      
      _tsMap[i] ++;
      _mx->unlock();
    }
  }

private:
  boost::shared_ptr<boost::mutex> _mx;
};


int main(int argc, char* argv[]) {
  Threaded th;
  int i = 1;
  boost::thread thrd1(boost::bind(&Threaded::count, &th, 1));
  //boost::thread thrd2(boost::bind(&th.count, 2));
  thrd1.join();
  //thrd2.join();

  return 0;
}

Upvotes: 1

Views: 1308

Answers (3)

John Humphreys
John Humphreys

Reputation: 39294

Conceptually, I think you do have a problem. Copying a std::shared_ptr will just increase its reference count, and the different objects will all use the same underlying mutex - meaning that whenever one of your objects is used, none of the rest of them can be used.

You, on the other hand, need each object to get its own mutex guard which is unrelated to other objects mutex guards.

What you need is to keep the mutex defined in the class private section as it is - but ensure that your copy constructor and copy assignment operator are overloaded to create a new one from scratch - one bearing no relation to the mutex in the object being copied/assigned from.

Upvotes: 1

Drew Hall
Drew Hall

Reputation: 29055

It looks like you're missing a constructor in your Threaded class that creates the mutex that _mx is intended to point at. In its current state (assuming you ran this code just as it is), the default constructor for Threaded calls the default constructor for shared_ptr, resulting in a null pointer (which is then dereferenced in your count() function.

You should add a constructor along the following lines:

Threaded::Threaded(int id)
  : _mx(new boost::mutex())
  , _mID(id) 
{
}

Then you could remove the argument from your count function as well.

Upvotes: 2

ravenspoint
ravenspoint

Reputation: 20472

A mutex is non-copyable for good reasons. Trying to outsmart the compiler by using a pointer to a mutex is a really bad idea. If you succeed, the compiler will fail to notice the problems, but they will still be there and will turn round and bite you at runtime.

There are two solutions

  • store the mutex in your class as a static
  • store the mutex outside your class.

There are advantages for both - I prefer the second.

For some more discussion of this, see my answer here mutexes with objects

Upvotes: 1

Related Questions