QSocketNotifier: Socket notifiers cannot be enabled or disabled from another thread

PaulOfford

I have written a small TCP Server using QTcpServer and QTcpSocket. The server listens for incoming connections on the main thread, then spins up a new service thread once a client is connected. This all seems to work fine.

The server needs to call some functions in the context of the main thread and results need to be sent to the client via the service thread. To do this I have a Signals and Slots set up in each thread like this:

qDebug() << QThread::currentThread() << socketDescriptor << " serviceThread value: " << serviceThread;

/* Here we connect our service thread back to the main thread so that we can process GotoFrame commands */
connect(serviceThread, SIGNAL(syncroGoFrame(int)), this, SLOT(syncroGoFrame(int)), Qt::QueuedConnection);

qDebug() << QThread::currentThread() << socketDescriptor << " serviceThread value: " << serviceThread;

/* This allows us to send Event messages to the client from the main thread */
connect(this, SIGNAL(sendMsg(QByteArray)), serviceThread, SLOT(sendMsg(QByteArray)), Qt::QueuedConnection);

The qDebug output for the above is:

QThread(0x2b9960) 696  serviceThread value:  SyncroThread(0x75595f0)
QThread(0x2b9960) 696  serviceThread value:  SyncroThread(0x75595f0)

And indeed other checks show that the main thread is 0x2b9960 and the service thread is 0x75595f0.

A signal from the service thread to the slot on the main thread works fine; the main thread slot runs in the context of the main thread. However, signals from the main thread to the slot for the service thread don't work correctly. The slot always runs in the context of the main thread.

Signal code like this:

qDebug() << QThread::currentThread() << "Emit sendMsg";  // temp - remove this line later
emit sendMsg(sendBA);

and slot code like this:

/* Added this as a slot function wrapper for sendData */
qDebug() << QThread::currentThread() << "In sendMsg Slot";  // temp - remove this line later
sendData(dataOut);

results in this:

QThread(0x2b9960) Emit sendMsg
QThread(0x2b9960) In sendMsg Slot

Eventually the slot code executes a socket write and, although it does send the data back to the client, it generates the error:

QSocketNotifier: Socket notifiers cannot be enabled or disabled from another thread

Am I doing something wrong?

Environment is Windows 7 and Qt 5.5.1.

Thanks and regards...Paul

kshegunov

Hello Paul,
Slots execute in the thread the object lives in (when talking about queued connections). I suspect you've subclassed QThread and reimplemented QThread::run, am I correct? If I am, then you don't have a running event loop, so you can't use queued connections. As for the socket notifier problem, it's hard to say without any code, but as a general rule you'll want your objects to be created in the thread they're supposed to live in (sometimes you move them manually by hand with QObject::moveToThread) and interact between threads only trough signals and slots. Sockets are a bit different, since you can't get the socket object on the main thread directly with QTcpServer::nextPendingConnection and then move it, you'll have to override QTcpServer::incomingConnection and send the socket descriptor to the service thread, only there you create the QTcpSocket object and set its descriptor.
I hope this helps.

Kind regards,
Konstantin.

PaulOfford

Hi Konstantin,

Yes you are right - SyncThread looks like this:

class SyncroThread : public QThread
{
Q_OBJECT

public:
SyncroThread(qintptr ID, QObject *parent);
void run();

QByteArray cmdType;
QByteArray cmdCommand;
QByteArray cmdParam1;
QByteArray cmdParam2;
QByteArray cmdParam3;

signals:
void error(QTcpSocket::SocketError socketError);
void syncroGoFrame(int);

public slots:
void readyRead();
void disconnected();
void sendMsg(QByteArray);

private:
int parseInput(QByteArray);
void sendData(QByteArray);

QTcpSocket *tcpSocket;
qintptr socketDescriptor;
};

I then have another class called SyncroServer:

class SyncroServer : public QTcpServer
{
Q_OBJECT

public:
explicit SyncroServer(QObject *parent = 0);
void StartServer();
void syncroSend(QByteArray);

signals:
void sendMsg(QByteArray);

public slots:
void syncroGoFrame(int);

private:

protected:
	void incomingConnection(qintptr socketDescriptor);
};

The two important member functions are:

void SyncroServer::StartServer()
{
/* Here we listen for incoming connections */
/* Note that we don't need an explicit connect here as the QTcpServer class already has
   an incomingConnection signal and slot in the base class definition.  Therefore, all
   we need to do is override the slot code to get the server to do our bidding - see
   SyncroServer::incomingConnection below. */
quint16 portOffset = 0;
servicePort = syncro_port_base;

qDebug() << QThread::currentThread() << "Listen for incoming connections";

while (!this->listen(QHostAddress::Any, servicePort) && portOffset < 10)
{
	qDebug() << QThread::currentThread() << "TCP Port " << servicePort << " already in use";
	portOffset += 1;
	servicePort += 1;
}

if(portOffset >= 10)
{
	qDebug() << QThread::currentThread() << "Server failed!";
}

}

void SyncroServer::incomingConnection(qintptr socketDescriptor)
{
/* The server has detected an incoming connection from a client - this is where we handle it */
qDebug() << QThread::currentThread() << socketDescriptor << " Connecting...";

SyncroThread *serviceThread = new SyncroThread(socketDescriptor, this);	/* Here we define the service thread */

connect(serviceThread, SIGNAL(finished()), serviceThread, SLOT(deleteLater()));	/* This tidies up after the service thread has exited */

/* And here is where we actually start our service thread. */
serviceThread->start();

qDebug() << QThread::currentThread() << socketDescriptor << " serviceThread value: " << serviceThread;

/* Here we connect our service thread back to the main thread so that we can process GotoFrame commands */
connect(serviceThread, SIGNAL(syncroGoFrame(int)), this, SLOT(syncroGoFrame(int)), Qt::QueuedConnection);

qDebug() << QThread::currentThread() << socketDescriptor << " serviceThread value: " << serviceThread;

/* This allows us to send Event messages to the client from the main thread */
connect(this, SIGNAL(sendMsg(QByteArray)), serviceThread, SLOT(sendMsg(QByteArray)), Qt::QueuedConnection);

}

As you can see, I do override incomingConnection and send the socketDescriptor to the service thread.

I'll go through your points in more detail tomorrow.

Thanks and regards...Paul

kshegunov

@PaulOfford
Hello Paul,
Here is a quite succinct and clear tutorial how you can do threading with an event loop (the so-called worker object approach). This is my recommendation for implementation. It allows you to have threading with an event loop, thus allowing you to use queued signal-slot connections. If you still are intent on reimplementing QThread::run you have to manage your own event loop and call QEventLoop::processEvents from time to time to get the events processed. Still this might be troublesome, so if possible do switch to the worker object idiom. Also, note that the QThread instance really lives in your main thread (as it should be), so anything you create in its constructor is also living in the main thread (provided you've not moved it by hand to another thread). This is why if you create your socket in the SyncroThread constructor, it really won't be living in your worker thread, unless you call QObject::moveToThread. I can't see from your code (you've not provided the SyncroThread implementation) if this is the case really, but just bear that in mind.
To wrap up, the cleanest and easiest way to have threaded TCP is as follows:

Have a class that extends QTcpServer and override QTcpServer::incomingConnection (just like you've done). Emit a signal with the socket descriptor from there.
Create a QThread instance in your main thread, connect the signals accordingly.
Have a worker QObject subclass and move an instance of it to the worker thread with QObject::moveToThread.
Subscribe the worker object to the signal providing the socket descriptor (from the QTcpServer subclass), and in the corresponding slot create and initialize the socket.
Connect other QTcpSocket signals as you see fit.

I hope this helps.
Kind regards,
Konstantin.

PaulOfford

@kshegunov Great tip Konstantin - that's fixed the problem.

The use of moveToThread seems to be very contentious. When I first wrote the code I saw an example using this technique but I also read elsewhere that it's bad practice, and so I didn't go that route.

It was surprisingly easy for me to change the code to work in the way you suggested - it's only taken about an hour.

Thanks again.

Best regards...Paul

kshegunov

@PaulOfford
Hello Paul,
You're welcome.

The use of moveToThread seems to be very contentious. When I first wrote the code I saw an example using this technique but I also read elsewhere that it's bad practice, and so I didn't go that route.

I guess there are a lot of examples floating around that are either outdated or presented incoherently. Using QObject::moveToThread is perfectly fine with one notable exception. One should not move the QThread instance with it. Consider this:

// ... With subclassing QThread
class MyThread : public QThread
{
    MyThread(Qbject * parent)
        : QThread(parent)
    {
        moveToThread(this);    // < This is very, very bad, and should not be done ever!
    }
}

// ... With moving QObjects to thread
QThread * thread = new QThread();
thread->start();

QObject * someObject = new QObjectSubclass;
someObject->moveToThread(thread);    // < This is perfectly fine and is the way to have slots handled in another thread

Kind regards,
Konstantin.