Restarting multiple QThreads results in a crash
-
@Plastic-Jesus I will just give some hints
- When you call QThread::start() it calls the function run() of QThread class.so you should reimplement the function run() not start()
- you should always consider deleting the thread when it has finished
- When you run many threads on the same time, it can results in unsufficient memory to execute them.Therefore also think about that aspect.
@Ronel_qtmaster said in Restarting multiple QThreads results in a crash:
When you call QThread::start() it calls the function run() of QThread class.so you should reimplement the function run() not start()
Please look at the code - QThread is not reimplemented anywhere so no need to override run()
you should always consider deleting the thread when it has finished
Why? You can reuse them, that's also the point of QThreadPool
When you run many threads on the same time, it can results in unsufficient memory to execute them.Therefore also think about that aspect.
It's a reproducer of the problem - no need to add additional error handling here for such kind of errors.
Qt Online Installer direct download: https://download.qt.io/official_releases/online_installers/
Visit the Qt Academy at https://academy.qt.io/catalog -
What's the goal of QThread reuse, other than avoiding repeat assignments of QObject thread affinity?
@jeremy_k Creating a new QThread/std::thread can be time consuming - therefore QThreadPool. But yes in general I would at least not create such stuff by myself and simply use the classes/toools provided.
The problem here is also not QThread but a race condition underneath.Qt Online Installer direct download: https://download.qt.io/official_releases/online_installers/
Visit the Qt Academy at https://academy.qt.io/catalog -
@Ronel_qtmaster said in Restarting multiple QThreads results in a crash:
When you call QThread::start() it calls the function run() of QThread class.so you should reimplement the function run() not start()
Please look at the code - QThread is not reimplemented anywhere so no need to override run()
you should always consider deleting the thread when it has finished
Why? You can reuse them, that's also the point of QThreadPool
When you run many threads on the same time, it can results in unsufficient memory to execute them.Therefore also think about that aspect.
It's a reproducer of the problem - no need to add additional error handling here for such kind of errors.
@Christian-Ehrlicher I was just giving some hints about threads in general.Also, even if you want to reuse a thread the proper way is the delete it first.And even threadpool that you mentioned automatically deletes runnable objects after the execution before you can restart the threadpool.
Please do not tell something is wrong without checking it.
Even the problem about memory you can see it above.And it creates crashes as well. -
@Christian-Ehrlicher I was just giving some hints about threads in general.Also, even if you want to reuse a thread the proper way is the delete it first.And even threadpool that you mentioned automatically deletes runnable objects after the execution before you can restart the threadpool.
Please do not tell something is wrong without checking it.
Even the problem about memory you can see it above.And it creates crashes as well.@Ronel_qtmaster said in Restarting multiple QThreads results in a crash:
even if you want to reuse a thread the proper way is the delete it first.
That's not a re-use... you delete one and create a new one with all the overhead involved. And that's not how QThreadPool is working.
Please do not tell something is wrong without checking it.
You do not understand what a testcase/reproducer is for ...
Qt Online Installer direct download: https://download.qt.io/official_releases/online_installers/
Visit the Qt Academy at https://academy.qt.io/catalog -
@Ronel_qtmaster said in Restarting multiple QThreads results in a crash:
even if you want to reuse a thread the proper way is the delete it first.
That's not a re-use... you delete one and create a new one with all the overhead involved. And that's not how QThreadPool is working.
Please do not tell something is wrong without checking it.
You do not understand what a testcase/reproducer is for ...
@Christian-Ehrlicher by reuse i mean a new thread object doing the same process.Even you asumed that we do not need to delete a thread after its use which is wrong.Even QThreadpool is doing that.I have shown you the proof that the use of many threads can create sometimes memory problems, therefore a crash.
-
@Christian-Ehrlicher by reuse i mean a new thread object doing the same process.Even you asumed that we do not need to delete a thread after its use which is wrong.Even QThreadpool is doing that.I have shown you the proof that the use of many threads can create sometimes memory problems, therefore a crash.
@Ronel_qtmaster said in Restarting multiple QThreads results in a crash:
have shown you the proof that the use of many threads can create sometimes memory problems, therefore a crash.
I don't know what you're trying to explain here - the crash the OP reports has nothing to do with too many threads. He is only creating ten and is reusing them. So what are you trying to tell us? That you can't create an indefinitly amount of objects without getting an oom situation? Thx I'm aware if this since 30 years of programming.
Qt Online Installer direct download: https://download.qt.io/official_releases/online_installers/
Visit the Qt Academy at https://academy.qt.io/catalog -
@Ronel_qtmaster said in Restarting multiple QThreads results in a crash:
have shown you the proof that the use of many threads can create sometimes memory problems, therefore a crash.
I don't know what you're trying to explain here - the crash the OP reports has nothing to do with too many threads. He is only creating ten and is reusing them. So what are you trying to tell us? That you can't create an indefinitly amount of objects without getting an oom situation? Thx I'm aware if this since 30 years of programming.
@Christian-Ehrlicher you earlier said that the information i wrote were false.Now you are saying you don't understand.
-
I just don't understand what your posts help to resolve the issue. The Maintainer of the Qt thread stuff already said it has nothing to do with the threads but it's a locking problem within QProperty handling. The testcase just reveals it in a very good and fast way.
-
@jeremy_k Creating a new QThread/std::thread can be time consuming - therefore QThreadPool. But yes in general I would at least not create such stuff by myself and simply use the classes/toools provided.
The problem here is also not QThread but a race condition underneath.@Christian-Ehrlicher said in Restarting multiple QThreads results in a crash:
@jeremy_k Creating a new QThread/std::thread can be time consuming
That's an interesting comparison. std::thread starts executing when it is instantiated. It doesn't have the ability to separate the equivalent of QThread::QThread() from QThread::start().
At a glance, there's a significant amount of code in the platform-specific implementations of QThread::start(), leading me to wonder how much avoiding additional calls to QThread::QThread saves. The underlying platform thread is (re)created during each successful call to QThread::start() in the Windows and unix implementations.
-
@Plastic-Jesus said in Restarting multiple QThreads results in a crash:
Just to make sure that my understanding of QThread restartability is correct
It is correct. As Thiago told you you should rework your stuff to avoid the crash. Maybe use QThreadPool instead your own solution.
@Christian-Ehrlicher Just a quick note. I agree with everything you said. Regarding QThreadPool, the actual software does not use a thread pool like my demo testcase. It simply has threads that fire up periodically to perform some maintenance operations. I created the thread pool in the test code as the most succinct demonstration of the issue.
Thanks again.
