Skip to content
  • Categories
  • Recent
  • Tags
  • Popular
  • Users
  • Groups
  • Search
  • Get Qt Extensions
  • Unsolved
Collapse
Brand Logo
  1. Home
  2. Qt Development
  3. General and Desktop
  4. Restarting multiple QThreads results in a crash
Forum Updated to NodeBB v4.3 + New Features

Restarting multiple QThreads results in a crash

Scheduled Pinned Locked Moved Unsolved General and Desktop
18 Posts 5 Posters 1.5k Views 2 Watching
  • Oldest to Newest
  • Newest to Oldest
  • Most Votes
Reply
  • Reply as topic
Log in to reply
This topic has been deleted. Only users with topic management privileges can see it.
  • P Plastic.Jesus

    Hi Everyone:

    I've encountered a strange crash issue. We have some worker objects, each with it's own thread (HAS-A). Periodically, we are encountering a crash when the objects thread is restarted.

    The call stack at the point of crash is in the property retrieval for objectName() of the worker object.

    Point of crash
    2e309b1e-530c-4191-b3a5-aaa422df6548-image.png

    Callstack of crash
    3bf048db-0395-4cad-ae46-afd7287f6af2-image.png

    The crash in our use case is sporadic but reliable. It is also very annoying as it is in a customer facing embedded application.

    I boiled it down to a very small program which reliably causes the identical crash in a few iterations.

    The worker object looks like this:

    #define SIMPLEWORKERTHREAD_H
    #include <QObject>
    #include <QThread>
    
    class SimpleWorkerThread : public QObject
    {
        Q_OBJECT
    public:
        SimpleWorkerThread(const QString& name);
    
        void start();
    
    private:
        QThread _thread;
    
    signals:
        void finished();
    
    private slots:
        void onThreadStarted();
        void onThreadFinished();
    };
    
    #endif // SIMPLEWORKERTHREAD_H
    
    #include <QCoreApplication>
    #include <QDateTime>
    #include <QDebug>
    #include <QTimer>
    
    SimpleWorkerThread::SimpleWorkerThread(const QString& name) :
        QObject()
    {
        SimpleWorkerThread::setObjectName(name);
        connect(&_thread, &QThread::started, this, &SimpleWorkerThread::onThreadStarted);
        connect(&_thread, &QThread::finished, this, &SimpleWorkerThread::onThreadFinished);
        moveToThread(&_thread);
    }
    
    void SimpleWorkerThread::start()
    {
        _thread.start();
    }
    
    void SimpleWorkerThread::onThreadStarted()
    {
        qDebug() << QString("%1 %2 Started").arg(QDateTime::currentDateTimeUtc().toString("yyyy-MM-ddThh:mm:ss.zzzZ")).arg(objectName());
    
        // I tried introducing a delay, but it fails exactly the same way
        static const int delay = 1000;
        if(delay > 0) {
            QTimer::singleShot(delay, &_thread, &QThread::quit);
        }
        else {
            _thread.quit();
        }
    }
    
    void SimpleWorkerThread::onThreadFinished()
    {
        qDebug() << QString("%1 %2 Finished").arg(QDateTime::currentDateTimeUtc().toString("yyyy-MM-ddThh:mm:ss.zzzZ")).arg(objectName());
        emit finished();
    }
    

    It starts, then calls QThread::quit either through a timer, or immediately, failing identically in either configuration.

    I then have a class responsible for starting and stopping a pool of these worker threads. It starts all the workers, and waits for the final one to join before starting the whole pool again. The code for the pool object is below.

    #ifndef THREADPOOL_H
    #define THREADPOOL_H
    #include <QObject>
    
    class SimpleWorkerThread;
    class ThreadPool : public QObject
    {
        Q_OBJECT
    public:
        ThreadPool(int testCount, int threadCount);
    
        void start();
    
    private:
        void die(const QString& message);
    
        int _testCount;
        int _finishedCount;
    
        QList<SimpleWorkerThread*> _threads;
    
    private slots:
        void startTestCycle();
        void onChildFinished();
    };
    
    #endif // THREADPOOL_H
    
    #include "threadpool.h"
    #include "simpleworkerthread.h"
    
    #include <QCoreApplication>
    #include <QDebug>
    #include <QTimer>
    
    ThreadPool::ThreadPool(int testCount, int threadCount) :
        QObject(),
        _testCount(testCount)
    {
        qDebug() << QString("Will perform %1 iterations with %2 threads").arg(testCount).arg(threadCount);
        for(int i = 0;i < threadCount;i++) {
            SimpleWorkerThread* thread = new SimpleWorkerThread(QString("Thread %1").arg(i + 1));
            connect(thread, &SimpleWorkerThread::finished, this, &ThreadPool::onChildFinished);
            _threads.append(thread);
        }
    }
    
    void ThreadPool::start()
    {
        QTimer::singleShot(0, this, &ThreadPool::startTestCycle);
    }
    
    void ThreadPool::die(const QString& message)
    {
        qDebug() << "FATAL: " << message;
        QCoreApplication::quit();
    }
    
    
    void ThreadPool::startTestCycle()
    {
        qDebug() << "********** Starting next cycle " << _testCount << " left to go";
    
        _finishedCount = 0;
    
        // Start all the threads
        for(SimpleWorkerThread* thread : _threads) {
            if(thread->thread()->isRunning()) {
                // Sanity check.... this never happens
                die("Thread was still running");
            }
            thread->start();
        }
    }
    
    void ThreadPool::onChildFinished()
    {
        SimpleWorkerThread* thread = static_cast<SimpleWorkerThread*>(sender());
        if(thread->thread()->wait(1000) == false) {
            // Sanity check.... this never happens
            die("Never joined");
        }
    
        _finishedCount++;
        if(_finishedCount >= _threads.count()) {
            if(--_testCount <= 0) {
                qDebug() << "All cycles complete successfully";
                QCoreApplication::quit();
            }
            else {
                QTimer::singleShot(0, this, &ThreadPool::startTestCycle);
            }
        }
    }
    

    main.cpp looks like this:

    #include "threadpool.h"
    
    #include <QCoreApplication>
    #include <QDebug>
    
    int main(int argc, char *argv[])
    {
        QCoreApplication a(argc, argv);
    
        ThreadPool test(500, 10);
        test.start();
        a.exec();
        return 0;
    }
    

    It appears that there may be something in the Qt property mechanism which is not thread-safe, but this seems unlikely.

    Our actual application is nowhere near as busy as the provided test code, but it reliably fails in the same way minutes or hours later.

    I've reproduced the problem on Qt 6.2.3, 6.5.3 and 6.7.1.

    The code is on my github at https://github.com/StevePunak/thread-restart-failure.git

    I would really some other eyeballs on this. Thanks in advance.

    Ronel_qtmasterR Offline
    Ronel_qtmasterR Offline
    Ronel_qtmaster
    wrote on last edited by
    #7

    @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.
    Christian EhrlicherC 1 Reply Last reply
    0
    • Ronel_qtmasterR Ronel_qtmaster

      @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.
      Christian EhrlicherC Offline
      Christian EhrlicherC Offline
      Christian Ehrlicher
      Lifetime Qt Champion
      wrote on last edited by
      #8

      @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

      Ronel_qtmasterR 1 Reply Last reply
      2
      • jeremy_kJ Offline
        jeremy_kJ Offline
        jeremy_k
        wrote on last edited by
        #9

        What's the goal of QThread reuse, other than avoiding repeat assignments of QObject thread affinity?

        Asking a question about code? http://eel.is/iso-c++/testcase/

        Christian EhrlicherC 1 Reply Last reply
        1
        • jeremy_kJ jeremy_k

          What's the goal of QThread reuse, other than avoiding repeat assignments of QObject thread affinity?

          Christian EhrlicherC Offline
          Christian EhrlicherC Offline
          Christian Ehrlicher
          Lifetime Qt Champion
          wrote on last edited by Christian Ehrlicher
          #10

          @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

          jeremy_kJ 1 Reply Last reply
          2
          • Christian EhrlicherC Christian Ehrlicher

            @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.

            Ronel_qtmasterR Offline
            Ronel_qtmasterR Offline
            Ronel_qtmaster
            wrote on last edited by
            #11

            @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.thread error.PNG
            Even the problem about memory you can see it above.And it creates crashes as well.

            Christian EhrlicherC 1 Reply Last reply
            0
            • Ronel_qtmasterR Ronel_qtmaster

              @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.thread error.PNG
              Even the problem about memory you can see it above.And it creates crashes as well.

              Christian EhrlicherC Offline
              Christian EhrlicherC Offline
              Christian Ehrlicher
              Lifetime Qt Champion
              wrote on last edited by Christian Ehrlicher
              #12

              @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_qtmasterR 1 Reply Last reply
              1
              • Christian EhrlicherC Christian Ehrlicher

                @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 ...

                Ronel_qtmasterR Offline
                Ronel_qtmasterR Offline
                Ronel_qtmaster
                wrote on last edited by
                #13

                @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 EhrlicherC 1 Reply Last reply
                0
                • Ronel_qtmasterR Ronel_qtmaster

                  @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 EhrlicherC Offline
                  Christian EhrlicherC Offline
                  Christian Ehrlicher
                  Lifetime Qt Champion
                  wrote on last edited by Christian Ehrlicher
                  #14

                  @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_qtmasterR 1 Reply Last reply
                  2
                  • Christian EhrlicherC Christian Ehrlicher

                    @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.

                    Ronel_qtmasterR Offline
                    Ronel_qtmasterR Offline
                    Ronel_qtmaster
                    wrote on last edited by
                    #15

                    @Christian-Ehrlicher you earlier said that the information i wrote were false.Now you are saying you don't understand.

                    1 Reply Last reply
                    0
                    • Christian EhrlicherC Offline
                      Christian EhrlicherC Offline
                      Christian Ehrlicher
                      Lifetime Qt Champion
                      wrote on last edited by
                      #16

                      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.

                      Qt Online Installer direct download: https://download.qt.io/official_releases/online_installers/
                      Visit the Qt Academy at https://academy.qt.io/catalog

                      1 Reply Last reply
                      1
                      • Christian EhrlicherC Christian Ehrlicher

                        @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.

                        jeremy_kJ Offline
                        jeremy_kJ Offline
                        jeremy_k
                        wrote on last edited by
                        #17

                        @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.

                        Asking a question about code? http://eel.is/iso-c++/testcase/

                        1 Reply Last reply
                        0
                        • Christian EhrlicherC Christian Ehrlicher

                          @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.

                          P Offline
                          P Offline
                          Plastic.Jesus
                          wrote on last edited by
                          #18

                          @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.

                          1 Reply Last reply
                          0

                          • Login

                          • Login or register to search.
                          • First post
                            Last post
                          0
                          • Categories
                          • Recent
                          • Tags
                          • Popular
                          • Users
                          • Groups
                          • Search
                          • Get Qt Extensions
                          • Unsolved