# Class member method based multithreading [C++]

In briefly: I'm working on a little chat program just for fun.

I am using threads to control two main parts of the application. I am making a socket controller class based on the following design:

//this is a "main.cpp example" for the design

std::mutex mtx;

{
public:
{
running = true;
value1 = 0;
value2 = 0;
value3 = 0;
}

//first method which is used for threads
void run()
{
{
std::lock_guard<std::mutex> lck(mtx);
value1 = 0; value2 = 0; value3 = 0;
}

while (running)
{
{
std::lock_guard<std::mutex> lck(mtx);
value1 += 2; value2 +=1; value3 -= 1;
if (value1 > 1000000)
{
running = false;
}
}
}
}

//this will possible be just a data
//retrieving function, not necessarly
void writeOut()
{
while(running)
{
std::lock_guard<std::mutex> lck(mtx);
std::cout << value1 << ';' << value2 << ';' << value3 << '\n';
}
}
private:
int value1, value2, value3;
std::atomic<bool> running;
};

int main()
{

t1.join();
t2.join();

return 0;
}


So, here are the things I was wondering about:

• Is this a good way to manage things? (I mean, using classes and their member functions for threads could be a stupid idea :/)
• Are there possible deadlocks, when I am mixing std::atomic with std::mutex like in the example? (So far it worked perfectly for me)

• Hello! Please don't make changes to the original post once it has been reviewed, as that invalidates the current answers. Please see our meta side on performing iterative reviews for more information! Jul 28 '16 at 19:30
• @syb0rg Okay, I got it. Jul 28 '16 at 19:32

• Is this a good way to manage things? (I mean, using classes and their member functions for threads could be a stupid idea :/)

No. You don't force any sequencing on the threads.

• Are there possible deadlocks, when I am mixing std::atomic with std::mutex like in the example? (So far it worked perfectly for me)

Yes. Though unlikely.

This loop can lock out the other thread from running completely (because it also tries to lock mtx).

    while(running)
{
std::lock_guard<std::mutex> lck(mtx);
std::cout << value1 << ';' << value2 << ';' << value3 << '\n';
}


Just because you release the lock does not mean waiting threads are guranteed to be the first thread that is given the lock next. It is just as likely that this thread will regain the lock again when it re-enters the loop.

You should be using condition variables to force an ordering.

• Thanks for your answer :). I've been thinking about the first (how to resolve the problem of random accesses) but I realized you're right, I should order the threads' processings. Jul 29 '16 at 15:01

I think you have an OK approach, however it's hard to tell if it's going to work for you because at the moment you seem to be at concept phase. As you develop the application, you may find that having different methods for different thread loops may mean your class is responsible for too much. I tend to prefer to have a class for each thread I create, which has a main thread function. An example of which would be the WorkerThread in this question I posted.

Nested scopes in functions

As far as your coding style goes, I'm not super keen on the way you're using braces to create scoping for your mutex uses:

{
std::lock_guard<std::mutex> lck(mtx);
value1 = 0; value2 = 0; value3 = 0;
}


I'd be inclined to create a function with a name if I felt like I needed to have the extra scoping and put the body into it. Your run method also re-initializes the values even these have been set in the constructor, it's unclear if this is on purpose or not.

Global Mutex

You've got std::mutex mtx; as a global. If you're only going to be using it from within your AThreadedClass, it would be better as a static class member. I'd also update its name to reflect what it is it's used to lock.

Atomic Vs Volatile

Whilst atomic is the safe option for running, I'm not sure you really need it to be atomic. You never update its value outside of the mtx lock. I would expect that marking it as volatile should be enough to prevent the compiler from optimizing out the checks in the while loops.

• Thank you for the answer :). (I'll read it again profoundly tomorrow, since I don't have time today.) Jul 28 '16 at 19:24
• @Lasoloz Hopefully you'll get a second opinion by that point as well ;) Jul 28 '16 at 19:25