Qt bug? Signal emitted once, slot called multiple times

[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.

[MattPhillips]

So.... I rewrote my code along the lines described by tbscope. The result is that now the object I moved to the QThread, MyObject, does not process events when the main gui thread has control. I've verified that MyObject is indeed owned by the QThread, and in addition they do non-event-processing related processing in parallel. My code in the gui window constructor is this:

QThread* thd = new QThread();
    my_object = new MyObject(); 
    my_object->moveToThread(thd);
    thd->start();

QThread* thd = new QThread();
    my_object = new MyObject(); 
    my_object->moveToThread(thd);
    thd->start();

To copy to clipboard, switch view to plain text mode 

All the signals and slots I do in the MyObject constructor, but I get the same thing when they are connected in this constructor. The order of the moveToThread, start() statements doesn't matter. I guess I haven't yet found the right way to do this, any ideas on what is? Thanks--

Edit: The events in question are readyRead() signals generated over a QLocalSocket connection. The QLocalSocket, my_local_socket, is created within MyObject(). However, even after making my_local_socket a child of the QThread, and calling my_local_socket->moveToThread(my_thread), the problem persists.

Matt

[MarekR22]

Check this article about QThread: "You’re doing it wrong…"

[wysota]

Show us how you establish your signal/slot connections.

[MattPhillips]

Sweet mother of Jesus, I figured it out. My problems were that a) I didn't at first realize that when you move a class instance to a new thread, you don't move its members, that has to be done separately, and b) you can't use moveToThread at all on a QLocalSocket. According to the warning message I got, my QLocalSocket instance had children, and you can't move an object with children. However I never created an object with my_local_socket as a parent, nor did I see such in the corresponding moc file. As a result QLocalSocket stayed in the gui thread, and I got the results I did. Here's code that actually works (knock on wood):

//include file declaring MyObject
 
class MainWindow : public QGLWidget  //The fact that I'm using GL here is not important
{
    Q_OBJECT
 
    MainWindow(QGLWidget* parent)
    ...
 
    signals:
        void ConnectSocket();
        ...
 
    public slots:
        //... various slots
        ...
 
    private:
         MyObject* my_object;
};
 
MainWindow::MainWindow
{
    QThread* thd = new QThread();
    my_object = new MyObject(this);  //main_window is not the parent of my_object, it's a receiver
    my_object->moveToThread(thd);
    cerr << "GUI thread: " << thread() << endl;
    cerr << "Object thread: " << thd << endl;
    cerr << "my_object thread: " << my_object->thread() << endl;
    thd->start();
 
    connect(this, SIGNAL(ConnectSocket()), my_object, SLOT(ConnectSocket()), Qt::QueuedConnection); 
        //I think this maneuver is the *only* way to ensure the QLocalSocket instance is in the worker thread.
    emit ConnectSocket();
}
 
 
class MyObject : public QObject
{
    Q_OBJECT
 
    public:
        MyObject(MainWindow* _gui);
 
    signals:
        ...
        //various signals sent to the MainWindow instance
 
    public slots:
        void ConnectSocket();
        void ReadyRead();
        void LostConnection();
	void SocketError(QLocalSocket::LocalSocketError err);
 
    private:
        MainWindow* gui;
        QLocalSocket* my_local_socket;
};
 
MyObject::MyObject(MainWindow* _gui) : gui(_gui)
{
}
void MyObject::ConnectSocket()
{
    bool b(true);
    my_local_socket = new QLocalSocket();
    my_local_socket->connectToServer("my_local_server", QLocalSocket::ReadOnly);
    cerr << "my_local_socket thread: " <<my_local_socket->thread() << endl;
    b &= connect(my_local_socket, SIGNAL(readyRead()), this, SLOT(ReadyRead()));
    b &= connect(my_local_socket ,SIGNAL(error(QLocalSocket::LocalSocketError)),this, SLOT(SocketError(QLocalSocket::LocalSocketError)));
 
    b &= connect(this,SIGNAL(my_object_signal()), gui, SLOT(gui_slot()), Qt::QueuedConnection);
        // ... and several more of these
 
    cerr << "Connect success, MyObject::ConnectSocket: " << b << endl;
}

//include file declaring MyObject

class MainWindow : public QGLWidget  //The fact that I'm using GL here is not important
{
    Q_OBJECT

    MainWindow(QGLWidget* parent)
    ...

    signals:
        void ConnectSocket();
        ...

    public slots:
        //... various slots
        ...
    
    private:
         MyObject* my_object;
};

MainWindow::MainWindow
{
    QThread* thd = new QThread();
    my_object = new MyObject(this);  //main_window is not the parent of my_object, it's a receiver
    my_object->moveToThread(thd);
    cerr << "GUI thread: " << thread() << endl;
    cerr << "Object thread: " << thd << endl;
    cerr << "my_object thread: " << my_object->thread() << endl;
    thd->start();

    connect(this, SIGNAL(ConnectSocket()), my_object, SLOT(ConnectSocket()), Qt::QueuedConnection); 
        //I think this maneuver is the *only* way to ensure the QLocalSocket instance is in the worker thread.
    emit ConnectSocket();
}


class MyObject : public QObject
{
    Q_OBJECT

    public:
        MyObject(MainWindow* _gui);

    signals:
        ...
        //various signals sent to the MainWindow instance

    public slots:
        void ConnectSocket();
        void ReadyRead();
        void LostConnection();
	void SocketError(QLocalSocket::LocalSocketError err);

    private:
        MainWindow* gui;
        QLocalSocket* my_local_socket;
};

MyObject::MyObject(MainWindow* _gui) : gui(_gui)
{
}
void MyObject::ConnectSocket()
{
    bool b(true);
    my_local_socket = new QLocalSocket();
    my_local_socket->connectToServer("my_local_server", QLocalSocket::ReadOnly);
    cerr << "my_local_socket thread: " <<my_local_socket->thread() << endl;
    b &= connect(my_local_socket, SIGNAL(readyRead()), this, SLOT(ReadyRead()));
    b &= connect(my_local_socket ,SIGNAL(error(QLocalSocket::LocalSocketError)),this, SLOT(SocketError(QLocalSocket::LocalSocketError)));

    b &= connect(this,SIGNAL(my_object_signal()), gui, SLOT(gui_slot()), Qt::QueuedConnection);
        // ... and several more of these

    cerr << "Connect success, MyObject::ConnectSocket: " << b << endl;
}

To copy to clipboard, switch view to plain text mode 

The cerr statements output what they should; my_object and my_local_socket are in the worker thread, not the gui thread, and the connection is a success.

Matt

[high_flyer]

a) I didn't at first realize that when you move a class instance to a new thread, you don't move its members, that has to be done separately

Where do you get this from?

@Witek - can you confirm this?

[wysota]

@Witek - can you confirm this?

Not really, it's quite the opposite. You can't move an object that has a parent in different thread. Moving an object that has no parent but it has children is fine. But they have to be children as in parent-child relationship, not as in class member relationship (in other words you have to have a "QObject" relationship between the object and its members).

[MattPhillips]

Hi,

Sorry this is so late--but ok, perhapds it's not strictly accurate to say that the members aren't moved. For example in the case of pointer members, the pointers themselves may be moved (if pointers even have a thread affinity--don't know), but the objects *pointed to*--which is generally what you care about--are not. It's analogous to a shallow copy. To illustrate, here is a version of my code with more cerr statements in it. MyObject::sock is a QLocalSocket, defined in the MyObject constructor.

MainWindow::MainWindow()
{
    ...
    QThread* thd = new QThread();
    my_object = new MyObject(this);
 
    cerr << "GUI thread: " << thread() << endl;
    cerr << "Object thread: " << thd << endl;
    cerr << "Before move: " << endl;
    cerr << "my_object thread affinity: " << my_object->thread() << endl;
    cerr << "my_object socket thread affinity: " << my_object->sock->thread() << endl;
 
    my_object->moveToThread(thd);
    thd->start();
 
    cerr << "After move: " << endl;
    cerr << "my_object thread affinity: " << my_object->thread() << endl;
    cerr << "my_object socket thread affinity: " << my_object->sock->thread() << endl;
}

MainWindow::MainWindow()
{
    ...
    QThread* thd = new QThread();
    my_object = new MyObject(this);

    cerr << "GUI thread: " << thread() << endl;
    cerr << "Object thread: " << thd << endl;
    cerr << "Before move: " << endl;
    cerr << "my_object thread affinity: " << my_object->thread() << endl;
    cerr << "my_object socket thread affinity: " << my_object->sock->thread() << endl;

    my_object->moveToThread(thd);
    thd->start();

    cerr << "After move: " << endl;
    cerr << "my_object thread affinity: " << my_object->thread() << endl;
    cerr << "my_object socket thread affinity: " << my_object->sock->thread() << endl;
}

To copy to clipboard, switch view to plain text mode 

The output is as follows:

GUI thread: 0x650f90
Object thread: 0x87b850

Before move:
my_object thread affinity: 0x650f90
my_object socket thread affinity: 0x650f90

After move:
my_object thread affinity: 0x87b850
my_object socket thread affinity: 0x650f90

So, as this example makes clear, when moveToThread is used to change the thread affinity of an object, its pointer members retain the thread affinity they had prior to the move. wysota, I wasn't referring to the parent/child relationship with a) so I don't think we're necessarily disagreeing about anything.

Best,
Matt

[wysota]

You can't prove anything without showing how you create your socket object. I still claim the whole tree of objects is moved to the new thread. If you say something about pointers then you probably don't really understand what thread affinity in Qt is.

[MattPhillips]

wysota, it was

class MyObject : public QObject
{
    Q_OBJECT
 
    public:
        MyObject(...);
 
    ...
 
        QLocalSocket* sock;
};
 
MyObject::MyObject(...)
{
    sock = new QLocalSocket();
}

class MyObject : public QObject
{
    Q_OBJECT

    public:
        MyObject(...);

    ...

        QLocalSocket* sock;
};

MyObject::MyObject(...)
{
    sock = new QLocalSocket();
}

To copy to clipboard, switch view to plain text mode 

But, after reading your comment I tried it with

sock = new QLocalSocket(this);

and indeed, like you said, sock *did* pick up the thread affinity of the parent. So it looks like the ConnectSignal workaround I came up with earlier wasn't necessary. So, thanks indeed for the insight and better solution. But it's still the case that *unless* you make a class member the child of the class instance, then member thread affinity is not changed when class affinity is, no?

Matt

[wysota]

Of course the member affinity is not changed. Why would it be? Besides, the socket object is not a member of your class. Only a pointer to a socket object is and that does not have thread affinity.