Make a safe Reader/Writer vector

Question

Inspired by this code, I am trying to implement a Reader/Writer vector that can safely call push_back() concurrently by threads.

Once this class is in place, I might then create method erase() by calling std::swap(), which swaps the target item and the last item and then erase the last item in the collection. In this way, I assume that the performance should be fair because deleting an item in the middle of collection does not invoke moving all items following the target item in the collection.

Unfortunately, the following code:

#include <vector>
#include <boost/thread/shared_mutex.hpp> //shared_mutex
#include <memory> //shared_ptr
#include <utility> //swap()

template <class T>
class readers_writer_vector
{
    std::shared_ptr<boost::shared_mutex> pm;
    std::vector<T> data;
public:
    readers_writer_vector() :
        pm(new std::shared_ptr<boost::shared_mutex>){}
    void push_back(const T& item){
        boost::unique_lock<boost::shared_mutex> lock(*pm); //wrong design
        data.push_back(item);
    }
};

int main()
{
    readers_writer_vector<int> db;
    db.push_back(1);
    return 0;
}

yields the following compilation errors:

/usr/include/c++/4.9/bits/shared_ptr_base.h:871:39: error: cannot convert ‘std::shared_ptr<boost::shared_mutex>*’ to ‘boost::shared_mutex*’ in initialization
         : _M_ptr(__p), _M_refcount(__p)

// g++ -std=c++11 -Iboost -lboost t.cpp

How do I fix it? Please!

EDIT:

The implementation task is far more complicated than I thought. It didn't take too long before I encountered the problem @Danh had warned. Now I get these errors:

t.cpp:28:8: note: ‘i::i(const i&)’ is implicitly deleted because the default definition would be ill-formed:
 struct i {
        ^
t.cpp:28:8: error: use of deleted function      ‘readers_writer_vector<T>::readers_writer_vector(const readers_writer_vector<T>&) [with T = z]’
t.cpp:13:2: note: declared here 
  readers_writer_vector(readers_writer_vector const&) = delete;

with this version:

template <class T>
class readers_writer_vector
{
    booster::shared_mutex m;
    std::vector<T> data;
public:
    readers_writer_vector() = default;
    readers_writer_vector(readers_writer_vector const&) = delete;
    void push_back(const T& item){
        booster::unique_lock<booster::shared_mutex> lock(m);
        data.push_back(item);
    }
    typename std::vector<T>::reference back(){
        return data.back();
    }
};

struct z {
    int zipcode;
    std::string address;
};

struct i {
    int id;
    readers_writer_vector<z> zipcodes;
};

int main()
{
    readers_writer_vector<i> db;
    db.push_back(i());
    auto &ii=db.back();
    ii.id=1;
    ii.zipcodes.push_back(z());

    auto &zz=ii.zipcodes.back();
    zz.zipcode=11;
    zz.address="aa";

    return 0;
}

In addition to fixing the existing errors, I will have to implement iterators for readers_writer_vector to make this class useful.

I am pondering whether or not I should continue...

score 3 · Accepted Answer · answered Nov 28 '16 at 10:19

3

Because pm is std::shared_ptr<boost::shared_mutex> not std::shared_ptr<boost::shared_mutex>*. You can use this:

readers_writer_vector() :
    pm(std::make_shared<boost::shared_mutex>()){}

Anyway, why do you need pointer/smart pointer? This is better fit:

template <class T>
class readers_writer_vector
{
    boost::shared_mutex pm;
    std::vector<T> data;
public:
    void push_back(const T& item){
        boost::unique_lock<boost::shared_mutex> lock(pm);
        data.push_back(item);
    }
};

answered Nov 28 '16 at 10:19

Danh

5,916
7
30
45

I used smart pointer because I thought putting mutex directly in container , like `std::vector v;`, would not compile according to @T.E.D. `An unfortunate implication of this is that a mutex cannot be placed into a container directly. Containers need the ability to safely move their contents around, and you can't do that with a mutex.` and the code illustrated by @Adrian Maire. – Masao Liu Nov 28 '16 at 14:08
shared mutex between container doesn't make sense to me – Danh Nov 28 '16 at 14:44
I do not intend to share mutex between containers. Now I presume T.E.D. was talking about different use case from mine. – Masao Liu Nov 28 '16 at 15:05
When you copy construct a `readers_writer_vector` from other `readers_writer_vector`, you're indeed shared the mutex with your original code, my suggestion should disable the copy – Danh Nov 28 '16 at 15:06
I am facing this contradiction: Copy constructor can not be disabled because `std::vector::push_back()` needs it. – Masao Liu Nov 28 '16 at 16:37
No, it will disable copy constructor of readers_writer_vector only, it doesn't disable copy constructor of T – Danh Nov 29 '16 at 01:34

score 1 · Answer 2 · answered Nov 28 '16 at 10:24

You're initialising pm with the wrong type; you effectively have

std::shared_ptr<> pm = new std::shared_ptr<>;

You can't assign a shared pointer from a pointer to shared pointer.

Replace the initialiser with

pm(new boost::shared_mutex)

or make the mutex a member directly, rather than using shared pointer.

Make a safe Reader/Writer vector

2 Answers2