Mutex, please explain what's wrong
-
Hi, Qts!
Please help, I study Mutex etc. My task is to print data by turns, first from the first thread, then from the second. First QMutextLocker blocks the second thread, after QMutextLocker auto destroied theoretically the second thread must start printing. But it doesn't work as intended. What I have done wrong?#include <QCoreApplication> #include <QDebug> #include <QMutex> #include "thread1_tst4.h" #include "thread2_tst4.h" thread1_tst thread1; thread2_tst thread2; QMutex mutex; int count = 0; int main(int argc, char *argv[]) { QCoreApplication a(argc, argv); thread1.start(); thread2.start(); return a.exec(); }
#include "thread1_tst4.h" extern QMutex mutex; extern int count; thread1_tst::thread1_tst(QObject *parent) : QThread{parent} {} //------------------------------ void thread1_tst::run() { while(1) { QMutexLocker locker(&mutex); for(int i=0; i < 5; i++) qDebug() << "thread 1 out: count=" << count++ << " (" << i << ")"; msleep(1000); qDebug() << "----end 1 -----"; } } //------------------------------
#include "thread2_tst4.h" extern QMutex mutex; extern int count; thread2_tst::thread2_tst(QObject *parent) : QThread{parent} {} //------------------------------ void thread2_tst::run() { while(1) { QMutexLocker locker(&mutex); for(int i=0; i < 5; i++) qDebug() << "thread 2 out: count=" << count++ << " (" << i << ")"; msleep(1000); qDebug() << "----end 2 -----"; } } //------------------------------
-
Your example is incomplete, as it misses the header files. For future posting, I suggest to also merge everything into a single file.
Anyhow, looking at your source code
while(1) { QMutexLocker locker(&mutex); for(int i=0; i < 5; i++) qDebug() << "thread 2 out: count=" << count++ << " (" << i << ")"; msleep(1000); qDebug() << "----end 2 -----"; }
Please re-read the documentation of QMutextLocker. In particular, when it unlocks the mutex.
-
@kkoehne said in Mutex, please explain what's wrong:
Please re-read the documentation of QMutextLocker. In particular, when it unlocks the mutex.
They say "Now, the mutex will always be unlocked when the QMutexLocker object is destroyed (when the function returns since locker is an auto variable)." I thought QMutexLocker object will be destroyed every while(1) cycle. Well, I do it in a separe function and all in one file:
#include <QCoreApplication> #include <QDebug> #include <QMutex> #include <QThread> #include <QObject> #include <QDebug> #include <QMutexLocker> QMutex mutex; int count = 0; //================= class thread1_tst : public QThread { Q_OBJECT public: explicit thread1_tst(QObject *parent = nullptr); void run(); void my_work(); }; //------------------------------ thread1_tst::thread1_tst(QObject *parent) : QThread{parent} {} //------------------------------ void thread1_tst::run() { while(1) { my_work(); } } //------------------------------ void thread1_tst::my_work() { QMutexLocker locker(&mutex); for(int i=0; i < 5; i++) qDebug() << "thread 1 out: count=" << count++ << " (" << i << ")"; msleep(1000); qDebug() << "----end 1 -----"; } //================= class thread2_tst : public QThread { Q_OBJECT public: explicit thread2_tst(QObject *parent = nullptr); void run(); void my_work(); }; thread2_tst::thread2_tst(QObject *parent) : QThread{parent} {} void thread2_tst::run() { while(1) { my_work(); } } //------------------------------ void thread2_tst::my_work() { QMutexLocker locker(&mutex); for(int i=0; i < 5; i++) qDebug() << "thread 2 out: count=" << count++ << " (" << i << ")"; msleep(1000); qDebug() << "----end 2 -----"; } //============================= thread1_tst thread1; thread2_tst thread2; int main(int argc, char *argv[]) { QCoreApplication a(argc, argv); thread1.start(); thread2.start(); return a.exec(); } #include "main.moc"
Locker is destroyed every while(1) cycle, but the output is as bad as usual
thread 1 out: count= 0 ( 0 ) thread 1 out: count= 1 ( 1 ) thread 1 out: count= 2 ( 2 ) thread 1 out: count= 3 ( 3 ) thread 1 out: count= 4 ( 4 ) ----end 1 ----- thread 1 out: count= 5 ( 0 ) thread 1 out: count= 6 ( 1 ) thread 1 out: count= 7 ( 2 ) thread 1 out: count= 8 ( 3 ) thread 1 out: count= 9 ( 4 ) ----end 1 ----- thread 1 out: count= 10 ( 0 ) thread 1 out: count= 11 ( 1 ) thread 1 out: count= 12 ( 2 ) thread 1 out: count= 13 ( 3 ) thread 1 out: count= 14 ( 4 ) ----end 1 -----
-
@qAlexKo said in Mutex, please explain what's wrong:
will be destroyed every while(1) cycle
It does, but nobody guarantees you that after this the other thread can lock or the current one will lock again.
-
@qAlexKo Your mutex locker locks for the entirety of the function, sleeping included. There's only a brief window when the
while(1)
loop spins when the two threads can fight it out for the lock, and it just so happens the first one wins.
What you probably meant is something like this:void thread1_tst::my_work() { { QMutexLocker locker(&mutex); for(int i=0; i < 5; i++) qDebug() << "thread 1 out: count=" << count++ << " (" << i << ")"; qDebug() << "----end 1 -----"; } // Locker unlocks here msleep(1000); }
in this case the other thread has a lot of time to acquire the mutex while this thread sleeps.
-
@Chris-Kawa that's what I also thought - that @qAlexKo misses the mutex is still being locked when msleep(1000) is executed.
Anyhow, he does have a point in that, when the mutex from thread_1 is unlocked, thread_2 is waiting for it. So in a very 'simple' mutex implementation, that would mean that thread_2 would immediately grab the mutex, so thread_1 is blocked from re-acquiring it. This would be an implementation that is considered 'fair'.
Anyhow, QMutex doesn't make such a guarantee - that's also what @Christian-Ehrlicher wrote. Instead , what happens is more like
- thread_1 unlocks mutex. THis informs all threads waiting for this mutex to wake up.
- thread_1 continues running, grabs mutex again
- thread_2 wakes up. "Where am I? A right, I want to have that mutex. Oh, it's gone again. Whatever, I'll go back to sleep".
- repeats.
For reference, the actual locking mechanism is implemented in QBasicMutex::lockInternal(QDeadlineTimer deadlineTimer) (qmutex.cpp), at least in Qt 6.6.
-
@Chris-Kawa said in Mutex, please explain what's wrong:
What you probably meant is something like this:
void thread1_tst::my_work()Yes, it works (although in not 100% cases, sometimes one thread repeats its out) :
These is a full text (sorry last time there was a slightly damaged text for some reason)
#include <QCoreApplication> #include <QDebug> #include <QMutex> #include <QThread> #include <QObject> #include <QDebug> #include <QMutexLocker> QMutex mutex; int count = 0; //================= class thread1_tst : public QThread { Q_OBJECT public: explicit thread1_tst(QObject *parent = nullptr); void run(); void my_work(); }; //------------------------------ thread1_tst::thread1_tst(QObject *parent) : QThread{parent} {} //------------------------------ void thread1_tst::run() { while(1) { my_work(); } } //------------------------------ void thread1_tst::my_work() { { QMutexLocker locker(&mutex); for(int i=0; i < 5; i++) qDebug() << "thread 1 out: count=" << count++ << " (" << i << ")"; qDebug() << "----end 1 -----"; } msleep(1000); } //================= class thread2_tst : public QThread { Q_OBJECT public: explicit thread2_tst(QObject *parent = nullptr); void run(); void my_work(); }; thread2_tst::thread2_tst(QObject *parent) : QThread{parent} {} void thread2_tst::run() { while(1) { my_work(); } } //------------------------------ void thread2_tst::my_work() { { QMutexLocker locker(&mutex); for(int i=0; i < 5; i++) qDebug() << "thread 2 out: count=" << count++ << " (" << i << ")"; qDebug() << "----end 2 -----"; } msleep(1000); } //============================= thread1_tst thread1; thread2_tst thread2; int main(int argc, char *argv[]) { QCoreApplication a(argc, argv); thread1.start(); thread2.start(); return a.exec(); } #include "waitconditions.moc"
-
@qAlexKo said in Mutex, please explain what's wrong:
My task is to print data by turns, first from the first thread, then from the second.
If this is your precise objective I think you cannot achieve this with a single mutex. It seems to me you have 3 alternatives:
-
Use semaphores. I see this is the "accepted" solution at How to make two threads strictly alternate using mutexes?. Warning: I don't like semaphores because nasty/undefined things happen if your program crashes....
-
Use a pair of mutexs. I see that possible solution is also used in another of the answers there.
-
Use Qt (queued) signals between the two threads. I suspect this could be pretty "slow", compared to direct mutexs/semaphores.
-
-
@JonB said in Mutex, please explain what's wrong:
If this is your precise objective I think you cannot achieve this with a single mutex. It seems to me you have 3 alternatives:
Use semaphores. I see this is the "accepted" solution at How
Thanx, Semaphors are on the list of my styding. Also I have fully resolved the problem by numerating threads and blocking "last processed thread" to work twice:
#include <QCoreApplication> #include <QDebug> #include <QMutex> #include <QThread> #include <QObject> #include <QDebug> #include <QMutexLocker> QMutex mutex; int count = 0; int last_thread_num = 0; //================= class thread1_tst : public QThread { Q_OBJECT public: int _thread_num; int& _last_thread_num; explicit thread1_tst(int thead_num, int& last_thread_num, QObject *parent = nullptr); void run(); void my_work(); }; //------------------------------ thread1_tst::thread1_tst(int thread_num, int& last_thread_num, QObject *parent) : QThread{parent}, _thread_num(thread_num), _last_thread_num(last_thread_num) {} //------------------------------ void thread1_tst::run() { while(1) { if(_thread_num != _last_thread_num) my_work(); else msleep(1); } } //------------------------------ void thread1_tst::my_work() { { QMutexLocker locker(&mutex); for(int i=0; i < 5; i++) qDebug() << "thread 1 out: count=" << count++ << " (" << i << ")"; qDebug() << "----end 1 -----"; } msleep(1000); last_thread_num = _thread_num; } //================= class thread2_tst : public QThread { Q_OBJECT public: int _thread_num; int& _last_thread_num; explicit thread2_tst(int thead_num, int& busy_fl, QObject *parent = nullptr); void run(); void my_work(); }; thread2_tst::thread2_tst(int thread_num, int& last_thread_num, QObject *parent) : QThread{parent}, _thread_num(thread_num), _last_thread_num(last_thread_num) {} void thread2_tst::run() { while(1) { if(_thread_num != _last_thread_num) my_work(); else msleep(1); } } //------------------------------ void thread2_tst::my_work() { { QMutexLocker locker(&mutex); for(int i=0; i < 5; i++) qDebug() << "thread 2 out: count=" << count++ << " (" << i << ")"; qDebug() << "----end 2 -----"; } msleep(1000); last_thread_num = _thread_num; } //============================= thread1_tst thread1(0, last_thread_num); //make threads enumerated thread2_tst thread2(1, last_thread_num); int main(int argc, char *argv[]) { QCoreApplication a(argc, argv); thread1.start(); thread2.start(); return a.exec(); } #include "waitconditions.moc"