4

I have a stl map which I would like to be synchronized across several threads. Currently I have...

Function A (Modifies map)

void Modify(std::string value)
{
    pthread_mutex_lock(&map_mutex);

    my_map[value] = value;

    pthread_mutex_unlock(&map_mutex);
}

Function B (Reads map)

std::string Read(std::string key)
{
    std::string value;

    pthread_mutex_lock(&map_mutex);

    std::map<std::string, std::string>::iterator it = my_map.find(key);

    pthread_mutex_unlock(&map_mutex);

    if(it != my_map.end())
    {
        return it->second;
    }
    else
    {
        return "DNE";
    }
}

This is synchronized across all threads, due to the mutex. However, I have to lock the mutex in Function B even though it is not modifying the map at all. Is there a way to lock the my_map object itself in function A, and not lock it in function B while keeping thread synchronization. This way, all instances/calls of Function B continue to run freely, so long as Function A is not being run?

Thanks

Joshua
  • 1,516
  • 2
  • 22
  • 31
  • 8
    You're looking for a reader/writer lock. I know that boost has one (it's called `shared_lock`). – R. Martinho Fernandes Jun 04 '12 at 18:21
  • 6
    Pthreads seem to already have it too: `pthread_rwlock_t` – hamstergene Jun 04 '12 at 18:24
  • 7
    "*Function B … is not modifying the map at all*". [Wanna bet?](http://ideone.com/7jnQE) `operator[]` can modify its map. – Robᵩ Jun 04 '12 at 18:26
  • @RobᵩI was going to point this out too, I had a nasty bug recently caused by exactly this. - use*find*instead if you don't want to add the missing item – jcoder Jun 04 '12 at 18:28
  • @hamstergene does the pthread_rw_lock (when it has to write) wait for the reading to be done? – Joshua Jun 04 '12 at 18:29
  • @R.MartinhoFernandes does the shared_lock(when it has to write) wait for the reading to be done? – Joshua Jun 04 '12 at 18:29
  • @Robᵩ Yes, thank you, I forgot, I will change the code asap – Joshua Jun 04 '12 at 18:30
  • Well, the only other option for operator[] would be to throw an exception so it's actually logical it modifies the map. – ScarletAmaranth Jun 04 '12 at 18:31
  • @Joshua yes, that's the point. At any one time you have either: no thread holding the lock; one thread holding the lock for writing; one or more threads holding the lock for reading. I suppose the mentioned `pthread_rwlock_t` works the same. – R. Martinho Fernandes Jun 04 '12 at 18:32
  • @R.MartinhoFernandes It seems that the pthread implementation is/can be slower, I will start out with the boost implementation and profile it to see for sure. Thanks for the info! – Joshua Jun 04 '12 at 18:44
  • 2
    Using the `find` iterator in `Read` outside the lock is bad news I would think. An over-write of the returned element can invalidate this iterator resulting in a horrific hard-to-find bug. Also your locks as shown here are not exception-safe, if you stick with POSIX use guards as with `boost::shared_lock`. Any good reason to not pass in the params as `const std::string&` ? – Steve Townsend Jun 04 '12 at 19:08
  • While i dont want to question the decisions made by OP i want to point to the Poco Framework which has a very easy to use and though complete multithreading support. http://pocoproject.org/slides/130-Threads.pdf – Sebastian Hoffmann Jun 04 '12 at 20:42

3 Answers3

3

You don't just want to lock the container, you also want to lock accesses into the container i.e. any iterators or pointers into it. You need to move those accesses into the locked region of the code.

std::string Read(std::string key)
{
    std::string value = "DNE";

    pthread_mutex_lock(&map_mutex);

    std::map<std::string, std::string>::iterator it = my_map.find(key);
    if(it != my_map.end())
    {
        value = it->second;
    }

    pthread_mutex_unlock(&map_mutex);

    return value;
}

There's really no practical way to do this from inside the object itself.

Mark Ransom
  • 299,747
  • 42
  • 398
  • 622
  • Note that this code is not exception safe. If `find` throws an exception, the mutex will be left locked ... likely forever. (That's probably fine for a map whose key is a `std::string`, but it can be a problem for a map that uses a type whose comparison operators can throw.) – David Schwartz Jun 04 '12 at 21:39
  • @DavidSchwartz, I didn't get into that because as you say `std::string` is unlikely to throw (except for std::bad_alloc), and other answers had already covered the solutions to that problem. – Mark Ransom Jun 04 '12 at 21:51
1

Warning: I have not compiled or tested any of this, but I've done similar things in the past.

Step one would be to control the mutex with class, as such:

class Lock {
    public:
        Lock(Mutex& mutex) {
            pthread_mutex_lock(mutex);
        }
        ~Lock(Mutex& mutex) {
            pthread_mutex_unlock(mutex);
        }
};

This saves you from all sorts of issues, for instance, if your map throws an exception.

Then your modify becomes:

void Modify(std::string value)
{
    Lock(map_mutex);    

    my_map[value] = value;
}

Create a reference counted lock class:

class RefCntLock {
    private:
        static int count;
        static Lock* lock;

    public:
        RefCountLock(Mutex& mutex) {

             // probably want to check that the mutex matches prior instances.
             if( !lock ) {
                  lock = new Lock(mutex);
                  count++;
             }
        }
        ~RefCountLock() {
             --count;
             if( count == 0 ) {
                 delete lock;
                 lock = NULL;
             }
        }
}; 

(Note: it'd be easy to generalize this to deal with multiple mutexes.)

In your read, use the RefCntLock class:

std::string Read(std::string key)
{
    {
        RefCntLock(&map_mutex);

        std::map<std::string, std::string>::iterator it = my_map.find(key);
    }

    if(it != my_map.end())
    {
        return it->second;
    }
    else
    {
        return "DNE";
    }
}

This means that each write gets a lock but all reads share a lock.

Gort the Robot
  • 2,329
  • 16
  • 21
  • I don't understand how the `RefCntLock` works. It seems like if one read has occured, then the constructor won't relock or increment? You should increment anyway. Also the name of the constructor and destructor is wrong. Only adding for others to more quickly clear their confusion. Thanks for answering. – Motomotes Sep 11 '16 at 21:31
1

In the upcoming C++17 standard, you can use std::shared_mutex and std::shared_lock to allow multiple readers exclusive read access, and std::unique_lock to implement exclusive write access.

std::shared_mutex map_lock;

void Modify(std::string value)
{
    auto write_lock = std::unique_lock(map_lock);
    my_map[value] = value;
}

std::string Read(std::string key)
{    
    auto read_lock = std::shared_lock(map_lock);
    auto it = my_map.find(key);
    return (it != my_map.end()) ? it->second : "DNE";    
}
jxh
  • 69,070
  • 8
  • 110
  • 193