Qt bug? Signal emitted once, slot called multiple times

[MattPhillips]

Hi,

I'm getting this really strange situation, on multiple (unix-based) platforms, whereby a signal that I emit once results in multiple calls to the same slot. The slot does time-critical inter-process communication via socket connections, so this is pretty debilitating. Here's the signal:

class MyClass
{
    Q_OBJECT
    ...
    signals:
        void WriteToProc(const QByteArray& qb, int* num_bytes = NULL);
};
 
MyClass::SendData(...)
{
    QByteArray qb;
    ... //Fill up qb;
    qDebug() << "Emitting signal";
    emit WriteToProc(qb);
}

class MyClass
{
    Q_OBJECT
    ...
    signals:
        void WriteToProc(const QByteArray& qb, int* num_bytes = NULL);
};

MyClass::SendData(...)
{
    QByteArray qb;
    ... //Fill up qb;
    qDebug() << "Emitting signal";
    emit WriteToProc(qb);
}

To copy to clipboard, switch view to plain text mode 

Here's the slot (has the same name):

class MyWorkerThread : public QThread
{
    Q_OBJECT
    ...
    public slots:
      	void WriteToProc(const QByteArray& qb, int* num_bytes);
    private:
        QLocalSocket* socket_out;
};
 
void MyWorkerThread::WriteToProc(const QByteArray& qb, int* num_bytes)
{
    if (num_bytes==NULL) socket_out->write(qb);
    else *num_bytes = socket_out->write(qb);
    qDebug() << "MyWorkerThread::WriteToProc";
}

class MyWorkerThread : public QThread
{
    Q_OBJECT
    ...
    public slots:
      	void WriteToProc(const QByteArray& qb, int* num_bytes);
    private:
        QLocalSocket* socket_out;
};

void MyWorkerThread::WriteToProc(const QByteArray& qb, int* num_bytes)
{
    if (num_bytes==NULL) socket_out->write(qb);
    else *num_bytes = socket_out->write(qb);
    qDebug() << "MyWorkerThread::WriteToProc";
}

To copy to clipboard, switch view to plain text mode 

There's some stuff in there that's not directly relevant to the point that I include for completeness. SendData() and WriteToProc are in different threads, and the connection type is default (Qt::QueuedConnection). Anyway, the output is e.g.

Emitting signal
MyWorkerThread::WriteToProc
MyWorkerThread::WriteToProc
MyWorkerThread::WriteToProc

Moreover, looking at the output at the other end, I can see that the QByteArray is getting sent 3 (in this case; it can vary) times. I've verified that only SendData() is calling WriteToProc at that point. It's so weird--what could be going on?

Best,
Matt

[Lykurg]

The interesting part is, how you establish the connection. Most probably you establish the connection not only once. Try to use Qt::UniqueConnection on (all) connect statements.

[high_flyer]

is WriteToProc () connected to other signals as well?
Do a full search in your project for the connect() calls that involve this slot.
If there is this slot is connected to another signals, figure out why this signal fires, maybe by placing a debug output as you did here.

[wysota]

You do realize that the WriteToProc() slot will be called from the context of the main thread and not the context of the worker thread, right?

[MattPhillips]

Hi,

Thanks all for getting back to me. The program--and the relevant sections in particular, isn't *that* big, maybe 4K lines, and I'm the sole author. The connect statement connecting signal and slot is made only once, and WriteToProc (signal) and WriteToProc (slot) are connected exclusively to each other. As for Qt::UniqueConnection, I can try that anyway, can it just be |'ed with the others? I guess I'll experiment and see. wysota--

You do realize that the WriteToProc() slot will be called from the context of the main thread and not the context of the worker thread, right?

No I did not! Here's my connect statement:

MyWorkerThread::run()
{
    ....
    connect(&expt, SIGNAL(WriteToProc(const QByteArray&,int*)), this, SLOT(WriteToProc(const QByteArray&,int*)));
    exec();
}

MyWorkerThread::run()
{
    ....
    connect(&expt, SIGNAL(WriteToProc(const QByteArray&,int*)), this, SLOT(WriteToProc(const QByteArray&,int*)));
    exec();
}

To copy to clipboard, switch view to plain text mode 

I also have 'moveToThread(this)' in MyWorkerThread's constructor. I thought the above produced a Qt::QueuedConnection and these executed the slot in the context of the worker thread. Is that wrong? Or did you mean, the signal is emitted in the context of the main thread? If the latter, yes, I know.

Matt

[wysota]

If you have moveToThread() in the constructor then the slot will be called in the right context. But you shouldn't move the QThread object it its own thread. What's the point of having this thread anyway? I don't know how much code you cut out from the snippet you pasted here but it seems the thread is sitting there doing nothing.

[MattPhillips]

Hi wysota,

Where should I move the QThread object? Anyway, the worker thread is for sending information to the secondary process, which runs graphics and needs to be updated every video frame. There has to be a separate thread for this so that sending this info won't get delayed by e.g. time-consuming gui events of the primary process. It's just a standard i/o worker thread.

Matt

[wysota]

Where should I move the QThread object?

You shouldn't move it anywhere.

Anyway, the worker thread is for sending information to the secondary process, which runs graphics and needs to be updated every video frame. There has to be a separate thread for this so that sending this info won't get delayed by e.g. time-consuming gui events of the primary process.

You don't need a thread for that.

It's just a standard i/o worker thread.

"Standard" according to what specifications? In Qt you don't need threads for i/o because all operations are asynchronous.

[MattPhillips]

Hi wysota,

Communication between the main and secondary processes is frequent (every video frame, at least) and has to be reliable. If a time-consuming gui operation is happening in the main process, and I didn't have a separate i/o thread, then even though the messages are getting received they are not getting processed, which would lead to a failure of my application. So I don't see how I can do this without a separate thread. As for 'standard', I just meant that this is a standard function of i/o threads; take e.g. the QThread main documentation page, where a single example of a QThread subclass is given and that example is for managing a QTcpSocket.

Anyway, I have 'solved' the problem by doing a direct function call:

thd->WriteToProc(qb);

instead of

emit WriteToProc(qb);

where thd is the instance of MyWorkerThread. Now the slot is just getting called once. I'd still like to know what's going on in the first place though.

Matt

[tbscope]

As for 'standard', I just meant that this is a standard function of i/o threads;

Understand that the QThread class by itself is NOT a different thread.

take e.g. the QThread main documentation page, where a single example of a QThread subclass is given and that example is for managing a QTcpSocket.

This is a very unfortunate example. In my opinion, it should be removed from the documentation and replaced by something more useful.

Anyway, I have 'solved' the problem by doing a direct function call:

thd->WriteToProc(qb);

instead of

emit WriteToProc(qb);

where thd is the instance of MyWorkerThread. Now the slot is just getting called once. I'd still like to know what's going on in the first place though.

Do you know what this code does? Can you explain it?
Please do not take this in the wrong way. I'm trying to help you understand why what you did is completely wrong. Even from the beginning (your first post).

What you should do is this:
1. Create your gui application like you normally do
2. Create a QObject subclass (the worker class) that will act as your worker thread. Yes, do not subclass QThread!
3. Create a QThread object and an object based on your worker class. You can make both objects members of your main window and instantiate them in the constructor.
4. Connect the signals and slots.
5. Move the object to the worker thread. Note again that the QThread object is not the new thread, it does manage a new thread though.
6. Whenever you're ready, start the thread.

Since you moved the worker object completely to a new thread, the signals and slots are called in the worker thread.

[wysota]

5. Move the object to the worker thread. Note again that the QThread object is not the new thread, it does manage a new thread though.

To help visualize it, consider that QThread object has a similar relationship to the thread it controls as QProcess object has to the process it controls.

If a time-consuming gui operation is happening in the main process, and I didn't have a separate i/o thread, then even though the messages are getting received they are not getting processed, which would lead to a failure of my application. So I don't see how I can do this without a separate thread.

A separate thread will not guarantee that.

[MattPhillips]

Hi,

Thanks a lot to both of you, especially tbscope for your instructions. I will definitely try to reorganize my code along those lines, though I probably can't get to it right away. It sounds more elegant than what I'm doing now. But as for what 'thd->WriteToProc(qb)' does, do I understand it? What level of understanding do you have in mind? thd is a pointer to a MyWorkerThread instance, '->' means 'access the member of', qb is a QByteArray, and WriteToProc is a member function of MyWorkerThread that sends qb to the auxiliary process using a QLocalSocket. 'emit WriteToProc(qb)' is essentially the same thing if the connection is a Qt:irectConnection (according to Qt documentation), but if it's a Qt::QueuedConnection then something like an event is generated which is processed within the receiver's event loop when it is ready. About Qt's implementation of signals/slots I know virtually nothing.

Understand that the QThread class by itself is NOT a different thread.

Right. I probably haven't had this clear in my mind. Though moveToThread(this) does kind of let you get away with not making that distinction--which is probably why I've seen you discourage that here and in other places wysota. Thinking about it more, it does seem like my way of doing it is conceptually mistaken or at least sub-optimal, compared to what you guys have described here. But basically on the new way, all of the methods and data members currently in my QThread subclass will go into the QObject subclass you described in 2). Googling moveToThread(this), the top post is a former QtCentre thread of mine asking about this very issue--Lol!! That's where the technique was suggested to me in the first place. NB, this guy

http://thesmithfam.org/blog/2009/09/...reading-in-qt/

does moveToThread(this), but this guy

http://labs.qt.nokia.com/2010/06/17/...oing-it-wrong/

hates it for what sounds like the same reasons.

Anyway, none of this answers my initial question but I guess after I reorganize things that won't matter.

Matt

[wysota]

Anyway, none of this answers my initial question but I guess after I reorganize things that won't matter.

You provided too little information to see where the double connection occurs.