Why does this Concurrent Hasmap using boost not work

Question

I have the following concurrent hash map:

template<typename Key, typename Value>
class ConcurrentMap
{

public:
    ConcurrentMap() {};
    bool has(Key k)
    {
        boost::shared_lock<boost::shared_mutex> lock(schemaAccess);
        return m.find(k) != m.end();
    }

    void set(Key k, Value v)
    {
        boost::shared_lock<boost::shared_mutex> lock(schemaAccess);

        // set k, v
        if(m.find(k) != m.end())
        {
            boost::upgrade_lock<boost::shared_mutex> valueLock(*valueAccess[k]);
            boost::upgrade_to_unique_lock<boost::shared_mutex> valueUniqueLock(valueLock);

            m.at(k) = v;
        }
        // insert k, v
        else
        {
            lock.unlock();
            boost::upgrade_lock<boost::shared_mutex> schemaLock(schemaAccess);
            boost::upgrade_to_unique_lock<boost::shared_mutex> schemaUniqueLock(schemaLock);

            boost::shared_ptr<boost::shared_mutex> mtx = boost::make_shared<boost::shared_mutex>();
            valueAccess.insert(std::pair<Key, boost::shared_ptr<boost::shared_mutex> >(k, mtx));
            m.insert(std::pair<Key,Value>(k,v));
        }
    }

    Value get(Key k)
    {
        boost::shared_lock<boost::shared_mutex> lock(schemaAccess);
        return m.at(k);
    }

    bool get(Key k, Value& v)
    {
        boost::shared_lock<boost::shared_mutex> lock(schemaAccess);
        if(m.find(k) != m.end()){
            v = m.at(k);
            return true;
        }
        return false;
    }


private:
    std::map<Key, Value> m;

    std::map<Key, boost::shared_ptr<boost::shared_mutex> > valueAccess;
    boost::shared_mutex schemaAccess;

    //http://stackoverflow.com/questions/36468270/how-to-use-a-boostmutex-as-the-mapped-type-in-stdmap/36469809?noredirect=1#comment60552035_36469809
};

And the following usage:

ConcurrentMap<int, cv::Mat> mapper;

void worker1(){
    int counter = 0;
    while(1){
        boost::this_thread::sleep(boost::posix_time::milliseconds(5));
        cv::Mat img = cv::Mat(1920,1080, CV_64F);
        cout << "w1" << counter++ << endl;
        mapper.set(0,img);
    }
}

void worker2(){
    int counter = 0;
    while(1){
        boost::this_thread::sleep(boost::posix_time::milliseconds(5));
        cout << "w2" << counter++ << endl;
        cv::Mat img;
        if(!mapper.get(0, img)) continue ;
        cout << img.rows << endl;
        cout << "done" << endl;
    }
}

int main(int argc, char **argv)
{
    ros::init(argc, argv, "loading_area_concat");


    boost::thread worker1_;
    boost::thread worker2_;
    worker1_ = boost::thread(worker1);
    worker2_ = boost::thread(worker2);

    ros::spin();

    return 0;
}

*** Error in `/home/raaj/catkin_ac2/devel/lib/loading_area/loading_area_concat': free(): invalid pointer: 0x00007fb71c000078 ***

However, my code keeps on crashing at the "get" part in the second thread. If i comment out the "get" it works. Either that, or if I fully lock the get code with the following below it works:

    boost::upgrade_lock<boost::shared_mutex> schemaLock(schemaAccess);
    boost::upgrade_to_unique_lock<boost::shared_mutex> schemaUniqueLock(schemaLock);

I can't seem to understand why this happens. I am locking it when I am writing to that key in the hash map, and when I am reading it, the writer shouldnt come in and corrupt the memory yes?

Answer 1

Did you try to lock in get() as well and check if the problem goes away then? From a high-level point of view, the reads only don't need to be locked, if nothing writes. If there is some thread which writes to the location, the reads need to be protected as well, otherwise there is a race condition (get() might end up using things which are no longer valid).

Ok, I see the edit that the locking in get() solves the issue, so it indeed seems to be the case.

The thing is, that concurrent reads of unchanging memory do not need to be locked. But if there is a concurrent write, the reads need to be locked as well (against the write, otherwise all kinds of strange things might happen, like partial/interleaved reads, etc.).