c++while-loopmemory-leaksqthreadqtcpsocket

QTcpSocket continuous write in worker process. Best practice to avoid memory leaks


I have to keep sending, at a specific time interval, a single small packet through a TCP socket to a server. I'm developing with Qt. My idea is to create class that inherits from QObject, put the Socket write part inside an infinite loop in such class and execute it as a separate thread.
It works, but leaks memory....and at this point i'm quite lost.
This is what I've done so far (sort of pseudo-code...):

Task_SendPacket.h

#ifndef TASK_SENDPACKET_H
#define TASK_SENDPACKET_H

#include <QObject>
#include <QTcpSocket>

class Task_SendPacket : public QObject
{
    Q_OBJECT
public:
    explicit Task_SendPacket(QObject *parent = nullptr);

public slots:
    void startTask();

    void _connected();
    void _readReady();
    void _error(QAbstractSocket::SocketError _Error);

signals:

private:

    QTcpSocket *mpSocket;
    char SomeData[100];
};

#endif // TASK_SENDPACKET_H

Task_SendPacket.cpp

#include "Task_SendPacket.h"

#include <QThread>
#include <QDebug>

Task_SendPacket::Task_SendPacket(QObject *parent)
    : QObject(parent)
{

}

void Task_SendPacket::startTask(){

while(true){

        mpSocket = new QTcpSocket(this);

        connect(mpSocket, &QTcpSocket::disconnected, mpSocket, &QTcpSocket::deleteLater);
        connect(mpSocket, &QTcpSocket::connected, this, &Task_SetSelectedCamera::_connected);
        connect(mpSocket, &QTcpSocket::readyRead, this, &Task_SetSelectedCamera::_readReady);
        connect(mpSocket, &QTcpSocket::errorOccurred,this, &Task_SetSelectedCamera::_error);

        mpSocket->connectToHost("192.168.0.1", 50100, QTcpSocket::ReadWrite);

        if(!mpSocket->waitForConnected(Protocol::NetworkTimeout_ms)){

            qWarning() << "SS Packet send - Connection timeout";
        }

        if(!mpSocket->waitForBytesWritten(Protocol::NetworkTimeout_ms)){

            qWarning() << "SS Packet send - write timeout";
            mpSocket->disconnectFromHost();
        }

        if(!mpSocket->waitForReadyRead(Protocol::NetworkTimeout_ms)){

            qWarning() << "SS Packet send - read timeout";
            mpSocket->disconnectFromHost();
        }
    
    QThread::msleep(1000);
}

void Task_SetSelectedCamera::_connected(){
    
    if(mpSocket->write(SomeData, 100) == -1){
        qWarning() << "Write Header error ";
    };
}

void Task_SetSelectedCamera::_readReady(){

    char DataRead[100] = mpSocket->readAll();

    /* Do stuff with DataRead...*/

    mpSocket->disconnectFromHost();

}

void Task_SetSelectedCamera::_error(QAbstractSocket::SocketError _Error){

    qWarning() << _Error;

}
}

main.cpp

/* Start Task SetSelectedCamera */
    QThread *ServiceThread1 = new QThread();
    Task_SendPacket *oTaskSendPacket = new Task_SendPacket();
    oTaskSendPacket->moveToThread(ServiceThread1);

    QObject::connect(ServiceThread1, &QThread::finished, oTaskSendPacket, &QObject::deleteLater);
    QObject::connect(ServiceThread1, &QThread::started, oTaskSendPacket, &Task_SendPacket::startTask);
    ServiceThread1->start();
    qInfo() << "Task SendPacket started";

If I let the application run for some days it grows memory and crashes.
Is this the correct way to create a new QTcpSocket, conenct it and send data ?

Thaks

EDIT
I've edited the original post because I was not focusing on the right point. In my idea the new QTcpSocket(...) must run at each iteration of the while loop because I expect that mpSocket is marked for deletion every time it disconnects.
In a second experiment (reported here below) I've tryed to move the socket part before the while loop...and it still works.

mpSocket = new QTcpSocket(this);

connect(mpSocket, &QTcpSocket::disconnected, mpSocket, &QTcpSocket::deleteLater);
connect(mpSocket, &QTcpSocket::connected, this, &Task_SetSelectedCamera::_connected);
connect(mpSocket, &QTcpSocket::readyRead, this, &Task_SetSelectedCamera::_readReady);
connect(mpSocket, &QTcpSocket::errorOccurred,this, &Task_SetSelectedCamera::_error);

while(true){ 

    mpSocket->connectToHost("192.168.0.1", 50100, QTcpSocket::ReadWrite);

    if(!mpSocket->waitForConnected(Protocol::NetworkTimeout_ms)){

        qWarning() << "SS Packet send - Connection timeout";
    }

    if(!mpSocket->waitForBytesWritten(Protocol::NetworkTimeout_ms)){

        qWarning() << "SS Packet send - write timeout";
        mpSocket->disconnectFromHost();
    }

    if(!mpSocket->waitForReadyRead(Protocol::NetworkTimeout_ms)){

        qWarning() << "SS Packet send - read timeout";
        mpSocket->disconnectFromHost();
    }

    QThread::msleep(1000);
}

So now questions are:

  1. Is it correct to create a new QTcpSocket at each iteration?
  2. Why does the socket still work after the disconnect (because I do receive a disconnect...I Tracked that)

Solution

  • Shame on me !
    I was wrong in so many ways, but please forgive me, I'm young and fancy.

    I want to post here about the mistakes I've made hoping it might help others, because I think that I've gotten to this wrong solution influenced by my previous sexperience in programming with RTOSs.

    First off
    Do never ever ever put an infinite while loop inside a QThread or in a QObject that is moveToThread(...) to a QThread. This because the while loop will keep the QEventLoop from running. Everything will remain stucked inside

    while(true){
    
    ...
    
    }
    

    and none of the lifecycle of the application (includeing signals and slots) will go on!

    Then - Signal/Slot stuck
    I should have gotten suspicious about something broken by the fact that nothing was woring without the

    ...
    if(!mpSocket->waitForBytesWritten(Protocol::NetworkTimeout_ms)){
    
        qWarning() << "SS Packet send - write timeout";
        mpSocket->disconnectFromHost();
    }
    
    if(!mpSocket->waitForReadyRead(Protocol::NetworkTimeout_ms)){
    
        qWarning() << "SS Packet send - read timeout";
        mpSocket->disconnectFromHost();
    }
    ...
    

    No ReadyRead signal, no BytesWritten signal and so on. This is (obviously...now) because theese function are blocking and since my infinite loop was blocking the signal/slot logic there was no other way to have my QTcpSocket working. Signals and Slots work if the app lifecycle is working, otherwise...they don't.

    Last - deleteAater() not working
    The last thing that I couldn't understand was why, putting my

    mpSocket = new QTcpSocket(this);
    

    before the while loop (which was supposed to delete the socket at each iteration, the socket would still be working.
    Also this is due to the fact that if the QEventLoop is not working the deleteLater() of the socket would not work...this is, by the way, also the reason why (I think) my application was leaking memory!

    Now I've re implemented with a QTimer logic and it is working properly.


    The reason why I got tricked, I think, is because coming from RTOSs the basic idea there is to have in each Thread an init part and a cycle part. This is why I was looking for an infinite loop...

    Still remains the doubt of how I could make an infinite loop inside a QThread without:

    1. Messing up the application logic
    2. Using QTimer

    Thanks