0

Qt Thread is failing instantly when run, this is my header:

class Stepper1_run : public QThread
{
public:
    void system_run();
private:
    void run();
};

This is the .cpp code:

void Stepper1_run::run(){
    //do whatever
}


void Stepper1_run::system_run()
{
   Stepper1_run stepper1_run;
   stepper1_run.start();
}

And it is called when a button is clicked from the gui:

void MainWindow::on_button_run_clicked()
{
    Stepper1_run stepper1_run;
    stepper1_run.system_run();
}

Originally, the system_run() function was just a function on its own, I was informed that it needs to be a member of the class otherwise the thread will close instantly. Unfortunately the function is still instantly closing on run with the error: `QThread: Destroyed while thread is still running" and the program quits.

What is causing this to happen?

JimmyBanks
  • 4,178
  • 8
  • 45
  • 72
  • Unless you're wanting to redefine how Qt uses threads, you shouldn't inherit from QThread. Take a look at this article which tells you why you're doing it wrong: http://blog.qt.digia.com/blog/2010/06/17/youre-doing-it-wrong/ – TheDarkKnight Mar 26 '14 at 08:37

2 Answers2

2

You have declared your thread object on the stack, here:

void MainWindow::on_button_run_clicked()
{
    Stepper1_run stepper1_run;  // <- Stepper1_run is instantiated
    stepper1_run.system_run();  // <- Thread starts
}  // <- stepper1_run goes out of scope and is destroyed.

The object is being destroyed as soon as it goes out of scope, which happens immediately after you call system_run(), hence the error.

Making it a class member is one option. You can make it a pointer (or one of the Qt auto pointers); this is appropriate if you can only have one instance of this thread running at a time. To start the thread, instantiate a Stepper1_run, start it, and store the pointer. To stop the thread, signal it to stop, join on it, then delete the instance and reset the pointer. Be sure to stop the thread before destroying it when the containing class is destroyed as well (you can perform your usual stop -> join -> delete logic in the containing class destructor).

Update; example as requested in comments:

class MainWindow : ... {
    ...
public:
    ~MainWindow ();
public slots:
    void startSystem ();
    void stopSystem ();
private:
    QScopedPointer<Stepper1_run> stepper_;
};

MainWindow::~MainWindow () {
    stopSystem(); // <- proper termination of thread on exit
}

void MainWindow::startSystem () {
    stopSystem();
    stepper_.reset(new Stepper1_run());
    stepper_->system_run();
}

void MainWindow::stopSystem () {
    if (stepper_) { 
        stepper_->whateverStopsTheThread(); // <- you'd need to implement this
        stepper_->wait(); // <- i.e. join; waits for thread to terminate
        stepper_.reset(); // <- i.e. delete
    }
}

That example is one of many possibilities, and your scenario may differ. Also error handling / exception-safety has been omitted for brevity (and details of that also depend on your situation). Above example assumes for simplicity that only one thread should be running, and that starting a new one should terminate the old one.

Note, by the way, that by using slots, you can simply connect() your button's clicked() signal to startSystem(). This also provides you with a means to call startSystem() and stopSystem() from other threads and let Qt take care of the synchronization details.

Use a QSharedPointer if you intend to pass pointers to the thread around to other objects (it has a slightly different API from QScopedPointer, so read the documentation). Use a C-style pointer if you want to for some reason, it is similar to a QScopedPointer in the above situation, just remember to initialize it to NULL in the MainWindow constructor if you do.

Jason C
  • 38,729
  • 14
  • 126
  • 182
  • Could you show me what the code you have in mind looks like? The explanation all makes sense to me, OOP im quite a noob in though and I have been trying to get the pointer working properly, and haven't managed. – JimmyBanks Mar 26 '14 at 02:25
  • 1
    In your class MAinWindow add: Stepper1_run *_stepper1_run as a member. You can make your Stepper1_run::system_run() method a slot that is connect to your button too. Or you can overload start slot of QThread instead – Mayhem50 Mar 26 '14 at 15:52
  • @BenREGNIER Good advice, especially re: slots. Minor point: Identifiers with leading underscores are reserved by the C++ standard for C++ implementation use; there's a good description at http://stackoverflow.com/questions/228783/what-are-the-rules-about-using-an-underscore-in-a-c-identifier. Technically the limitation is only in the global namespace but it's usually better to be safe; you never know when you might conflict with e.g. a poorly named macro in a library header. – Jason C Mar 26 '14 at 16:41
-2

Just declare the stepper1_run on the header just like this.

class Stepper1_run : public QThread
      {
      public:
      void system_run();
      private:
     void run();
     Stepper1_run stepper1_run;
      };


    void MainWindow::on_button_run_clicked()
    {
     stepper1_run.system_run();
    }  
James Seva
  • 139
  • 1
  • 2
  • 7