CODEWITHSUNDEEP
c++classmutex
DrTarr
DrTarr

Reputation: 952

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?

Upvotes: 0

Views: 2239

Answers (2)

Superlokkus
Superlokkus

Reputation: 5059

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.

Supplemental: Better think in operations: Changing a nonatomic variable are multiple operations in machine code. std::atomics are only helping in regard that hey offer common operations which are or seem to be atomic, by for example using special machine instructions for comparinging and changing a value in one atomic machine step. But more complex operations like "change this atomic, and with that value change this other data and do this", and these operations are dependent on each other, than you have to protect that by a mutex. And the best way is to name this, now by the mutex, atomic operation and put it into a method like described below.

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);
    }
}

Upvotes: 1

LibertyPaul
LibertyPaul

Reputation: 1167

You probably shouldn't use class with a number of std:atomic<T> variables because it won't guarantee you consistence of them:

Let say you have a class like that:

class A{
    std::atomic<int> a1, a2, sum;
public:
    void update(int a1, int a2){
        this->a1 = a1;
        this->a2 = a2;
        this->sum = a1 + a2;
    }
};

There is possible race condition in void update: one thread had changed a1 and a2 and was stopped, second thread did the same and sum, first thread woke up and rewrited sum with old value.

So only way to provide consistence for complex structures - is to use mutex.

Upvotes: 0

Related Questions