QTcpServer won't receive client data on connected QTcpSocket

Christian Ehrlicher

As always when a thread is used for an async Qt class - why do you need a thread at all?
Apart from this you should use the worker thread approach and properly connect QTcpSocket::readyRead() to receive data instead blocking the event loop and the wondering why nothing happens.

dmitttri

@Christian-Ehrlicher said in QTcpServer won't receive client data on connected QTcpSocket:

Hi Christian, thanks for your answer.

As always when a thread is used for an async Qt class - why do you need a thread at all?

Simply because there will be multiple clients, requesting longer jobs to be done in parallel by the server (remote project building). This is a very common server design decision.

Apart from this you should use the worker thread approach and properly connect QTcpSocket::readyRead() to receive data instead blocking the event loop and the wondering why nothing happens.

I have tried the approach with QTcpSocket::readyRead() and the outcome was the same. Connected slot never get called.

When you say blocking the even loop, what do you exactly mean? tcpSocket.read will block until MAX_RECEIVE_BUFFER_SIZE is being received? I have tried to wait exactly number of bytes sent by the client, and it still won't receive anything. read always returns 0, and do not block, this is what I observed by testing the server side. Even if it blocks until requested number of bytes arrive, that's perfectly fine for my use case, I will just change the thread timeout approach.

My example server code is based on the Qt official example threadedfortuneserver, with the only difference that Qt example just sends some data once client connects, and do not try to receive any data.

I still don't understand your suggestions, are developers supposed to avoid QTcpSocket's read method, and using signals/slots is absolutely required?

Thanks in advance!

Christian Ehrlicher

@dmitttri said in QTcpServer won't receive client data on connected QTcpSocket:

I have tried the approach with QTcpSocket::readyRead() and the outcome was the same. Connected slot never get called.

Pleas show your code. Let the QThread run an event loop so Qt can properly receive the data from the os.

I still don't understand your suggestions, are developers supposed to avoid QTcpSocket's read method, and using signals/slots is absolutely required?

I never said you should not use read() (how else would you read data from the socket?) but you have to give Qt a chance to process the data and retrieve it from the os.

jeremy_k

@dmitttri said in QTcpServer won't receive client data on connected QTcpSocket:

@Christian-Ehrlicher said in QTcpServer won't receive client data on connected QTcpSocket:

Hi Christian, thanks for your answer.

As always when a thread is used for an async Qt class - why do you need a thread at all?

Simply because there will be multiple clients, requesting longer jobs to be done in parallel by the server (remote project building). This is a very common server design decision.

Is the build (dependency walking, compilation, linking, etc) performed in process, or is it invoking external programs?

dmitttri

@Christian-Ehrlicher said in QTcpServer won't receive client data on connected QTcpSocket:

Thanks for your patience and assistance.

Pleas show your code. Let the QThread run an event loop so Qt can properly receive the data from the os.

Yes, seems it's better to copy the complete code, most likely there will be obvious issues with managing QThread. Seems I don't get the part with running QThread in an event loop. My (still shallow) understanding of Qt event processing is that the thread execution can't directly affect the main event loop (in terms of blocking it), which should execute in the main process.

SERVER SIDE

MainServer.cpp

#include <QCoreApplication>
#include <QtCore>

#include "SimpleServer.h"


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

    SimpleServer server;
    server.start("127.0.0.1", 9000);

    return app.exec();
}

SimpleServer.h

#ifndef SIMPLESERVER_H
#define SIMPLESERVER_H

#include <QDebug>
#include <QTcpServer>

class SimpleServer : public QTcpServer
{
    Q_OBJECT;

public:
    SimpleServer(QObject *parent = nullptr);

    bool start(QString serverIP, qint16 serverPort);

protected:
    void incomingConnection(qintptr socketDescriptor) override; \
};

#endif // SIMPLESERVER_H

SimpleServer.cpp

#include <QTcpSocket>
#include <QHostAddress>

#include "SimpleServer.h"
#include "ServerJobThread.h"


SimpleServer::SimpleServer(QObject *parent)
    : QTcpServer(parent) {}


bool SimpleServer::start(QString serverIP, qint16 serverPort)
{
    qDebug() << "Awaiting for new connections "
             << "IP" << serverIP
             << "PORT" << serverPort;

    this->listen(QHostAddress(serverIP), serverPort);
    return true;
}


void SimpleServer::incomingConnection(qintptr socketDescriptor)
{
    qDebug() << "New incoming connection";

    ServerJobThread *thread = new ServerJobThread(socketDescriptor, this);
    connect(thread, SIGNAL(finished()), thread, SLOT(deleteLater()));
    thread->start();
}

ServerJobThread.h

#ifndef SERVERJOBTHREAD_H
#define SERVERJOBTHREAD_H

#include <QThread>
#include <QTcpSocket>

class ServerJobThread : public QThread
{
    Q_OBJECT

public:
    ServerJobThread(qintptr socketDescriptor, QObject *parent);

    void run() override;

private:
    qintptr socketDescriptor;
};

#endif // SERVERJOBTHREAD_H

ServerJobThread.cpp

#include <QByteArray>
#include <QTime>

#include "ServerJobThread.h"

#define THREAD_MAX_RUNNING_TIME_S 5
#define THREAD_ID this->currentThreadId()

#define MAX_RECEIVE_BUFFER_SIZE  1024

ServerJobThread::ServerJobThread(qintptr socketDescriptor, QObject *parent)
    : QThread(parent)
{
    this->socketDescriptor = socketDescriptor;
}


void ServerJobThread::run()
{
    bool workerThreadShouldRun = true;
    QTcpSocket tcpSocket;

    if (!tcpSocket.setSocketDescriptor(this->socketDescriptor))
    {
        qDebug() << THREAD_ID << "setSocketDescriptor fail";
        return;
    }

    qDebug() << THREAD_ID << "Socket info:" << tcpSocket.state() << tcpSocket.openMode();
    qDebug() << THREAD_ID << "Awaiting client message";

    QTime workerThreadTimeoutTime = QTime::currentTime().addSecs(THREAD_MAX_RUNNING_TIME_S);
    while (workerThreadShouldRun)
    {
        QByteArray buffer = tcpSocket.read(MAX_RECEIVE_BUFFER_SIZE);
        if (buffer.size() > 0)
        {
            qDebug() << THREAD_ID 
                     << "Received client message:" << QString(buffer.data())
                     << buffer.size() << "bytes";

            workerThreadShouldRun = false;
        }
        
        if (QTime::currentTime() > workerThreadTimeoutTime)
        {
            qDebug() << THREAD_ID
                     << "Thread max running time expired";
            workerThreadShouldRun = false;
        }
        this->msleep(100);
    }
    qDebug() << THREAD_ID << "Thread exit";
}

CLIENT SIDE

MainClient.cpp

#include <QThread>
#include <QDebug>
#include "SimpleClient.h"

int main(int argc, char *argv[])
{
    SimpleClient client;

    if(!client.connectToServer("127.0.0.1", 9000))
    {
        return -1;
    }

    client.pokeServer("POKE");

    QThread::sleep(2);
    client.disconnectFromServer();

    qDebug() << "Client exit";
    return 0;
}

SimpleClient.h

#ifndef SIMPLECLIENT_H
#define SIMPLECLIENT_H

#include <QTcpSocket>
#include <QAbstractSocket>

#define SOCKET_OPERATION_TIMEOUT_MS 3000

class SimpleClient
{
public:
    SimpleClient();

    bool connectToServer(QString serverIP, qint16 serverPort);
    bool isConnected();
    void disconnectFromServer();

    bool pokeServer(const char *pokeMsg);

private:
    QTcpSocket *socket = nullptr;
};

#endif // SIMPLECLIENT_H

SimpleClient.cpp

#include <QByteArray>
#include "SimpleClient.h"


SimpleClient::SimpleClient()
{
    this->socket = new QTcpSocket;
}

bool SimpleClient::connectToServer(QString serverIP, qint16 serverPort)
{
    if (!this->socket)
    {
        qDebug() << "Error, socket not initialized.";
        return false;
    }
    qDebug() << "Connecting to" << serverIP << ":" << serverPort;

    socket->connectToHost(QHostAddress(serverIP), serverPort);

    if (!socket->waitForConnected(SOCKET_OPERATION_TIMEOUT_MS))
    {
        qDebug() << "Error, connection timeout, server not accessible.";
        return false;
    }

    qDebug() << "Connected";
    return true;
}

bool SimpleClient::isConnected()
{
    return (this->socket->state() == QAbstractSocket::ConnectedState);
}

void SimpleClient::disconnectFromServer()
{
    if (this->socket)
    {
        if (this->isConnected())
        {
            this->socket->disconnectFromHost();
        }
    }
}

bool SimpleClient::pokeServer (const char *pokeMsg)
{
    if (!this->isConnected())
    {
        return false;
    }

    int bytesWritten = this->socket->write(pokeMsg);
    if (bytesWritten != strlen(pokeMsg))
    {
        qDebug() << "Socket write error" << strlen(pokeMsg) << bytesWritten;
        return false;
    }
    
    qDebug() << "Sent to server: " << pokeMsg;
    return true;
}

SERVER STDOUT

$ ./Server
Awaiting for new connections  IP "127.0.0.1" PORT 9000
New incoming connection
0x4b58 Socket info: QAbstractSocket::ConnectedState OpenMode( "ReadOnly|WriteOnly" )
0x4b58 Awaiting client message
0x4b58 Thread max running time expired
0x4b58 Thread exit

CLIENT STDOUT

$ ./Client
Connecting to "127.0.0.1" : 9000
Connected
Sent to server:  POKE
Client exit

dmitttri

@jeremy_k said in QTcpServer won't receive client data on connected QTcpSocket:

@dmitttri said in QTcpServer won't receive client data on connected QTcpSocket:

@Christian-Ehrlicher said in QTcpServer won't receive client data on connected QTcpSocket:

Hi Christian, thanks for your answer.

As always when a thread is used for an async Qt class - why do you need a thread at all?

Simply because there will be multiple clients, requesting longer jobs to be done in parallel by the server (remote project building). This is a very common server design decision.

Hi Jeremy!

Is the build (dependency walking, compilation, linking, etc) performed in process, or is it invoking external programs?

Basically it will be a multi step job:

client send a request for a remote job
client uploads project archive to the server
server executes external build script
server packs building results (and logs) into another archive and sends it back to the client

jeremy_k

@dmitttri said in QTcpServer won't receive client data on connected QTcpSocket:

@jeremy_k said in QTcpServer won't receive client data on connected QTcpSocket:

@dmitttri said in QTcpServer won't receive client data on connected QTcpSocket:

@Christian-Ehrlicher said in QTcpServer won't receive client data on connected QTcpSocket:

Hi Christian, thanks for your answer.

As always when a thread is used for an async Qt class - why do you need a thread at all?

Simply because there will be multiple clients, requesting longer jobs to be done in parallel by the server (remote project building). This is a very common server design decision.

Hi Jeremy!

Is the build (dependency walking, compilation, linking, etc) performed in process, or is it invoking external programs?

Basically it will be a multi step job:

client send a request for a remote job

client uploads project archive to the server

server executes external build script

server packs building results (and logs) into another archive and sends it back to the client

This sounds reasonable for a single thread handling multiple clients. Doing so will reduce the code complexity, and may result in better performance than running a thread per client. If there are ever so many (N) clients that a single thread can't handle them, a second thread can be added to handle another N clients. It may be easier to instead execute an additional instance of the server.

Use the QTcpServer::newConnection() signal rather than subclassing QTcpServer.
If the uploaded project is really large, read data in chunks in order to reduce latency for other clients. Chances are good that the network stack is going to cause this to happen anyway.
Use QProcess to execute the external build script. Connect to the appropriate signals to get output and exit status.
Do the same for the archiving step if it uses an external program. If it is library or application code, use a QRunnable.

dmitttri

@jeremy_k said in QTcpServer won't receive client data on connected QTcpSocket:

This sounds reasonable for a single thread handling multiple clients. Doing so will reduce the code complexity, and may result in better performance than running a thread per client. If there are ever so many (N) clients that a single thread can't handle them, a second thread can be added to handle another N clients. It may be easier to instead execute an additional instance of the server.

Thanks for the suggestion. This is a bit off-topic. The decision of multiple threads vs single thread is mostly like a decision about scheduler policy fair scheduling vs round robin. The server I am working on won't just do remote building, there will be other jobs of variable duration. Handing in one thread may impose delays for shorter jobs in case of a pending longer job. The other question may be what happens if one job get stuck? Will a single thread server get stuck too, preventing other jobs being executed? For example, while uploading a file from a connected client, if client gets suddenly disconnected, or ends up in long delays with reaching server, what server can do about it and make other jobs running?

There are pros and cons for the threaded approach, and I settled down with the multiple threads, for now.

jeremy_k

@dmitttri said in QTcpServer won't receive client data on connected QTcpSocket:

@jeremy_k said in QTcpServer won't receive client data on connected QTcpSocket:

This sounds reasonable for a single thread handling multiple clients. Doing so will reduce the code complexity, and may result in better performance than running a thread per client. If there are ever so many (N) clients that a single thread can't handle them, a second thread can be added to handle another N clients. It may be easier to instead execute an additional instance of the server.

Thanks for the suggestion. This is a bit off-topic. The decision of multiple threads vs single thread is mostly like a decision about scheduler policy fair scheduling vs round robin. The server I am working on won't just do remote building, there will be other jobs of variable duration. Handing in one thread may impose delays for shorter jobs in case of a pending longer job. The other question may be what happens if one job get stuck? Will a single thread server get stuck too, preventing other jobs being executed? For example, while uploading a file from a connected client, if client gets suddenly disconnected, or ends up in long delays with reaching server, what server can do about it and make other jobs running?

I'll preface this by stating that this sounds like a common misunderstanding for programmers who are unfamiliar with asynchronous or non-blocking I/O.

Presuming that all of these jobs are running in external processes: The ability for any job to impede another really comes down to the amount of output the jobs generate and rate at which jobs arrive and complete. There should be no blocking waits. Jobs and clients will be scheduled for compute time as they generate data.

If a job is "stuck" in that the external process stops responding, it also stops generating data for the server to process. If the client stops mid-upload, the server is likewise not impeded. The interim overhead on the server is the space to buffer a partial upload (presumably flushed to disk anyway) or output, and the open connection. Neither changes with 1 versus many threads.

There are pros and cons for the threaded approach, and I settled down with the multiple threads, for now.

Sure, it's your program. You are hopefully now aware that adding threads adds complexity with limited prospects for improved performance in the described use case.

dmitttri

@jeremy_k said in QTcpServer won't receive client data on connected QTcpSocket:

Sure, it's your program. You are hopefully now aware that adding threads adds complexity with limited prospects for improved performance in the described use case.

Sure Jeremy, have no worries about my design decisions, I'll be perfectly fine even if I may be wrong form time to time.

I would prefer to get back to the original issue.

dmitttri

@Christian-Ehrlicher said in QTcpServer won't receive client data on connected QTcpSocket:

Pleas show your code. Let the QThread run an event loop so Qt can properly receive the data from the os.

Hm, now I am starting to understand your point. By default QThread run method will just perform exec(), which is what you refer to running Qt event loop in a thread. This is why QThread doc page offers Worker/Controller example.

This may be actually what's wrong with my code above. Inheriting QThread and overriding run actually works only if we'll do something unrelated with Qt event loop (like processing an image form a RAM located buffer).

Let me try to re implement my simple example and use Worker approach.

dmitttri

@Christian-Ehrlicher said in QTcpServer won't receive client data on connected QTcpSocket:

Pleas show your code. Let the QThread run an event loop so Qt can properly receive the data from the os.

Ok, now I tried approach with worker object, following the documentation link you kindly provided. Result is the same, server side simply don't receive any data on the connected socket.

@Christian-Ehrlicher Can you check the following code in which QThread runs an event loop?

SERVER SIDE

MainServer.cpp

#include <QCoreApplication>
#include <QtCore>

#include "SimpleServer.h"


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

    SimpleServer server;
    server.start("127.0.0.1", 9000);

    return app.exec();
}

SimpleServer.h

#ifndef SIMPLESERVER_H
#define SIMPLESERVER_H

#include <QDebug>
#include <QTcpServer>

class SimpleServer : public QTcpServer
{
    Q_OBJECT;

public:
    SimpleServer(QObject *parent = nullptr);

    bool start(QString serverIP, qint16 serverPort);

protected:
    void incomingConnection(qintptr socketDescriptor) override; \
};

#endif // SIMPLESERVER_H

SimpleServer.cpp

#include <QTcpSocket>
#include <QHostAddress>

#include "SimpleServer.h"
#include "ServerWorkerThread.h"


SimpleServer::SimpleServer(QObject *parent)
    : QTcpServer(parent) {}


bool SimpleServer::start(QString serverIP, qint16 serverPort)
{
    qDebug() << "Awaiting for new connections "
             << "IP" << serverIP
             << "PORT" << serverPort;

    this->listen(QHostAddress(serverIP), serverPort);
    return true;
}

void SimpleServer::incomingConnection(qintptr socketDescriptor)
{
    qDebug() << "New incoming connection";

    ServerWorkerThread *workerThread = new ServerWorkerThread();
    workerThread->start(socketDescriptor);
}

ServerWorkerThread.h

#ifndef SERVERWORKERTHREAD_H
#define SERVERWORKERTHREAD_H

#include <QThread>
#include <QTcpSocket>

class Worker : public QObject
{
    Q_OBJECT

public slots:
    void doWork(qintptr socketDescriptor);

signals:
    void resultReady();
};

class ServerWorkerThread : public QObject
{
    Q_OBJECT
    QThread workerThread;
public:
    ServerWorkerThread()
    {
        Worker *worker = new Worker;
        worker->moveToThread(&workerThread);
        connect(&workerThread, &QThread::finished, worker, &QObject::deleteLater);
        connect(this, &ServerWorkerThread::operate, worker, &Worker::doWork);
        connect(worker, &Worker::resultReady, this, &ServerWorkerThread::handleResults);
        workerThread.start();
    }
    ~ServerWorkerThread()
    {
        workerThread.quit();
        workerThread.wait();
    }

    void start(qintptr socketDecriptor)
    {
        emit this->operate(socketDecriptor);
    }

public slots:
    void handleResults();
signals:
    void operate(qintptr socketDescriptor);
};

#endif // SERVERWORKERTHREAD_H

ServerWorkerThread.cpp

#include <QByteArray>
#include <QTime>

#include "ServerWorkerThread.h"

#define WORKER_THREAD_MAX_RUNNING_TIME_S 5


void Worker::doWork(qintptr socketDescriptor)
{
    bool workerThreadShouldRun = true;
    QTcpSocket tcpSocket;

    if (!tcpSocket.setSocketDescriptor(socketDescriptor))
    {
        qDebug() << "setSocketDescriptor fail";
        return;
    }

    qDebug() << "Socket info:" << tcpSocket.state() << tcpSocket.openMode();
    qDebug() << "Awaiting client message";

    QTime workerThreadTimeoutTime = QTime::currentTime().addSecs(WORKER_THREAD_MAX_RUNNING_TIME_S);
    while (workerThreadShouldRun)
    {
        qint64 bytesAvailable = tcpSocket.bytesAvailable();
        if(bytesAvailable > 0)
        {
            qDebug() << "Received bytes" << bytesAvailable;

            QByteArray buffer = tcpSocket.readAll();
            if (buffer.size() > 0)
            {
                qDebug() << "Received client message:" << QString(buffer.data())
                         << buffer.size() << "bytes";

                workerThreadShouldRun = false;
            }
        }
        
        if (QTime::currentTime() > workerThreadTimeoutTime)
        {
            qDebug() << "Worker max running time expired";
            workerThreadShouldRun = false;
        }
        QThread::msleep(100);
    }
    qDebug() << "Worker exit";
    emit this->resultReady();
}

void ServerWorkerThread::handleResults()
{
}

Client side remains the same..

SERVER STDOUT

$ ./Server
Awaiting for new connections  IP "127.0.0.1" PORT 9000
New incoming connection
Socket info: QAbstractSocket::ConnectedState OpenMode( "ReadOnly|WriteOnly" )
Awaiting client message
Worker max running time expired
Worker exit

CLIENT STDOUT

$ ./Client
Connecting to "127.0.0.1" : 9000
Connected
Sent to server:  POKE
Client exit

jsulm

@dmitttri said in QTcpServer won't receive client data on connected QTcpSocket:

while (workerThreadShouldRun)
{

You're blocking Qt event loop again.
You should use signals/slots instead. There is readyRead signal which you should use.

dmitttri

@jsulm said in QTcpServer won't receive client data on connected QTcpSocket:

You're blocking Qt event loop again.
You should use signals/slots instead. There is readyRead signal which you should use.

Hi, thanks for your kind answer!

I don't get this point with blocking the Qt event loop, by doing my own processing in a dedicated thread. I mean, threads are meant to be used with some kind of while loop until the job is done. doWork is implemented according to the Qt documentation, which states:

public slots:
    void doWork(const QString &parameter) {
        QString result;
        /* ... here is the expensive or blocking operation ... */
        emit resultReady(result);
    }

doWork in my case should communicate with the connected client, perform some dedicated job and then exit. Are you suggesting to use readyRead signal handler to process incoming data, and call doWork from it, taking care that doWork do not loop, but finish soon as possible? In such case my yet to come processing state machine would end up in readyRead handler, and there will be mutiple doWork methods doWork1, doWork2, which implements job steps. I can try such approach

I am sorry for asking such questions, but this is somehow fundamental for me to understand why and how my thread handler manage to stops main Qt event loop, which executes in the main program process.

I have already used thread with a Qt GUI application, and always had this pattern of while (threadShoudRun) and never had issues with event loop and GUI processing. Why this time it happens to be so problematic to run a thread processing loop?

Are there any other Qt examples of a multi threaded TCP server, other than just fortune examples?

Christian Ehrlicher

I already told you why an event loop is needed - to get the tcp data from the os into the QTcpSocket buffer so you can read it.

jsulm

@dmitttri said in QTcpServer won't receive client data on connected QTcpSocket:

threads are meant to be used with some kind of while loop until the job is done

That's what Qt event loop is for...

dmitttri

@Christian-Ehrlicher said in QTcpServer won't receive client data on connected QTcpSocket:

I already told you why an event loop is needed - to get the tcp data from the os into the QTcpSocket buffer so you can read it.

Sure thanks, but it don't really provide an explanation how to tell a new thread, run in an event loop so your tcp socket can get chance to read data.

I thought it was about worker thread article.

As explained there, in case of the Worker object approach, run method just runs exec() and this is should enable thread to run an event loop, while doWork is meant to do the thread processing job.

Other valid approach (also explained there) is to override run method and do the threaded work there. This was my original approach also posted above. Which does not work.

So I tried both options, posted the code, and with the short hints you and other members provided, I still could not understand the culprit and get it work. But this is related to my very low Qt coverage.

Thanks for your help, I will figure out what I am doing wrong, and I will take care to post the proper solution.

dmitttri

@jsulm said in QTcpServer won't receive client data on connected QTcpSocket:

@dmitttri said in QTcpServer won't receive client data on connected QTcpSocket:

threads are meant to be used with some kind of while loop until the job is done

That's what Qt event loop is for...

Hm, this is what I may be missing, as someone not really familiar with Qt.

Threads Events QObjects

In particular, under Per-thread event loop there is an explanation of what I couldn't understand up to now. Apart of the Main Qt event loop, there are Per-thread event loops and specially networking classes are required to explicitly run such loop.

jsulm

@dmitttri Please read https://wiki.qt.io/QThreads_general_usage

dmitttri

@jsulm said in QTcpServer won't receive client data on connected QTcpSocket:

@dmitttri Please read https://wiki.qt.io/QThreads_general_usage

Yep, I did it already. In the provided example, process is very simple:

void Worker::process() { // Process. Start processing data.
    // allocate resources using new here
    qDebug("Hello World!");
    emit finished();
}

This method simply return immediately, while my processing (doWork in the code above) requires networking (socket read/write) and several discrete steps to complete.

Worker approach creates a thread object, which run() method will just perform exec(), allowing thread event loop to run. This is perfectly clear.

Then it creates Worker object which has process() method, and moves Worker object to the thread context. Now process() will execute in the thread context too.

Key questions are:

How run() and process() now coexists and executes in the same thread?"
When process() is running, what's happening with the run method? It hangs until process finishes?

It seems that in my case, where process() method uses Networking, I should not use process() method at all.

More precise, instead of the process() method it seems that I need a socket readyRead slot handler which executes in the worker thread, process incoming data, keep processing state variables, and emit finished() signal. once whole client related work is done.