Search code examples
qtsignals-slotsqthread

what happens if i don't use deletelater for deleting qthread?


I'm trying to use Qthread with an object. For this purpose this code is written:

QThread *thread1 = new QThread();
serialclass *obje = new serialclass();
void MainWindow::on_pushButton_baglan_clicked()
{
  obje->moveToThread(thread1);
  connect(thread1,SIGNAL(started()),obje,SLOT(baglan()), Qt::UniqueConnection);
  connect(obje,SIGNAL(finished()),thread1,SLOT(quit())); //end of the baglan function finished() signal is emitted.
  thread1->start();
}

My code works. But I used quit(), not deletelater(). The button can be pushed a lot of time. First question is, is this method true? My second question is what happens if i push the button a lot of time. Are there a lot of thread? Is every time one thread created?


Solution

  • This is fine. You should understand what you're doing and why: the code and its purpose should come from you and from such understanding.

    When you push the button a lot of times, the application might be in one of two states:

    1. The thread has already finished: obje->thread() == nullptr and you're restarting the thread - it will work fine.

    2. The thread is still running: obje->thread() == thread1 and both moveToThread and thread1->start() do nothing.

    Alas, there's no point in stopping the thread. It has an event loop that is blocked until new events arrive, it's not as if an idle QThread is using up any CPU. Starting a thread is expensive: it creates a new native thread. A finished thread ceases to exist: yes, you still have a QThread, but that's like having a handle to a closed file. QThread is a thread handle, after all.

    You should keep your members by value, not by pointer - this avoids the silly premature pessimization of extra indirection via a point. You can set up all the connections ahead of time. You don't need the on_pushButton_baglan_clicked() slot: you can connect the pushbutton directly with the SerialClass instance. You should also use a QThread-derived class that is safely destructible.

    Your code can look as follows. The compiler will generate a proper resource-deallocating destructor for you. It's the compiler's job, and it won't ever fail you in that job - whereas a human developer is very prone to failure. Thus you should leverage RAII to your full advantage, as it shifts a menial manual job onto the machine that will do it perfectly and for next to nothing :)

    // MyWindow.h
    #include <QMainWindow>
    #include <QThread>
    #include "SerialClass.h"
    #include "ui_MyWindow.h"
    
    class MyWindow : public QMainWindow {
      Q_OBJECT
      class SafeThread : public QThread {
        using QThread::run; // final
      public:
        ~SafeThread() { quit(); wait(); }
      } m_serialThread;
      SerialClass m_serial;
      Ui::MyWindow ui;
    public:
      MyWindow(QWidget * parent = nullptr);
    };
    
    // MyWindow.cpp
    #include "MyWindow.h"
    
    MyWindow::MyWindow(QWidget * parent) :
      QMainWindow{this}
    {
      ui.setupUi(this);
      connect(ui.pushButton_baglan, &QPushButton::clicked,
              &m_serial, &SerialClass::baglan);
      m_serial.moveToThread(&m_serialThread);
      m_serialThread.start();
    }
    

    If you don't want all the implementation details to live in MyWindow's header, you should use a PIMPL.