views:

205

answers:

6

I've a class that is accessed from multiple threads. Both getter and setter functions are guarded with locks. Are the locks for getter functions relly needed? Why?

class foo {
public:
    void setCount (int count) {
        boost::lock_guard<boost::mutex> lg(mutex_);
        count_ = count;
    }

    int count () {
        boost::lock_guard<boost::mutex> lg(mutex_); // mutex needed?
        return count_;
    }

private:
    boost::mutex mutex_;
    int count_;
};
+3  A: 

If you don't have a mutex around the getter, and a thread is reading it while another thread is writing it, you'll get funny results.

Ink-Jet
Not necessarily. Reading and writing an `int` are probably atomic operations. Of course this is architecture dependent and not portable.
Dan
Exactly - it's undefined behavior. It's better to have the minute overhead of a lock, as opposed to memory being corrupted.
Ink-Jet
Usually, yes. But if you have a demonstrated need to increase performance, it may be possible to omit the lock. See my answer.
Dan
+8  A: 

The only way you can get around having the lock is if you can convince yourself that the system will transfer the guarded variable atomicly in all cases. If you can't be sure of that for one reason or another, then you'll need the mutex.

For a simple type like an int, you may be able to convince yourself this is true, depending on architecture, and assuming that it's properly aligned for single-instruction transfer. For any type that's more complicated than this, you're going to have to have the lock.

Michael Kohne
+1  A: 

in you case probably not, if your cpu is 32 bit, however if count is a complex object or cpu needs more than one instruction to update its value, then yes

test1
+2  A: 

Is the mutex really only protecting a single int? It makes a difference -- if it is a more complex datatype you definitely need locking.

But if it is just an int, and you are sure that int is an atomic type (i.e., the processor will not have to do two separate memory reads to load the int into a register), and you have benchmarked the performance and determined you need better performance, then you may consider dropping the lock from both the getter and the setter. If you do that, make sure to qualify the int as volatile. And write a comment explaining why you do not have mutex protection, and under what conditions you would need it if the class changes.

Also, beware that you don't have code like this:

void func(foo &f) {
  int temp = f.count();
  ++temp;
  f.setCount(temp);
}

That is not threadsafe, regardless of whether you use a mutex or not. If you need to do something like that, the mutex protection has to be outside the setter/getter functions.

Dan
A: 

The lock is necessary to serialize access to shared resource. In your specific case you might get away with just atomic integer operations but in general, for larger objects that require more then one bus transaction, you do need locks to guarantee that reader always sees a consistent object.

Nikolai N Fetissov
A: 

It depends on the exact implementation of the object being locked. However, in general you do not want someone modifying (setting?) an object while someone else is in the process of reading (getting?) it. The easiest way to prevent that is to have a reader lock it.

In more complicated setups the lock will be implemented in such a way that any number of folks can read at once, but nobody can write to it while anyone is reading, and nobody can read while a write is going on.

T.E.D.