Right way to close a QThread?

Josz

Hello comunity,
I have an object that starts a thread in a long loop; When I close the main Window, main window close, but i get an error Window: "The program don't work anymore... " and the thread continues working until the end.

Is there a way to close the thread befor the main window? I tried with a connection, but did not fit.

Any help will be wellcommed.
Thanks in advance

Here is my code

#include "mainwindow.h"
#include <QApplication>
#include <iostream>
#include <QObject>
#include <QThread>
#include <QDebug>

extern MainWindow *w;                           //this is here only for remove a warning
MainWindow *w;

class CommunicationFirestarter : public QThread
{
    Q_OBJECT
public slots:
    void firestarter(){
        start();
    }

private:
    void run(){
        for(int i = 0; i < 2000000; i++)
        {
             std::cout << "helloworld parallel " << i << std::endl;
        }
    }

};

/******************************************************************************************************************************/
int main(int argc, char *argv[])
{
    QApplication a(argc, argv);    
    w = new MainWindow;
    CommunicationFirestarter *cFireStarter = new CommunicationFirestarter;
    QObject::connect(w, &MainWindow::ipValidatedClicked, cFireStarter, &CommunicationFirestarter::firestarter);
                                  //The connection next is true, but the error persists, don't close the tread
    qDebug() << QObject::connect(w, &MainWindow::destroyed, cFireStarter, &CommunicationFirestarter::quit); 
    w->show();

    QApplication::exec();
    std::cout << "We wish you a nice day!" << std::endl;
    delete cFireStarter;
    delete w;
    return 0;
}

#include "main.moc"                         //don't move from here. Is the right place for main.moc

jsulm

@Josz You should first notify the thread that you want it to stop (use a shared variable for example which is set to true and which you check inside the loop in the thread) and then call http://doc.qt.io/qt-5/qthread.html#wait in your main thread.

void run(){
        while (!terminate)
        {
             std::cout << "helloworld parallel " << i << std::endl;
        }
    }

QApplication::exec();
terminate = true;
cFireStarter.wait();

Josz

@jsulm Thank you very much, that will work in the example that I showed; But that loop was only to simplify the code.

I actually have some communication routines that get stuck and the loop method is not valid :-(

Is there oher method?

jsulm

@Josz said in Right way to close a QThread?:

Is there oher method?

If your thread is hanging then the only other method I know is to call terminate(), but this should be avoided (see its documentation).
Why is your thread stuck?

J.Hilk

@Josz said in Right way to close a QThread?:

@jsulm Thank you very much, that will work in the example that I showed; But that loop was only to simplify the code.

I actually have some communication routines that get stuck and the loop method is not valid :-(

Is there oher method?

When you do more complex tasks, do you call eventually call exec() to start the threads eventLoop?
Or do you "wait" by calling QThread::sleep by any chance ?

JKSH

@Josz said in Right way to close a QThread?:

class CommunicationFirestarter : public QThread
{
    Q_OBJECT
public slots:
    void firestarter(){
        start();
    }
...

IMPORTANT: your firestarter() slot will run in your main thread, not your other thread! See the documentation (http://doc.qt.io/qt-5/qthread.html#details ):

It is important to remember that a QThread instance lives in the old thread that instantiated it, not in the new thread that calls run(). This means that all of QThread's queued slots and invoked methods will execute in the old thread. Thus, a developer who wishes to invoke slots in the new thread must use the worker-object approach; new slots should not be implemented directly into a subclassed QThread.

Anyway, does your thread have an infinite loop (while(...)) or an event loop? You can only have one, not both.

Josz

@jsulm It hangs when don't find a valid ip. The communication code is from an old inherited project.

Josz

@J.Hilk said in Right way to close a QThread?:

c() to start the threads eventLoop?
Or do you "wait" by calling QThread::sleep by any chance ?

No, the old inherited project uses

#include <process.h>
/* Start new Thread */
HANDLE m_thread = (HANDLE)_beginthread(&my_server_run_thread, 0, 0);

Don't use QThreads anymore

Josz

@JKSH the example loop code, it's actually

        /* TCP transport layer initialization */
        erpc_transport_t transport = erpc_transport_tcp_init("Localhost", 12345); // MHx IP KTP"192.168.2.30", 5002

        /* MessageBufferFactory initialization */
        erpc_mbf_t message_buffer_factory = erpc_mbf_dynamic_init();

        /* eRPC client side initialization */
        erpc_client_init(transport, message_buffer_factory);

        /* eRPC server side initialization */
        erpc_server_init(transport, message_buffer_factory); // using server funktion on client side to get server funktion ( for "callback" purposes)

        /* connect generated service into server, look into erpc_App_AppService.h */
        erpc_add_service_to_server(create_ServiceOnClient_service()); // using server funktion on client side to get service ID

        /* Start new Thread */
        HANDLE m_thread = (HANDLE)_beginthread(&my_server_run_thread, 0, 0);
        (void)m_thread;
        FunctionAppToService(pCallback1_t);
        Sleep(1000);

but, that's not my part from project

JKSH

@Josz You must make all your threads stop running before main() returns.

"Stop running" means: You must call break; in the loop of every thread.

kshegunov

@Josz said in Right way to close a QThread?:

the example loop code, it's actually
[snip]

Patch that code by having a global atomic that is signaled when the thread needs to exit, and check that global in the my_server_run_thread function whenever possible. If the flag is raised call _endthread. At the end of the thread that spawns that (i.e. the thread that calls _beginthread) call WaitForSingleObject on m_thread.
And for the love of god, forget Sleep ... forever!

JKSH

@kshegunov said in Right way to close a QThread?:

Patch that code by having a global atomic that is signaled when the thread needs to exit, and check that global in the my_server_run_thread function whenever possible.

+1

If the flag is raised call _endthread.

Are you sure?

I don't know much about the Windows thread API, but it sounds like _endthread() aborts/terminates without cleaning up: https://stackoverflow.com/questions/26960199/using-endthread

I suggest breaking out of the loop, to let the function return naturally.

At the end of the thread that spawns that (i.e. the thread that calls _beginthread) call WaitForSingleObject on m_thread.

+1

And for the love of god, forget Sleep ... forever!

The code looks like an old-school infinite loop, so Sleep() is probably needed to stop the thread from maxing out the CPU.

kshegunov

@JKSH said in Right way to close a QThread?:

Are you sure?

Yep.
https://msdn.microsoft.com/en-us/library/hw264s73.aspx

The code looks like an old-school infinite loop, so Sleep() is probably needed to stop the thread from maxing out the CPU.

That's where WaitForSingleObject comes into play.

JKSH

@kshegunov said in Right way to close a QThread?:

@JKSH said in Right way to close a QThread?:

Are you sure?

Yep.
https://msdn.microsoft.com/en-us/library/hw264s73.aspx

"NOTE: _endthread and _endthreadex cause C++ destructors pending in the thread not to be called."

That doesn't sound very good.

The code looks like an old-school infinite loop, so Sleep() is probably needed to stop the thread from maxing out the CPU.

That's where WaitForSingleObject comes into play.

OK

kshegunov

@JKSH said in Right way to close a QThread?:

That doesn't sound very good.

True, didn't read that far. (the damn MS API is all over the place)
x_x
Then you're completely right, just return from the function.

PS:
Note: As this is C, you're supposed to call destructor-like functions manually.