[Boost-users] boost::shared_locks and boost::upgrade_locks

Subject: [Boost-users] boost::shared_locks and boost::upgrade_locks
From: Kelvin Chung (kelvSYC_at_[hidden])
Date: 2011-12-07 19:05:38

I'm trying to implement a read-write lock, and I was told to use 
boost::upgrade_lock.  However, only one thread can have a 
boost::upgrade_lock, and everyone else gets blocked, so it's not much 
on the read end.  (Correct me if I am wrong, but boost::upgrade_lock 
only waits if someone has exclusive, but someone with 
boost::upgrade_lock will cause everyone else to block)

But reads are cheap in my case - I just need to use locks for a cache.  
So, suppose I have the following:

class Cache {
        std::map<Input, Output> cache;
        boost::shared_mutex mutex;
public:
        Output query(const Input& in);
};

It would appear that the proper way to implement Cache::query() is as follows:

Output query(const Input& in) {
        boost::shared_lock<boost::shared_mutex> readLock(mutex);
        if (cache.count(in) == 0) {
                readLock.unlock();
                boost::unique_lock<boost::shared_mutex> writeLock(mutex);
                
                // Check to see if the result was added while waiting for lock
                if (cache.count(in) != 0) return cache[in];
                
                cache[in] = compute_output(in);
                return cache[in];
        } else {
                return cache[in];
        }
}

Is this correct?  Or maybe it should be this:

Output query(const Input& in) {
        boost::shared_lock<boost::shared_mutex> readLock(mutex);
        if (cache.count(in) == 0) {
                readLock.unlock();
                boost::upgrade_lock<boost::shared_mutex> rereadLock(mutex);
                
                // Check to see if the result was added while waiting for lock
                if (cache.count(in) != 0) {
                        return cache[in];
                } else {
                        boost::upgrade_to_unique_lock<boost::shared_mutex> writeLock(rereadLock);
                        
                        cache[in] = compute_output(in);
                        return cache[in];
                }
        } else {
                return cache[in];
        }
}

A side question is that if compute_output() is an expensive operation, 
should that be moved to the part between releasing the read lock and 
getting the write/reread lock?