3

Problem

So I have a CommandRetriever class that holds some commands, and should execute these commands on different threads.

class CommandRetriever
{
    public:
        CommandRetriever();
        ~CommandRetriever();
    
        void addCommand( QString, Command* );
        void executeCommands();

    private:
        QMap<QString, Command*> m_commands;
};

addCommand will add a new entry into m_commands. Here is the function in question, however.

void CommandRetriever::executeCommands()
{
    for( auto iter : m_commands.keys() )
    {
        Command *cmd = m_commands.value( iter ); 
         
        // Command, password //
        RemoteConnection *rcon = new RemoteConnection( cmd, "" );

        QThread *thread = new QThread();
        rcon->moveToThread( thread );
        QObject::connect( thread, SIGNAL( started() ), rcon, SLOT( doAsync() ) );
        QObject::connect( rcon, SIGNAL( finished() ), thread, SLOT( quit() ) );
        QObject::connect( rcon, SIGNAL( finished() ), rcon, SLOT( deleteLater() ) );
        QObject::connect( thread, SIGNAL( finished() ), thread, SLOT( deleteLater() ) );
        thread->start();
    }
}

RemoteConnection is my worker thread. The doAsync() slot is what processes what needs to be processed.

For some reason, my doAsync() function for my rcon object will be called, but the thread will never exit... my main class looks like this:

int main( int argc, char *argv[] )
{           
    QCoreApplication app( argc, argv );     
    CommandRetriever cr( addr, port );

    //cr.addCommand() ...

    cr.executeCommands();

    return app.exec();
}

My worker objects (rcon) will output what they're supposed to. However, even though my worker objects emit a finished() signal, the thread still lives, and my application never finishes. I connected a signal like this before:

QObject::connect( rcon, SIGNAL( finished() ), thread, SLOT( deleteLater() ) );

But then I got the error: QThread: Destroyed while thread is still running and a seg fault.

I recently posted a question that had a related issue to my threads, and the solution was to call thread->wait(), however, I saw that if I have an event loop, thread->wait() is not needed, and is not an appropriate solution. With the current code, I get no error, but my application runs forever. I'm guessing the app.exec() event loop is infinitely running, and if I'm right, how can I shut everything down correctly?


Solution

As thuga said:

You need to tell your QCoreApplication to quit, otherwise it will never return from the exec function.

This could easily be achieved by connecting a my worker object's finished() signal to make my app quit:

QObject::connect( rcon, SIGNAL( finished() ), app, SLOT( quit() ) );

However, this makes more sense if I had a window or some other GUI elements so that I would be able to definitively know when I need to shut down my program (e.g. the user closes the main window).

Because I was running multiple threads and because my application is only a console application, it didn't make sense to connect my thread's finished() signal to my QCoreApplication's quit() slot. So what I ended up using was QThreadPool:

void CommandRetriever::executeCommands()
{
     // Quick iterate through the command map //
    for( auto iter : m_commands.keys() )
    {     
        Command *cmd = m_commands.value( iter ); 

         // Command, password; start a new thread for a remote connection. //
        RemoteConnection *rcon = new RemoteConnection( cmd, "" );
        QThreadPool::globalInstance()->start( rcon );
    }
}

It should be noted that the class RemoteConnection extends QRunnable.

To wait for the threads to finish, I call:

QThreadPool::globalInstance()->waitForDone();

This will block the main thread until all of the threads finish execution, which makes more sense for my console application. It also makes the code a hell of a lot cleaner.

I also removed app.exec() in my main.cpp because I no longer needed the event loop that QCoreApplication provides.

What I learned? QThread isn't the only way to achieve multithreading. In my case, it makes more sense to use QThreadPool because I didn't need the signals/slots of QThread. Also, you shouldn't use the QCoreApplication event loop if it is not needed. Most console applications will fall into this category.


Sources

http://doc.qt.io/qt-5/qrunnable.html

http://doc.qt.io/qt-4.8/qthreadpool.html

Community
  • 1
  • 1
Kenny Worden
  • 4,335
  • 11
  • 35
  • 62
  • does your `RemoteConnection` object finish the job / emit `finished()`? in case of your error message, store the pointers to the thread objects and call `QThread::wait()` in destructor to ensure – Zaiborg Apr 15 '15 at 07:14
  • How do you know that the thread still lives? You need to tell your `QCoreApplication` to quit, otherwise it will never return from the `exec` function. – thuga Apr 15 '15 at 07:27
  • @thuga In this case, how would I tell my application to quit? – Kenny Worden Apr 15 '15 at 14:49
  • You can connect a signal to the [`QCoreApplication::quit()`](http://doc.qt.io/qt-5/qcoreapplication.html#quit) slot. – thuga Apr 16 '15 at 06:24
  • There is a good explanation of different threading technologies in the [docs here](http://doc.qt.io/qt-5/threads-technologies.html#choosing-an-appropriate-approach). – thuga Apr 16 '15 at 13:51

3 Answers3

3

If you don't tell your QCoreApplication object to quit, it will never leave the exec() function.

You can either call QCoreApplication::quit() slot directly, or connect a signal to it.

...
connect(thread, SIGNAL(finished()), qApp, SLOT(quit()));

qApp is a global pointer referring to the unique application object. It's same as calling QCoreApplication::instance().

thuga
  • 12,601
  • 42
  • 52
  • This is correct, although it makes more sense for programs for a UI. I instead switched from `QThread` to `QThreadPool` and made my RemoteConnection extend `QRunnable`. – Kenny Worden Apr 16 '15 at 11:42
  • Because this answer is correct, I accepted it- although a more comprehensive solution is in the OP. – Kenny Worden Apr 16 '15 at 13:39
1

Try connecting the finished() signal of the RemoteConnection to the terminate() slot of the QThread instead of the quit() slot.

QObject::connect( rcon, SIGNAL( finished() ), thread, SLOT( terminate() ) );

The thread may or may not be terminated immediately, depending on the operating system's scheduling policies. Use QThread::wait() after terminate(), to be sure.

Nithish
  • 1,580
  • 12
  • 21
0

Just make a new QCoreApplication object at the initializer of your dll class "CommandRetriever", then call: processEvents, once. as follow:

//Create the QCoreApplication object
int argc = 1;
char * argv[] = {"my.dll", NULL};
dllEventHandler = new QCoreApplication(argc, argv);
//Start: process events
dllEventHandler->processEvents();

That is all. Your don't need main function or QCoreApplication .exec().

Tarek.Mh
  • 638
  • 10
  • 8