Sign in to follow this  
Permafried-

Thread Safety in Accessor Methods

Recommended Posts

Permafried-    274
Since I've been contemplating the best way to handle this for a while now I thought I'd look for some opinions here to help me out. I've been stuck on how to handle thread safety in my foundation classes for a while now particularly in regards to accessor methods. I've been playing with two options on how to handle this so far. The first, I lock the mutex and copy the member variable to a temporary function variable. Then, I unlock the mutex and return the temporary varable. For example:
int CClass::GetClassVariable( void )
{
   m_mutex.Lock();
   int iTempVar = m_iMemberVariable;
   m_mutex.Unlock();

   return iTempVar;
}


m_mutex is of type CMutex which is a wrapper class but the implementation isn't important enough to post that much code. My second option is to add an additional foundation class which acts as an automatic mutex. The constructor accepts a source mutex object and waits to get a lock. The destructor will then release access to the mutex object when it goes out of scope.
class CAutoMutex
{
public:
   CAutoMutex( CMutex& srcMutex )
   {
      srcMutex.Lock();      // wait indefinitely
      m_mutex = srcMutex;   // copy so i can unlock in destructor
   }

   virtual ~CAutoMutex( void )
   {
      m_mutex.Unlock();
   }

private:
   CAutoMutex( void );

   CMutex   m_mutex;
};


Now, using the same method as above it would execute as follows:
int CClass::GetClassVariable( void )
{
   CAutoMutex lock( m_classMutexToUseAsSource );

   return m_iMemberVariable;

   // lock will automatically unlock the mutex
   // when it goes out of scope here
}


I guess really what I'm looking for is feedback on what people feel the best approach would be of the above two or if there are any other ideas. Thanks,

Share this post


Link to post
Share on other sites
Sharlin    864
The second way, utilizing RAII, is certainly the preferred and idiomatic approach. In fact, the first one has a serious problem: it is not exception-safe. If an exception is thrown while m_mutex is locked, it is never released without ugly try/catch/rethrow wrappings.

Share this post


Link to post
Share on other sites
Permafried-    274
Quote:
Original post by Telastyn
The second is pretty much what boost::thread does, so umm... why not use that?

I'd considered using Boost for a number of things but I wanted to get some exposure to multi-threading and syncronization myself rather than using a pre-existing library. As it stands there would be nothing stopping an end-user from using the Boost library as opposed to my foundation classes but I'm trying to design them all carefully enough that they're useful and fully-functional.

Quote:
Original post by Sharlin
The second way, utilizing RAII, is certainly the preferred and idiomatic approach. In fact, the first one has a serious problem: it is not exception-safe. If an exception is thrown while m_mutex is locked, it is never released without ugly try/catch/rethrow wrappings.

I figured as much and didn't really like the look of the first approach at all. The second is more appealing programatically and is thread safe in and of itself.

Thanks to both of you for the feedback...good to know I'm on the right path :).

Share this post


Link to post
Share on other sites

Create an account or sign in to comment

You need to be a member in order to leave a comment

Create an account

Sign up for a new account in our community. It's easy!

Register a new account

Sign in

Already have an account? Sign in here.

Sign In Now

Sign in to follow this