Slot on main thread not called when signal is emitted from another thread

PaulOfford

I am trying to write a plugin for Wireshark, a Qt GUI application. Wireshark is mostly written in C with C++ and Qt used to handle the user interface. The plugin framework comprises mainly a set of three or four C callback functions. I'm writing the plugin in C++, exposing the callbacks using extern "C" declarations. The callbacks are being called correctly and that all looks fine.

This topic follows on from https://forum.qt.io/topic/61678/qtcpserver-newconnection-slot-not-being-called

The plugin needs to provide a TCP server but here I have simplified the code to demonstrate the problem I'm having. The code is:

syncro.h

#ifndef SYNCRO_H
#define SYNCRO_H
#include <QtCore>

#include "mythread.h"

class Syncro : public QObject
{
Q_OBJECT

public:
void Initialise(MyThread *);

public slots:
void jumpToFrame(int);
};

#endif // SYNCRO_H

syncro.cpp

#include <plugin_if.h>

#include "syncro.h"
#include <QtCore>
#include <QDebug>

void Syncro::Initialise(MyThread *serviceThread)
{
// connect(serviceThread, SIGNAL(syncroGoFrame(int)), this, SLOT(jumpToFrame(int)), Qt::DirectConnection);
connect(serviceThread, SIGNAL(syncroGoFrame(int)), this, SLOT(jumpToFrame(int)), Qt::QueuedConnection);
}

void Syncro::jumpToFrame(int new_frame)
{
plugin_if_goto_frame(new_frame);
return;
}

mythread.h

#ifndef MYTHREAD_H
#define MYTHREAD_H
#include <QtCore>

class MyThread : public QThread
{
Q_OBJECT

public:
    MyThread();
    void run();

signals:
void syncroGoFrame(int);
};

#endif // MYTHREAD_H

mythread.cpp

#include "mythread.h"
#include <QtCore>
#include <QDebug>

MyThread::MyThread()
{

}

void MyThread::run()
{
while (true)
{
	qDebug() << "Running";
	sleep(3);
	qDebug() << "Still Running";
	emit syncroGoFrame(22);
}
}

packet-syncro.cpp

static int proto_syncro = -1;
static MyThread *serviceThread;
static Syncro *mainObject;

void
proto_register_syncro(void)
{
    module_t *syncro_module;

     proto_syncro = proto_register_protocol("Syncro Service",
        "Syncro",
        "syncro");

    syncro_module = prefs_register_protocol(proto_syncro, proto_reg_handoff_syncro);

if (!serviceThread)
{
	serviceThread = new MyThread;
	mainObject = new Syncro;
	mainObject->Initialise(serviceThread);

	serviceThread->start();
}

    return;
}

The function proto_register_syncro(void) is a Wireshark callback that gets called three times at startup.

When I run the code the extra thread starts OK and it starts to cycle around the loop with the 3 second sleep. However, the connected slot function Syncro::jumpToFrame(int new_frame) does not get called. If I change the connect from QueuedConnection to DirectConnection the slot function gets called but, as expected, in the context of my serviceThread. I need the slot function to run on the main thread.

jsulm

You block the event loop of your thread, maybe this is the problem:

void MyThread::run()
{
while (true)
{
    qDebug() << "Running";
    sleep(3); // Here you block
    qDebug() << "Still Running";
    emit syncroGoFrame(22);
}
}

PaulOfford

@jsulm Hi, I may have confused everyone here. I intended to block the thread with the sleep as this is just a test routine and I didn't want it to spin in a tight loop. The problem is that when the emit syncroGoFrame(22); is called I never get a call to Syncro::jumpToFrame(int new_frame).

mbruel

Did you start the event loop in the main thread?
It should end up with an exec() and not be stuck in a loop without having reached it.
Something like this:

int main(int argc, char *argv[])
{
    QCoreApplication a(argc, argv);
     .
     .
    .

    return a.exec(); // Run the event loop of main thread
}

In your example you just have a return, not return a.exec()

dheerendra

You are doing the signal and slot across thread. Signal is sent from MyThread and Slot is main thread. Is there even main event loop started before you start your service thread ? I suspect that you may not be starting the event loop where the mainObject exist. Hence you are hitting the issue.

PaulOfford

@dheerendra and @mbruel thanks for your help. Wireshark is a QtWidget application and so as you would expect there is an event loop. The main function contains the line:

/* Create The Wireshark app */
WiresharkApplication ws_app(argc, argv);

WiresharkApplication is defined like this:

class WiresharkApplication : public QApplication
{
    Q_OBJECT
    public:
        explicit WiresharkApplication(int &argc,  char **argv);

    enum AppSignal {
        ColumnsChanged,
        FilterExpressionsChanged,
        PacketDissectionChanged,
        PreferencesChanged,
        RecentFilesRead,
        FieldsChanged
    };

    void registerUpdate(register_action_e action, const char *message);
    void emitAppSignal(AppSignal signal);
    .
    .
    const QString windowTitleString(QString title_part) { return windowTitleString(QStringList() << title_part); }

    QTranslator translator;
    QTranslator translatorQt;
    void loadLanguage(const QString& language);

private:
    bool initialized_;
    bool is_reloading_lua_;
    QFont mono_font_;
    QTimer recent_timer_;
    .
    .
    int active_captures_;

protected:
    bool event(QEvent *event);

signals:
    void appInitialized();
    void localInterfaceListChanged();
.
.
public slots:
    void clearRecentItems();
    void captureFileReadStarted();
    void captureStarted() { active_captures_++; }
    void captureFinished() { active_captures_--; }
    void updateTaps();

private slots:
    void cleanup();
    void ifChangeEventsAvailable();
    void itemStatusFinished(const QString filename = "", qint64 size = 0, bool accessible = false);
    void refreshRecentFiles(void);
    void refreshAddressResolution(void);
};

extern WiresharkApplication *wsApp;

The last line in the main function reads:

return wsApp->exec();

bsomervi

If you want to pass signals between threads then you need an event loop in both the receiving thread and the sending thread. The default implementation of QThread::run() does this for you so if you do not override QThread::run() your signal will be passed. If you do override QThread::run() then you should finish with QThread::run() or simply call exec(). I doubt you need you override QThread::run() since it seems you are implementing a TCP/IP server using QTcpServer and this will play nicely with the thread event loop. Just make sure that you have the QTcpServer running in your thread and not the main GUI thread

dheerendra

I still suspect the event loop issue at main thread. Your post says that Qt::DirectConnection works. But not Queued Connection. So implies that main thread is missing the event loop. It is possible that your thread is blocked for something else before starting the event loops ? I have sample prepared which exactly your scenario. I can you send you the same.

PaulOfford

@bsomervi I tried adding the exec() to the end of QThread::run() but that made no difference. I'm not surprised since the while (TRUE) loop means it never executes the exec(). Also, I used the Qt Mandelbrot example ( http://doc.qt.io/qt-4.8/qt-threads-mandelbrot-example.html ) as a guide and this overrides the run() function in much the same way as I have done it.

@dheerendra I think you are right that it's a main thread event loop issue. Even though I have the problem Wireshark remains responsive to the user. If the main thread event loop was blocked wouldn't the application freeze?

It might help if I understood how the slot gets included in the event loop. As the slot is connected via my plugin DLL I assume that there is some mechanism that inserts my slot into the main thread event loop at run time. How does this work?

dheerendra

in the current scenario exec().. in worker thread will not help. Here worker thread is sending the signal. So event will not in worker thread will not help.

According to you, your Main UI works fine. If the main event loop is issue UI would have been freezed. This puts other question in my mind.

How many threads are there your program ? I think mainObject is not created in main thread. It would have been created by another worker thread. That thread may not have even loop.

Just to ensure that can you print QThread::currentThreadID() in main and also in initialise method ?

Also you don't have to do anything for inserting the slot in to main thread event loop. It internally create the QEvent subclass object and post this to destination thread event loop.

PaulOfford

@dheerendra I used g_thread_self() rather than QThread::currentThreadID() as it was much easier in the main function. The details are:

GThread pointer in main - 0x02181000
GThread pointer in Syncro::Initialise - 0x02181000
GThread pointer in MyThread::run() - 0x6bf54e0

Best regards...Paul

dheerendra

It is interesting now. Is it related to your other problem where qApp is become NULL ? By any chance your UI Widgets are created in different thread rather than the main thread ?

PaulOfford

Well that was one long side track. The answer to my problem is that I had indeed mixed release and debug objects. I have been adding Qt Libraries to the Linker Input list like this:

\Qt\Qt5.5.1\5.5\msvc2013_64\lib\Qt5Core.lib
\Qt\Qt5.5.1\5.5\msvc2013_64\lib\Qt5Widgets.lib
\Qt\Qt5.5.1\5.5\msvc2013_64\lib\Qt5Gui.lib
\Qt\Qt5.5.1\5.5\msvc2013_64\lib\Qt5Network.lib

I have now changed the include list to:

\Qt\Qt5.5.1\5.5\msvc2013_64\lib\Qt5Cored.lib
\Qt\Qt5.5.1\5.5\msvc2013_64\lib\Qt5Widgetsd.lib
\Qt\Qt5.5.1\5.5\msvc2013_64\lib\Qt5Guid.lib
\Qt\Qt5.5.1\5.5\msvc2013_64\lib\Qt5Networkd.lib

and voila - all is working.

Thanks to everyone who has been advising me here.

dheerendra

cool man. I answer has indeed helped(mix of debug/release from other qn) you can make this qn solved, upvote. Have good time.