NateGreco NateGreco - 2 months ago 18
C++ Question

Do you need mutex for a class/struct if mutex is managed by members?

So if I pass a variable by reference/pointer between threads I should either implement a mutex or use std::atomic (I know there's other options). But what if I instead pass a class containing members that are std::atomic or have corresponding mutex members and wish to access the member variables?

Example:

//class.h
#include<mutex>
#include<atomic>
class MyClass {
Public:
std::atomic<int> i;
double d;
std::mutex dmutex;
MyClass();
Private:
~MyClass();
}


//main.cpp
#includ<mutex>
#include "class.h"

void ThreadFunction (MyClass &myclass) {
for (i = 0; i < 100; i++) {
myclass.i++;
myclass.dmutex.lock();
myclass.d += 0.5;
myclass.dmutex.unlock();
}
return;
}

int main () {
MyClass commonclass;
std::thread t_thread1 (ThreadFunction, commonclass);
std::thread t_thread1 (ThreadFunction, commonclass);
}


OR

//main.cpp
#includ<mutex>
#include "class.h"

void ThreadFunction (MyClass &myclass, std::mutex &mymutex) {
for (i = 0; i < 100; i++) {
mymutex.lock();
myclass.i++;
myclass.d += 0.5;
mymutex.unlock();
}
return;
}

int main () {
MyClass commonclass;
std::mutex commonmutex;
std::thread t_thread1 (ThreadFunction, commonclass, commonmutex);
std::thread t_thread2 (ThreadFunction, commonclass, commonmutex);
t_thread1.join();
t_thread2.join();
}


Lets talk in terms of only accessing member variables (my concern now) and then member functions, assuming they modifiy these member variables and handle mutexing accordingly. Is one more right than the other? Is second unecessary?

Answer

You are mixing things up here: Using stuff as a public member does not change anything. You're just taking an extra way over the this Pointer deference.

I comes down to the essence: If two threads are changing data which isn't atomic by itself, they must synchronize via, for instance, a mutex. Both cases are regarding to synchronization the same.

A class could help you by making d and its mutex private and only changing and reading it by readers and setters, which are doing the mutex handling for you. I.e. the concept of encapsulation. std::atomic does exactly this on platforms, where it's not lock free.

But you loop will be really slow, so an std::atomic would still be the best.

Short: Both cases are wrong (regarding good practices )

You class should roughly look like this

#include<mutex>
#include<atomic>

class MyClass {
public:
    std::atomic<int> i;
    MyClass() = default; //you could also just spare this line because you don't declare other constructors
    double d() {
        std::lock_guard<std::mutex> d_guard(dmutex);
        return this->d_;//this-> is not needed
    }
    void setd(double d) {
        std::lock_guard<std::mutex> d_guard(dmutex);
        d_ = d;
    }
    void add_to_d(double to_add){
        std::lock_guard<std::mutex> d_guard(dmutex);
        d_ += to_add;
    }
private:
    double d_;
    std::mutex dmutex;
};


void ThreadFunction (MyClass &myclass) {
    for (int i = 0; i < 100; i++) {
        myclass.i++;
        myclass.add_to_d(0.5);
    }
}   
Comments