CODEWITHSUNDEEP
c++multithreadingoopmutex
ianhobo
ianhobo

Reputation: 210

Is locking a dereferenced mutex bad behaviour?

c++ pseudocode class:

Simple class which has a member variable, and mutex to control access to it.

I'm curious about the pro's and con's of managing the data and it's access. In a multithreaded enviroment, is it wrong to use the approach to accessing and locking the member mutex in cbMethodA()?

I've seen samples where the members are accessed directly, and it seems incorrect to do that. The class exposes access via a public method for a reason. Also, dereferencing a mutex to then lock it doesn't seem like best practice. Any comments? Thanks

 class A
   {
    public:
        A():val(0);
        ~A();
        int getVal(void);
        static void cbMethodA();
        static void cbMethodB();

     private:
        Mutex m_mutex;
        int val;
    }

    int A::getVal(){
    {
       int returnVal = 0;
       lockMutex(m_mutex);
       returnVal = m_val;
       unlock(mutex);
       return returnVal;
    }

 void A::cbMethodA(void *ptr)
{
    A* ptr = static_cast<A*> (ptr);
    //get val
    lockMutex(ptr->m_mutex);
    //read val
    int tempVal = ptr->m_val;
    unlockMutex(ptr->m_mutex);
    //do something with data
}

 void A::cbMethodB(void *ptr)
{
    A* ptr = static_cast<A*> (ptr);
    //get val
    int tempVal = ptr->getVal();
    //process val....
}

Upvotes: 0

Views: 118

Answers (1)

brenns10
brenns10

Reputation: 3369

This seems like a direct application of SPOT (Single Point Of Truth), a.k.a. DRY (Don't Repeat Yourself), two names for a single important idea. You've created a function for accessing val that performs some tasks that should always go along with accessing it. Unless there is some private, implementation-specific reason to access the member field directly, you should probably use the getter method you define. That way, if you change the synchronization mechanism that protects val, you only need to update one piece of code.

I can't think of any reason why "dereferencing a mutex to lock it" would be a bad thing, repeating yourself is a bad thing.

Upvotes: 2

Related Questions