patterncppMinor
Class member method based multithreading [C++]
Viewed 0 times
methodmemberbasedmultithreadingclass
Problem
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:
So, here are the things I was wondering about:
And if I didn't notice an existing question about this, please point to it.
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;
class AThreadedClass
{
public:
AThreadedClass()
{
running = true;
value1 = 0;
value2 = 0;
value3 = 0;
}
//first method which is used for threads
void run()
{
{
std::lock_guard lck(mtx);
value1 = 0; value2 = 0; value3 = 0;
}
while (running)
{
{
std::lock_guard lck(mtx);
value1 += 2; value2 +=1; value3 -= 1;
if (value1 > 1000000)
{
running = false;
}
}
}
}
//second method used as thread
//this will possible be just a data
//retrieving function, not necessarly
//used as thread
void writeOut()
{
while(running)
{
std::lock_guard lck(mtx);
std::cout running;
};
int main()
{
AThreadedClass class1;
std::thread t1(&AThreadedClass::run, &class1);
std::thread t2(&AThreadedClass::writeOut, &class1);
t1.join();
t2.join();
std::cout << "Threads finished successfully!\n";
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::atomicwithstd::mutexlike in the example? (So far it worked perfectly for me)
And if I didn't notice an existing question about this, please point to it.
Solution
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:
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
Global Mutex
You've got
Atomic Vs Volatile
Whilst
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 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.Code Snippets
{
std::lock_guard<std::mutex> lck(mtx);
value1 = 0; value2 = 0; value3 = 0;
}Context
StackExchange Code Review Q#136232, answer score: 3
Revisions (0)
No revisions yet.