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. Question about QThread object deletion
Forum Updated to NodeBB v4.3 + New Features

Question about QThread object deletion

Scheduled Pinned Locked Moved Solved General and Desktop
threadingoopc++
11 Posts 5 Posters 992 Views 3 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.
  • Kirill GusarevK Kirill Gusarev

    Hi smart guys! I have doubts about the correctness of following code:
    [mainwindow.cpp]

    MainWindow::MainWindow(QWidget *parent)
        : QMainWindow(parent)
        , ui(new Ui::MainWindow)
    {
        ui->setupUi(this);
    
        thread = new QThread(this);
        connect(this, SIGNAL(destroyed()),
                thread, SLOT(quit()));
    
        worker = new IanColemanSeleniumWorker();
        connect(this, SIGNAL(startWork(uint64_t)),
                worker, SLOT(run(uint64_t)));
        worker->moveToThread(thread);
        thread->start();
    }
    
    MainWindow::~MainWindow()
    {
        delete ui;
    }
    

    I wrote this code according to the video guide.
    I'm worried about the line "thread = new QThread(this);". Is "this" argument necessary here? Please pay attention to line "connect(this, SIGNAL(destroyed()), thread, SLOT(quit()));".
    I want the thread to exit gracefully when closing the application. Should a thread have a parent?

    Pl45m4P Offline
    Pl45m4P Offline
    Pl45m4
    wrote on last edited by Pl45m4
    #2

    @Kirill-Gusarev said in Question about QThread object deletion:

    I'm worried about the line "thread = new QThread(this);". Is "this" argument necessary here?

    It's not wrong, but not needed,

    Please pay attention to line "connect(this, SIGNAL(destroyed()), thread, SLOT(quit()));".
    I want the thread to exit gracefully when closing the application. Should a thread have a parent?

    since, when you want have a clean worker every time, you can do something like:

    MainWindow::spawnWorker(uint64_t workerArg) 
    {
       QThread *thread = new QThread();
       IanColemanSeleniumWorker *worker = new IanColemanSeleniumWorker();
       worker->moveToThread(thread);
    
       // maybe error handling
       // post error to MainWindow
       connect( worker, &IanColemanSeleniumWorker::error, this, &MainWindow::errorString);
    
       connect( this, &MainWindow::startWork, worker, &IanColemanSeleniumWorker::run);
       // emit a finished() signal in your worker, once you are done
       connect( worker, &IanColemanSeleniumWorker::finished, thread, &QThread::quit);
       connect( worker, &IanColemanSeleniumWorker::finished, worker, &IanColemanSeleniumWorker::deleteLater);
       connect( thread, &QThread::finished, thread, &QThread::deleteLater);
    
       thread->start();
       emit startWork(workerArg);
    }
    

    This will create a new thread and worker, let the worker do its job and then they are cleaned up properly without the need to being child of MainWindow / this.
    Keep in mind not to allocate new objects in the workers c'tor, because the c'tor is called when the worker is still in MainWindow's main GUI thread.

    Also using the above approach you might need to handle the destroyed signal, like you did in your code, to stop/interrupt running workers when you want to exit.


    If debugging is the process of removing software bugs, then programming must be the process of putting them in.

    ~E. W. Dijkstra

    Kirill GusarevK 3 Replies Last reply
    6
    • Pl45m4P Pl45m4

      @Kirill-Gusarev said in Question about QThread object deletion:

      I'm worried about the line "thread = new QThread(this);". Is "this" argument necessary here?

      It's not wrong, but not needed,

      Please pay attention to line "connect(this, SIGNAL(destroyed()), thread, SLOT(quit()));".
      I want the thread to exit gracefully when closing the application. Should a thread have a parent?

      since, when you want have a clean worker every time, you can do something like:

      MainWindow::spawnWorker(uint64_t workerArg) 
      {
         QThread *thread = new QThread();
         IanColemanSeleniumWorker *worker = new IanColemanSeleniumWorker();
         worker->moveToThread(thread);
      
         // maybe error handling
         // post error to MainWindow
         connect( worker, &IanColemanSeleniumWorker::error, this, &MainWindow::errorString);
      
         connect( this, &MainWindow::startWork, worker, &IanColemanSeleniumWorker::run);
         // emit a finished() signal in your worker, once you are done
         connect( worker, &IanColemanSeleniumWorker::finished, thread, &QThread::quit);
         connect( worker, &IanColemanSeleniumWorker::finished, worker, &IanColemanSeleniumWorker::deleteLater);
         connect( thread, &QThread::finished, thread, &QThread::deleteLater);
      
         thread->start();
         emit startWork(workerArg);
      }
      

      This will create a new thread and worker, let the worker do its job and then they are cleaned up properly without the need to being child of MainWindow / this.
      Keep in mind not to allocate new objects in the workers c'tor, because the c'tor is called when the worker is still in MainWindow's main GUI thread.

      Also using the above approach you might need to handle the destroyed signal, like you did in your code, to stop/interrupt running workers when you want to exit.

      Kirill GusarevK Offline
      Kirill GusarevK Offline
      Kirill Gusarev
      wrote on last edited by
      #3

      @Pl45m4 Your code is beautiful.
      Большое спасибо!

      Ъуъ

      1 Reply Last reply
      0
      • Kirill GusarevK Kirill Gusarev has marked this topic as solved on
      • Pl45m4P Pl45m4

        @Kirill-Gusarev said in Question about QThread object deletion:

        I'm worried about the line "thread = new QThread(this);". Is "this" argument necessary here?

        It's not wrong, but not needed,

        Please pay attention to line "connect(this, SIGNAL(destroyed()), thread, SLOT(quit()));".
        I want the thread to exit gracefully when closing the application. Should a thread have a parent?

        since, when you want have a clean worker every time, you can do something like:

        MainWindow::spawnWorker(uint64_t workerArg) 
        {
           QThread *thread = new QThread();
           IanColemanSeleniumWorker *worker = new IanColemanSeleniumWorker();
           worker->moveToThread(thread);
        
           // maybe error handling
           // post error to MainWindow
           connect( worker, &IanColemanSeleniumWorker::error, this, &MainWindow::errorString);
        
           connect( this, &MainWindow::startWork, worker, &IanColemanSeleniumWorker::run);
           // emit a finished() signal in your worker, once you are done
           connect( worker, &IanColemanSeleniumWorker::finished, thread, &QThread::quit);
           connect( worker, &IanColemanSeleniumWorker::finished, worker, &IanColemanSeleniumWorker::deleteLater);
           connect( thread, &QThread::finished, thread, &QThread::deleteLater);
        
           thread->start();
           emit startWork(workerArg);
        }
        

        This will create a new thread and worker, let the worker do its job and then they are cleaned up properly without the need to being child of MainWindow / this.
        Keep in mind not to allocate new objects in the workers c'tor, because the c'tor is called when the worker is still in MainWindow's main GUI thread.

        Also using the above approach you might need to handle the destroyed signal, like you did in your code, to stop/interrupt running workers when you want to exit.

        Kirill GusarevK Offline
        Kirill GusarevK Offline
        Kirill Gusarev
        wrote on last edited by
        #4

        @Pl45m4 what does "c'tor" mean?

        Ъуъ

        Pl45m4P 1 Reply Last reply
        0
        • Kirill GusarevK Kirill Gusarev

          @Pl45m4 what does "c'tor" mean?

          Pl45m4P Offline
          Pl45m4P Offline
          Pl45m4
          wrote on last edited by Pl45m4
          #5

          @Kirill-Gusarev said in Question about QThread object deletion:

          what does "c'tor" mean?

          constructor

          If your IanColemanSeleniumWorker class has private members:

          IanColemanSeleniumWorker::IanColemanSeleniumWorker(QObject *parent)
            : QObject{parent}
          {
            // DONT do something like that here
            // allocate memory and assign data AFTER the object was moved
            // ideally init your members at the beginning of your run function
            m_workerData = new SomeDataClass();
            m_intptr = new int;
          }
          

          Better:

          IanColemanSeleniumWorker::run(uint64_t int)
          {
             m_data = new SomeDataClass();
          
             // do work
             // . . .
             // . . . 
          
             emit finished(); // send finished signal when work is done
          }
          

          because of these lines

             // ...
            IanColemanSeleniumWorker *worker = new IanColemanSeleniumWorker(); // worker and all its childs in main thread
             worker->moveToThread(thread); // worker moved
             // worker belongs to "thread" Thread from now on
             // ...
          

          If debugging is the process of removing software bugs, then programming must be the process of putting them in.

          ~E. W. Dijkstra

          1 Reply Last reply
          0
          • Pl45m4P Pl45m4

            @Kirill-Gusarev said in Question about QThread object deletion:

            I'm worried about the line "thread = new QThread(this);". Is "this" argument necessary here?

            It's not wrong, but not needed,

            Please pay attention to line "connect(this, SIGNAL(destroyed()), thread, SLOT(quit()));".
            I want the thread to exit gracefully when closing the application. Should a thread have a parent?

            since, when you want have a clean worker every time, you can do something like:

            MainWindow::spawnWorker(uint64_t workerArg) 
            {
               QThread *thread = new QThread();
               IanColemanSeleniumWorker *worker = new IanColemanSeleniumWorker();
               worker->moveToThread(thread);
            
               // maybe error handling
               // post error to MainWindow
               connect( worker, &IanColemanSeleniumWorker::error, this, &MainWindow::errorString);
            
               connect( this, &MainWindow::startWork, worker, &IanColemanSeleniumWorker::run);
               // emit a finished() signal in your worker, once you are done
               connect( worker, &IanColemanSeleniumWorker::finished, thread, &QThread::quit);
               connect( worker, &IanColemanSeleniumWorker::finished, worker, &IanColemanSeleniumWorker::deleteLater);
               connect( thread, &QThread::finished, thread, &QThread::deleteLater);
            
               thread->start();
               emit startWork(workerArg);
            }
            

            This will create a new thread and worker, let the worker do its job and then they are cleaned up properly without the need to being child of MainWindow / this.
            Keep in mind not to allocate new objects in the workers c'tor, because the c'tor is called when the worker is still in MainWindow's main GUI thread.

            Also using the above approach you might need to handle the destroyed signal, like you did in your code, to stop/interrupt running workers when you want to exit.

            Kirill GusarevK Offline
            Kirill GusarevK Offline
            Kirill Gusarev
            wrote on last edited by
            #6

            @Pl45m4 said in Question about QThread object deletion:

            Keep in mind not to allocate new objects in the workers c'tor, because the c'tor is called when the worker is still in MainWindow's main GUI thread.

            What will happen if I allocate new objects in the worker's constructor?

            Ъуъ

            jsulmJ S 2 Replies Last reply
            0
            • Kirill GusarevK Kirill Gusarev

              @Pl45m4 said in Question about QThread object deletion:

              Keep in mind not to allocate new objects in the workers c'tor, because the c'tor is called when the worker is still in MainWindow's main GUI thread.

              What will happen if I allocate new objects in the worker's constructor?

              jsulmJ Offline
              jsulmJ Offline
              jsulm
              Lifetime Qt Champion
              wrote on last edited by
              #7

              @Kirill-Gusarev The explanation provided by @Pl45m4 answers your question: worker constructor is not running in the other thread, so everything allocated in the constructor is allocated in the thread where the worker is created.

              https://forum.qt.io/topic/113070/qt-code-of-conduct

              Kirill GusarevK 1 Reply Last reply
              0
              • Kirill GusarevK Kirill Gusarev

                @Pl45m4 said in Question about QThread object deletion:

                Keep in mind not to allocate new objects in the workers c'tor, because the c'tor is called when the worker is still in MainWindow's main GUI thread.

                What will happen if I allocate new objects in the worker's constructor?

                S Offline
                S Offline
                SimonSchroeder
                wrote on last edited by
                #8

                @Kirill-Gusarev said in Question about QThread object deletion:

                What will happen if I allocate new objects in the worker's constructor?

                This mostly relates to objects derived from QObject. QObject is thread affine which is why you need to use moveToThread(). As said before, the constructor is not yet run inside the new thread, but only the run() method. This rule does not necessarily apply for objects not derived from QObject, but it is certainly easier to follow the rule to not create any object inside the constructor. This also keeps all initialization in one place (i.e. the run() method).

                kshegunovK 1 Reply Last reply
                2
                • S SimonSchroeder

                  @Kirill-Gusarev said in Question about QThread object deletion:

                  What will happen if I allocate new objects in the worker's constructor?

                  This mostly relates to objects derived from QObject. QObject is thread affine which is why you need to use moveToThread(). As said before, the constructor is not yet run inside the new thread, but only the run() method. This rule does not necessarily apply for objects not derived from QObject, but it is certainly easier to follow the rule to not create any object inside the constructor. This also keeps all initialization in one place (i.e. the run() method).

                  kshegunovK Offline
                  kshegunovK Offline
                  kshegunov
                  Moderators
                  wrote on last edited by
                  #9

                  @SimonSchroeder said in Question about QThread object deletion:

                  As said before, the constructor is not yet run inside the new thread, but only the run() method.

                  Just so we are very clear - because the run method is the new thread ...

                  Read and abide by the Qt Code of Conduct

                  1 Reply Last reply
                  1
                  • jsulmJ jsulm

                    @Kirill-Gusarev The explanation provided by @Pl45m4 answers your question: worker constructor is not running in the other thread, so everything allocated in the constructor is allocated in the thread where the worker is created.

                    Kirill GusarevK Offline
                    Kirill GusarevK Offline
                    Kirill Gusarev
                    wrote on last edited by
                    #10

                    @jsulm Why is that bad?

                    Ъуъ

                    jsulmJ 1 Reply Last reply
                    0
                    • Kirill GusarevK Kirill Gusarev

                      @jsulm Why is that bad?

                      jsulmJ Offline
                      jsulmJ Offline
                      jsulm
                      Lifetime Qt Champion
                      wrote on last edited by
                      #11

                      @Kirill-Gusarev See @SimonSchroeder explanation and also read https://doc.qt.io/qt-6/threads-qobject.html

                      https://forum.qt.io/topic/113070/qt-code-of-conduct

                      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