[SOLVED] Thread safe global variables
-
Hi
I needed a way to use global variables in multiple threads without coding the thread safe stuff for every variable. The performance is not an issue because they will be accessed every now and then. I came up with the code below. I wonder if there is already a better (Qt) way of doing this. Thank you.
@
class QSafeString
{
private:
QMutex mMutex;
QString mVar;public:
QSafeString(const QString& pValue = QString())
{
set(pValue);
}
void set(const QString& pValue)
{
mMutex.lock();
mVar = pValue;
mMutex.unlock();
}
QString get()
{
mMutex.lock();
return mVar;
mMutex.unlock();
}
};class QSafeInt
{
private:
QMutex mMutex;
int mVar;public:
QSafeInt(const int& pValue = 0)
{
set(pValue);
}
void set(const int& pValue)
{
mMutex.lock();
mVar = pValue;
mMutex.unlock();
}
int get()
{
mMutex.lock();
return mVar;
mMutex.unlock();
}
};@
Usage:
@// Main Thread:
QSafeString globalSafeString;
// Thread 1:
globalSafeString.set("value");
// Thread 2:
globalSafeString.set("value");
// Thread 3:
QString str = globalSafeString.get();@ -
Not directly answering your question, but there are some problems with your code.
First of all make it a template. You don't want to copy/paste it for every possible type don't you? Just make it like SafeType<int> or SafeType<QString>.
Next, this is a very nasty bug:
@
mMutex.lock();
return mVar;
mMutex.unlock();
@
You will never unlock the mutex this way. It's a deadlock. Use something like get(int& param) and assign to the param instead of returning a value.This is also bad(a lot less bad but still):
@
mMutex.lock();
mVar = pValue;
mMutex.unlock();
@
What if you have some other type than string or int and the assignment operator throws an exception? This is a deadlock again. Use QMutexLocker instead. It has a RAII semantics and will unlock the mutex when it goes out of scope.I'd also overload the operator=
-
Ough thanks I made a bug indeed, too much rush.. unlock should be before return. Below is a code that uses QMutexLocker. I would probably turn it into a template, but I wonder if Qt has some similar class that maybe I'm not aware of?
@class QSafeString
{
private:
QMutex mMutex;
QString mVar;
public:
QSafeString(const QString& pValue = QString())
{
set(pValue);
}
void set(const QString& pValue)
{
QMutexLocker locker(&mMutex);
mVar = pValue;
}
QString get()
{
QMutexLocker locker(&mMutex);
return mVar;
}
};@ -
@
QString get()
{
QMutexLocker locker(&mMutex);
return mVar;
}
@
This won't work. The mutex will unlock on exit and then the copy constructor or assignment operator of the type will be called to actually copy the value. This is a possible race condition.You need to do something like this to guard the assignment oparation:
@
void get(T& param)
{
QMutexLocker(mMutex);
param = mVar;
}
@I'm not aware of any Qt class that does it except for maybe QAtomicInt class, which does something even stronger.
-
[quote author="Krzysztof Kawa" date="1361552630"]@
QString get()
{
QMutexLocker locker(&mMutex);
return mVar;
}
@
This won't work. The mutex will unlock on exit and then the copy constructor or assignment operator of the type will be called to actually copy the value. This is a possible race condition.You need to do something like this to guard the assignment oparation:
@
void get(T& param)
{
QMutexLocker(mMutex);
param = mVar;
}
@I'm not aware of any Qt class that does it except for maybe QAtomicInt class, which does something even stronger.[/quote]
I based on the example for QMutexLocker given here: "http://qt-project.org/doc/qt-5.0/qtcore/qmutexlocker.html":http://qt-project.org/doc/qt-5.0/qtcore/qmutexlocker.html
The example shows a function that returns an int type. Is something wrong with the example?
In any case, what is the proper way of doing it in a case when we want the get function to return a variable?
-
The examples in the doc show something different. QMutexLocker is used there to guarantee correct order of functions execution. In there the returned value is not important from the locking point of view. It's a local variable, not shared between threads, so does not need to be guarded.
You use mutex to guard a variable access, so this means that any place, that the variable is written to or being read from needs to be surrounded by lock/unlock (or be in scope of mutex locker). You can't do that when you return the guarded variable by value, because the actual reading takes place "after" the return, in the copy constructor or operator= of the variable that it is being assigned to. You would have to place the 'unlock' part there, which is of course not possible elegantly.
The proper way I'd say is not to do it ;) You could do it like I showed you - via a function parameter nstead of return value, or, if you have the pleasure to work with C++11 there are r-value reference tricks, but that's another story.
-
Ok, I just realized you're not returning by reference, so you might get away with this:
@
void get(int& param)
{
QMutexLocker(mMutex);
param = mVar;
}int get()
{
int temp;
get(temp);
return temp;
}
@
or even that:
@
int get()
{
int temp;
{
QMutexLocker(mMutex);
temp = mVar;
}
return temp;
}
@
Just be careful not to do that:
@
int& get()
{
int temp;
get(temp);
return temp;
}
@ -
[quote author="Krzysztof Kawa" date="1361557626"]
@int get()
{
int temp;
{
QMutexLocker(mMutex);
temp = mVar;
}
return temp;
}@
[/quote]Are the braces required in your example above? I think the following should work as well:
@QString get()
{
QString temp;
QMutexLocker locker(&mMutex);
temp = mVar;
return temp;
}
// OR
QString get()
{
QString temp;mMutex.lock() temp = mVar; mMutex.unlock(); return temp;}
@ -
The second variant (with the lock/unlock) sure.
The first one.. hm, it might be the case but to be honest I'm just not sure and don't want to mislead you.I recently saw "this great talk on atomics, locks etc.":http://channel9.msdn.com/Shows/Going+Deep/Cpp-and-Beyond-2012-Herb-Sutter-atomic-Weapons-1-of-2 and it just brainwashed me. I see traps everywhere :P
-
[quote author="Krzysztof Kawa" date="1361560263"]The second variant (with the lock/unlock) sure.
The first one.. hm, it might be the case but to be honest I'm just not sure and don't want to mislead you.I recently saw "this great talk on atomics, locks etc.":http://channel9.msdn.com/Shows/Going+Deep/Cpp-and-Beyond-2012-Herb-Sutter-atomic-Weapons-1-of-2 and it just brainwashed me. I see traps everywhere :P[/quote]
Haha right, it's like that when it comes to threads. Thanks a lot you really helped me here.
-
Oh it will optimize the s*** out of it with things like RVO, OoOE and whatnot, but it's a good thing and a lot of people are paid a lot of money for it to stay safe and you don't need to think about it when you write threading code. The guarantee is nothing will escape the guarded region, otherwise locking/unlocking wouldn't make much sense. So don't be afraid of the compiler, just stay frosty ;)
-
"VoidRealms":http://www.youtube.com/watch?v=yazMHbIew0Q
Just look at his tutorials (previous ones i think...)
Hope it helps... -
All those QThread discussions seem to lead to just different approaches to the same old problem - how to abstract away the low level threading stuff. I mean - what we really want to do is to make an asynchronous call and do something when the result is available. We shouldn't be bothered by threads, mutexes, locks and race conditions. Multi-threaded algorithms are hard enough on their own.
That's what the industry has slowly started to realize lately and that's why there's been change in threading APis approach. Examples are PPL, AMP, QtConcurrent and even the new std c++11 threading api.
In Qt it was an evolution from QThread to QThreadPool to QtConcurrent.
Personally whenever I can I try to stick to basic QtConncurrent::run or QtConcurrent::map etc and just wait for the result with QFuture<>::waitForFinished() or better yet - emit a finished signal when the work is done and react to it in a slot. This way the code becomes simpler, more semantic and really about the algorithm it performs and not about the underlying machinery. I know this is disputable, but I believe threading is done right when there are no QThreads around at all and as little threading primitives (like mutexes or atomics) as possible.
-
When you are working with a complex, parallel algorithm the "threading stuff" indeed becomes low level. Of course you can go lower and lower right down to the processor instructions. That's actually one of the key strengths of C++, but that's not my point. What I'm saying is that threading part shouldn't take 60% of the code. It should be 5% or 2% - one or two lines here and there, or a single keyword (like in some of the AMP constructs).
You might be right that we'll never get rid of the complexities entirely, but you know - somebody said that we'll never need more that 64k of RAM and look what happened :)
-
Yes, there is actually no real need for user space software developers to use fences etc. (and most of them probably never heard of them), but for kernel module or driver developers.
I think the main question is, how much do you want to have done by the compiler - should it perform most of the multi-threading synchronization things itself by analysing your code.
The next question is: is the current approach to write multi-threaded apps actually a good way to do it? Is the old "here is my imperative code, now execute it" way the right way? I don't think so. There is a huge lack in visibility of what the code that you write actually does. Prove me wrong, but neither the language(s) nor the software tools are good enough to manage writing multi-threaded applications in a way where you have a good overview of what is really going on.
The EDA tools that hardware developers use are much better, but who wants to spend 1xxxxx for good software development tools, when the whole world wants free software.
If you know great tools for multi-threaded programming, please tell me. ;)
