Christopher Currie said:
This question sparked another "usage convetion" question in my memory that came up around the office lately. I've been slow introducing the members of my development team to a subset of Boost.Threads and trying to get them to use RAII thinking when implementing their locks.
There seem to be two schools of thought on how to synchronize access to a class. One school, primarly from the developers who have experience in Java, tend to write classes that do their own mutex locking:
Please note that Java allows both types of synchronization to objects.
class CalleeLocked { private: typedef boost::mutex mutex_type; mutex_type m_mutex;
public: void synchronizedFunction() { mutex_type::scoped_lock lock( m_mutex ); // ... do work ... } };
The other school, to which I must admit I belong, believes that it should be the caller's responsibility to perform the locking:
class CallerLocked { public: typedef boost::mutex mutex_type;
void unsynchronizedFunction() { // ... do work ... }
mutex_type & myMutex() { return m_mutex; }
private: mutex_type m_mutex; }
void Caller( CallerLocked & c ) { CallerLocked::mutex_type::scoped_lock lock( c.myMutex() ); c.unsynchronizedFunction(); }
The Callee lockers believe that is more important for the class to implement the locking, because the class knows what data it has that requires synchronized access, and that putting the locking inside the class means that you only have to get it right once. The Caller lockers argue that adding locks to all your class functions introduces overhead that is unnecessary if you never share instances between threads, and that making the locking explicit in the caller makes it somewhat easier to prevent, or at least track down, deadlock errors.
With the exception of accessing external, global data, inside a class method, if any of the data needs protecting, all of it does. So I'm not sure I agree with that argument. The argument about added overhead, even when not accessed by multiple threads, is a legitimate concern. There's a design pattern for this, where the mutex type used is a template parameter and a "null mutex" which does no synchronization can be chosen in cases where there will be no sharing. I'm not sure I agree that making the locking explicit at the call site will make it any easier to prevent or track down deadlock errors. A bigger issue with internal locking that you've not mentioned is that this approach limits the granularity of the lock to the method call. This is one reason why Java supports both types of synchronization. Andrei Alexandrescu has a nice article on this very subject at http://www.informit.com/isapi/product_id~{E3967A89-4E20-425B-BCFF-B84B6DEED6CA}/session_id~{021CB913-CD4D-49C4-9B32-4240CC8BA93C}/content/index.asp. -- William E. Kempf