Qt ModbusTCPClient Memory Leak

[Ashish Epsilon]

Hi,

I have an application that runs 24/7 on ModbusTCPClient and reads the data from the Server Device When I run an Application Without running the ModbusTCPClient then the size of the application in the task manager is about 18 to 20 Mb in 7-day use.

But when I start ModbusTCPClient and read continuous data (about 150 - 200 registers per second) then the application size increased and for 1 day it increased by about 450 Mb.

My ModbusTCPClient is running on a different thread when I stop reading the application memory size goes to near about 20 Mb from 450 Mb. and when I start then it will directly go to 450 Mb and gradually increase.

This memory leak case the Application to crash...

Do you have any idea what is going wrong...

Thanks and Regards,
Ashish Upara

[J.Hilk]

@Ashish-Epsilon do you delete your finished QModbusReply(s) ?

[Ashish Epsilon]

Hi @J-Hilk,

Thanks for the reply,

I have deleted the QModbusReply here you can see it in my code section in the last in the onReadReady function.

When I get data then I am performing the typecasting of the result as per the requirement and then store it in a particular location...

I am also taking one flag for it will not give the signal for multiple requests. the flag is changed by this function... Set_Modbus_Read_status(WriteRegisterModel::Modbus_FREE);

int WriteRegisterModel::read_Modbus_data(int Read_counter)
{
    num_val = Read_counter;
    QString  Function_code = DATA_Read->getValueAt(CSV_read::Set_Modbus,Read_counter, Function_code_column) ;
    QString Reg_address =DATA_Read->getValueAt(CSV_read::Set_Modbus,Read_counter, Fun_REG_column) ;
    QString length = DATA_Read->getValueAt(CSV_read::Set_Modbus,Read_counter, length_column);
    int Server_address = 1;

    if (auto *reply = modbusDevice->sendReadRequest(RegRequest(Function_code,Reg_address,length), Server_address))
    {
        if (!reply->isFinished())
        {
            modbus_read_status = WriteRegisterModel::Modbus_BUSY;
            connect(reply, &QModbusReply::finished, Genset_Modbus_obj, &WriteRegisterModel::onReadReady);
            return 1;
        }
        else
        {
            Error_string = modbusDevice->errorString();
            delete reply; // broadcast replies return immediately
        }
    }
    QCoreApplication::processEvents();
    return 0;
}

void WriteRegisterModel::onReadReady()
{
    auto reply = qobject_cast<QModbusReply *>(sender());
    if (!reply)

    return;

    if (reply->error() == QModbusDevice::NoError)
    {
        const QModbusDataUnit unit = reply->result();

        qDebug() <<  "starting add  " << unit.startAddress();
        qDebug() << "length " << unit.valueCount();

        for (qsizetype j = 0, total = unit.valueCount() ; j < total; ++j)
        {
            qDebug() << "length " << j <<unit.value(j);
        }
    
        int i = num_val;
        if((QString::number(unit.startAddress()) == DATA_Read->getValueAt(CSV_read::Set_Modbus,i, Fun_REG_column)) )
        {
            if(DATA_Read->getValueAt(CSV_read::Set_Modbus, i , unit_column) == "char")
            {
                QString Serial_no = "";

                if (DATA_Read->getValueAt(CSV_read::Set_Modbus, i , shift_column) == '1' && DATA_Read->getValueAt(CSV_read::Set_Modbus, Flag_row , shift_column) == '1' )
                {
                    for (qsizetype j = 0, total = unit.valueCount() ; j < total; ++j)
                    {

                        const QString entry = tr("Address: %1, Value: %2").arg(unit.startAddress() + j)
                                                  .arg(QString::number(unit.value(j),
                                                                       unit.registerType() <= QModbusDataUnit::Coils ? 10 : 16));
               \
                        uint16_t value = unit.value(j);

                        char LO_char =char(value) ;
                        char HI_char = value >> 8 ;

                        Serial_no = Serial_no + HI_char + LO_char;

                    }
                }
                else if (DATA_Read->getValueAt(CSV_read::Set_Modbus, i , shift_column) == '2' && DATA_Read->getValueAt(CSV_read::Set_Modbus, Flag_row , shift_column) == '1' )
                {
                    for (qsizetype j = 0, total = unit.valueCount() ; j < total; ++j)
                    {

                        const QString entry = tr("Address: %1, Value: %2").arg(unit.startAddress() + j)
                                                  .arg(QString::number(unit.value(j),
                                                                       unit.registerType() <= QModbusDataUnit::Coils ? 10 : 16));
                    
                        uint16_t value = unit.value(j);

                        char LO_char =char(value) ;
                        char HI_char = value >> 8 ;

                        Serial_no = Serial_no + LO_char + HI_char  ;
                    }
                }
                else if (DATA_Read->getValueAt(CSV_read::Set_Modbus, i , shift_column) == '3' && DATA_Read->getValueAt(CSV_read::Set_Modbus, Flag_row , shift_column) == '1' )
                {
                    for (int j = unit.valueCount() -1 ; j >= 0  ; j--)
                    {
                        const QString entry = tr("Address: %1, Value: %2").arg(unit.startAddress() + j)
                                                  .arg(QString::number(unit.value(j),
                                                                       unit.registerType() <= QModbusDataUnit::Coils ? 10 : 16));
                  
                        uint16_t value = unit.value(j);

                        char LO_char =char(value) ;
                        char HI_char = value >> 8 ;

                        Serial_no = Serial_no  + LO_char + HI_char;
                    }
                }
                else
                {
                    for (int j = unit.valueCount() -1 ; j >= 0  ; j--)
                    {
                        const QString entry = tr("Address: %1, Value: %2").arg(unit.startAddress() + j)
                                                  .arg(QString::number(unit.value(j),
                                                                       unit.registerType() <= QModbusDataUnit::Coils ? 10 : 16));
                        uint16_t value = unit.value(j);

                        char LO_char =char(value) ;
                        char HI_char = value >> 8 ;

                        Serial_no = Serial_no + HI_char + LO_char;
                    }
                }
                int length_of_SR = DATA_Read->getValueAt(CSV_read::Set_Modbus,i, scalefactor_column).toInt();
                QString final_SR_no = "";
                for (int z = 0 ; (z < length_of_SR && z < unit.valueCount()*2)   ; z++)
                {
                    final_SR_no = final_SR_no + Serial_no[z];
                }
                DATA_Read->setValueAt(CSV_read::Set_Modbus,i, Result_value_column, final_SR_no);

            }
            else
            {
                uint64_t value = 0;
                if (DATA_Read->getValueAt(CSV_read::Set_Modbus, i , shift_column) == '1' && DATA_Read->getValueAt(CSV_read::Set_Modbus, Flag_row , shift_column) == '1' )
                {
                    for (qsizetype j = 0, total = unit.valueCount() ; j < total; ++j)
                    {

                        const QString entry = tr("Address: %1, Value: %2").arg(unit.startAddress() + j).arg(QString::number(unit.value(j),
                          unit.registerType() <= QModbusDataUnit::Coils ? 10 : 16));
     
                        value = (value << 16) | unit.value(j);

                    }
                }
                else
                {
                    for (int j = unit.valueCount() -1 ; j >= 0  ; j--)
                    {
                        const QString entry = tr("Address: %1, Value: %2").arg(unit.startAddress() + j)
                                                  .arg(QString::number(unit.value(j),
                                                                       unit.registerType() <= QModbusDataUnit::Coils ? 10 : 16));
                
                        value = (value << 16) | unit.value(j);

                    }
                }

                int64_t a = 0;
                double val =0;
                if(DATA_Read->getValueAt(CSV_read::Set_Modbus, i , unit_column) == "int16")
                {
                    a = int16_t (value);
                    val = a;
                }
                else if(DATA_Read->getValueAt(CSV_read::Set_Modbus, i , unit_column) == "uint16") {
                    a = uint16_t (value);
                    val = a;
                }
                else if(DATA_Read->getValueAt(CSV_read::Set_Modbus, i , unit_column) == "int32")
                {
                    a = int32_t (value);
                    val = a;
                }
                else if(DATA_Read->getValueAt(CSV_read::Set_Modbus, i , unit_column) == "uint32")
                {
                    a = uint32_t (value);
                    val = a;
                }
                else if(DATA_Read->getValueAt(CSV_read::Set_Modbus, i , unit_column) == "int64")
                {
                    a = int64_t (value);
                    val = a;
                }
                else // if the data type is uint64_t then thake directly
                {
                    val = value;
                }



                if ( DATA_Read->getValueAt(CSV_read::Set_Modbus, Flag_row , scalefactor_column) == '1')  // if scale factor flag enable then do this
                {
                    val = val / (DATA_Read->getValueAt(CSV_read::Set_Modbus, i , scalefactor_column).toDouble());
                }

                if ( DATA_Read->getValueAt(CSV_read::Set_Modbus, Flag_row , offset_column) == '1')  // if offset flag enable then do this
                {
                    val = val + (DATA_Read->getValueAt(CSV_read::Set_Modbus, i , offset_column).toDouble());
                }

                QString setst = tr("%1").arg(val) ;
                DATA_Read->setValueAt(CSV_read::Set_Modbus,i, Result_value_column, setst);
                DATA_Read->Set_value_in_modbus_json(DATA_Read->getValueAt(CSV_read::Set_Modbus, i , parameter_column) ,  setst);
        }

        }

    } else if (reply->error() == QModbusDevice::ProtocolError) {
        Error_string = (tr("Read response error: %1 (Modbus exception: 0x%2)").
                        arg(reply->errorString()).
                        arg(reply->rawResult().exceptionCode(), -1, 16));
        qWarning() << Error_string;
        modbus_read_status = WriteRegisterModel::Modbus_ERROR;
    } else {
        Error_string = (tr("Read response error: %1 (code: 0x%2)").
                        arg(reply->errorString()).
                        arg(reply->error(), -1, 16));
        qWarning() << Error_string;
        modbus_read_status = WriteRegisterModel::Modbus_ERROR;
    }
    change_buttton_green();
    reply->deleteLater();
    connect(reply, &QModbusReply::destroyed, [this] { QTimer::singleShot(1, [this] { Set_Modbus_Read_status(WriteRegisterModel::Modbus_FREE); }); });
}

[J.Hilk]

@Ashish-Epsilon said in Qt ModbusTCPClient Memory Leak:

auto reply = qobject_cast<QModbusReply *>(sender());

ok, first of all, please don't use sender(). I know some people seem to promote it, just don't use it "This function violates the object-oriented principle of modularity"

Simply store your reply in a class variable after the connect statement

also don't spin the event loop manually !!
@Ashish-Epsilon said in Qt ModbusTCPClient Memory Leak:

QCoreApplication::processEvents();

this is far from free, in relation to memory, consider using a member timer or QMetaObject::Invoke with Qt::QueuedConnection

connect(reply, &QModbusReply::destroyed, [this] { QTimer::singleShot(1, [this] { Set_Modbus_Read_status(WriteRegisterModel::Modbus_FREE); }); });

Besides those nothing obvious jumps out.

Do you accumulate all your data in memory? For example when you call DATA_Read->setDataAt and never overwrite or clear that memory ?

[Ashish Epsilon]

Hi @J-Hilk ,

Sorry For the late reply Actually I have made changes and tested it.

Removing the event loop,

QCoreApplication::processEvents();

and also remove this single shot timer and directly delete the reply and take the reply as a class variable, and assign a null pointer to reply before the changing the Modbus flag.

int WriteRegisterModel::read_Modbus_data(int Read_counter)
{
    num_val = Read_counter;
    QString  Function_code = DATA_Read->getValueAt(CSV_read::Set_Modbus,Read_counter, Function_code_column) ;
    QString Reg_address =DATA_Read->getValueAt(CSV_read::Set_Modbus,Read_counter, Fun_REG_column) ;
    QString length = DATA_Read->getValueAt(CSV_read::Set_Modbus,Read_counter, length_column);
    int Server_address = 1;

    if (modbus_reply = modbusDevice->sendReadRequest(RegRequest(Function_code,Reg_address,length), Server_address))
    {

       if (!modbus_reply->isFinished())
        {
            modbus_read_status = WriteRegisterModel::Modbus_BUSY;
            connect(modbus_reply, &QModbusReply::finished, Genset_Modbus_obj, &WriteRegisterModel::onReadReady);
            return 1;
        }
        else
        {
           Error_string = modbusDevice->errorString();
            delete modbus_reply; // broadcast replies return immediately
            modbus_reply = nullptr;
        }
    }
    return 0;
}

void WriteRegisterModel::onReadReady()
{
    if (!modbus_reply)
    {
        Set_Modbus_Read_status(WriteRegisterModel::Modbus_FREE);
        return;
    }
    if (modbus_reply->error() == QModbusDevice::NoError)
    {
        const QModbusDataUnit unit = modbus_reply->result();
#if DEBUG_EN
        qDebug() <<  "starting add  " << unit.startAddress();
        qDebug() << "length " << unit.valueCount();

        for (qsizetype j = 0, total = unit.valueCount() ; j < total; ++j)
        {
            qDebug() << "length " << j <<unit.value(j);
        }
#endif
     
        int i = num_val;
        if((QString::number(unit.startAddress()) == DATA_Read->getValueAt(CSV_read::Set_Modbus,i, Fun_REG_column)) )
        {
            if(DATA_Read->getValueAt(CSV_read::Set_Modbus, i , unit_column) == "char")
            {
                QString Serial_no = "";

                if (DATA_Read->getValueAt(CSV_read::Set_Modbus, i , shift_column) == '1' && DATA_Read->getValueAt(CSV_read::Set_Modbus, Flag_row , shift_column) == '1' )
                {
                    for (qsizetype j = 0, total = unit.valueCount() ; j < total; ++j)
                    {

                        const QString entry = tr("Address: %1, Value: %2").arg(unit.startAddress() + j)
                                                  .arg(QString::number(unit.value(j),
                                                                       unit.registerType() <= QModbusDataUnit::Coils ? 10 : 16));
     
                        uint16_t value = unit.value(j);

                        char LO_char =char(value) ;
                        char HI_char = value >> 8 ;

                        Serial_no = Serial_no + HI_char + LO_char;

                    }
                }
                else if (DATA_Read->getValueAt(CSV_read::Set_Modbus, i , shift_column) == '2' && DATA_Read->getValueAt(CSV_read::Set_Modbus, Flag_row , shift_column) == '1' )
                {
                    for (qsizetype j = 0, total = unit.valueCount() ; j < total; ++j)
                    {

                        const QString entry = tr("Address: %1, Value: %2").arg(unit.startAddress() + j)
                                                  .arg(QString::number(unit.value(j),
                                                                       unit.registerType() <= QModbusDataUnit::Coils ? 10 : 16));
     
                        uint16_t value = unit.value(j);

                        char LO_char =char(value) ;
                        char HI_char = value >> 8 ;

                        Serial_no = Serial_no + LO_char + HI_char  ;
                    }
                }
                else if (DATA_Read->getValueAt(CSV_read::Set_Modbus, i , shift_column) == '3' && DATA_Read->getValueAt(CSV_read::Set_Modbus, Flag_row , shift_column) == '1' )
                {
                    for (int j = unit.valueCount() -1 ; j >= 0  ; j--)
                    {
                        const QString entry = tr("Address: %1, Value: %2").arg(unit.startAddress() + j)
                                                  .arg(QString::number(unit.value(j),
                                                                       unit.registerType() <= QModbusDataUnit::Coils ? 10 : 16));

                        uint16_t value = unit.value(j);

                        char LO_char =char(value) ;
                        char HI_char = value >> 8 ;

                        Serial_no = Serial_no  + LO_char + HI_char;
                    }
                }
                else
                {
                    for (int j = unit.valueCount() -1 ; j >= 0  ; j--)
                    {
                        const QString entry = tr("Address: %1, Value: %2").arg(unit.startAddress() + j)
                                                  .arg(QString::number(unit.value(j),
                                                                       unit.registerType() <= QModbusDataUnit::Coils ? 10 : 16));

                        uint16_t value = unit.value(j);

                        char LO_char =char(value) ;
                        char HI_char = value >> 8 ;

                        Serial_no = Serial_no + HI_char + LO_char;

                    }
                }

                int length_of_SR = DATA_Read->getValueAt(CSV_read::Set_Modbus,i, scalefactor_column).toInt();
                QString final_SR_no = "";
                for (int z = 0 ; (z < length_of_SR && z < unit.valueCount()*2)   ; z++)
                {
                    final_SR_no = final_SR_no + Serial_no[z];
                }
                DATA_Read->setValueAt(CSV_read::Set_Modbus,i, Result_value_column, final_SR_no);

            }
            else
            {
                uint64_t value = 0;
                if (DATA_Read->getValueAt(CSV_read::Set_Modbus, i , shift_column) == '1' && DATA_Read->getValueAt(CSV_read::Set_Modbus, Flag_row , shift_column) == '1' )
                {
                    for (qsizetype j = 0, total = unit.valueCount() ; j < total; ++j)
                    {

                        const QString entry = tr("Address: %1, Value: %2").arg(unit.startAddress() + j)
                                                  .arg(QString::number(unit.value(j),
                                                                       unit.registerType() <= QModbusDataUnit::Coils ? 10 : 16));

                        value = (value << 16) | unit.value(j);

                    }
                }
                else
                {
                    for (int j = unit.valueCount() -1 ; j >= 0  ; j--)
                    {
                        const QString entry = tr("Address: %1, Value: %2").arg(unit.startAddress() + j)
                                                  .arg(QString::number(unit.value(j),
                                                                       unit.registerType() <= QModbusDataUnit::Coils ? 10 : 16));
   
                        value = (value << 16) | unit.value(j);

                    }
                }

                int64_t a = 0;
                double val =0;
                if(DATA_Read->getValueAt(CSV_read::Set_Modbus, i , unit_column) == "int16")
                {
                    a = int16_t (value);
                    val = a;
                }
                else if(DATA_Read->getValueAt(CSV_read::Set_Modbus, i , unit_column) == "uint16") {
                    a = uint16_t (value);
                    val = a;
                }
                else if(DATA_Read->getValueAt(CSV_read::Set_Modbus, i , unit_column) == "int32")
                {
                    a = int32_t (value);
                    val = a;
                }
                else if(DATA_Read->getValueAt(CSV_read::Set_Modbus, i , unit_column) == "uint32")
                {
                    a = uint32_t (value);
                    val = a;
                }
                else if(DATA_Read->getValueAt(CSV_read::Set_Modbus, i , unit_column) == "int64")
                {
                    a = int64_t (value);
                    val = a;
                }
                else // if the data type is uint64_t then thake directly
                {
                    val = value;
                }

                if ( DATA_Read->getValueAt(CSV_read::Set_Modbus, Flag_row , scalefactor_column) == '1')  // if scale factor flag enable then do this
                {
                    val = val / (DATA_Read->getValueAt(CSV_read::Set_Modbus, i , scalefactor_column).toDouble());
                }

                if ( DATA_Read->getValueAt(CSV_read::Set_Modbus, Flag_row , offset_column) == '1')  // if offset flag enable then do this
                {
                    val = val + (DATA_Read->getValueAt(CSV_read::Set_Modbus, i , offset_column).toDouble());
                }

                QString setst = tr("%1").arg(val) ;
     
                DATA_Read->setValueAt(CSV_read::Set_Modbus,i, Result_value_column, setst);
                DATA_Read->Set_value_in_modbus_json(DATA_Read->getValueAt(CSV_read::Set_Modbus, i , parameter_column) ,  setst);

            }


        }

    } else if (modbus_reply->error() == QModbusDevice::ProtocolError) {
        Error_string = (tr("Read response error: %1 (Modbus exception: 0x%2)").
                        arg(modbus_reply->errorString()).
                        arg(modbus_reply->rawResult().exceptionCode(), -1, 16));
        qWarning() << Error_string;
        modbus_read_status = WriteRegisterModel::Modbus_ERROR;
    } else {
        Error_string = (tr("Read response error: %1 (code: 0x%2)").
                        arg(modbus_reply->errorString()).
                        arg(modbus_reply->error(), -1, 16));
        qWarning() << Error_string;
        modbus_read_status = WriteRegisterModel::Modbus_ERROR;
    }
    change_buttton_green();
    delete modbus_reply;
    modbus_reply = nullptr;
    Set_Modbus_Read_status(WriteRegisterModel::Modbus_FREE);
}

And When I call DATA_Read->setDataAt it will first assign a new QStandardItem into the QStandardItemModel and after that, it will overwrite at that location here you can see

QStandardItemModel *Modbus_config =  new QStandardItemModel();  ; 
void CSV_read::setValueAt(CSV_read::Config_type type , int row, int column, const QString &value )
{
    if (type == CSV_read::Set_Modbus)
    {
        if (!Modbus_config->item(row, column))
        {     
            Modbus_config->setItem(row, column, new QStandardItem(value));              
        }
        else
        {
            Modbus_config->item(row, column)->setText(value);
        }
    }
}

After these changes I tested for 16 hours Still, the application size is increasing with time...

Do you have any idea where this leak happen...

Thanks and Regards,
Ashish Upara

[J.Hilk]

@Ashish-Epsilon said in Qt ModbusTCPClient Memory Leak:

Sorry For the late reply Actually I have made changes and tested it.

Excellent ! * thumbsup *

@Ashish-Epsilon said in Qt ModbusTCPClient Memory Leak:

After these changes I tested for 16 hours Still, the application size is increasing with time...
Do you have any idea where this leak happen...

Like I said, thats the only things that I noticed right away.

I would now, go and "break your code down". Remove or disable everything but modbus communication. Just the basic back an forth, see if that still increases in size.

[Ashish Epsilon]

Hi @J-Hilk,

Sorry For the late reply Actually I have breakdown my code and run only Modbus functionality with 2 different devices. from that, I found one suspicious line.

      DATA_Read->Set_value_in_modbus_json(DATA_Read->getValueAt(CSV_read::Set_Modbus, i , parameter_column) ,  setst);

in the code section of

.......

                if ( DATA_Read->getValueAt(CSV_read::Set_Modbus, Flag_row , offset_column) == '1')  // if offset flag enable then do this
                {
                    val = val + (DATA_Read->getValueAt(CSV_read::Set_Modbus, i , offset_column).toDouble());
                }

                QString setst = tr("%1").arg(val) ;
     
                DATA_Read->setValueAt(CSV_read::Set_Modbus,i, Result_value_column, setst);
                DATA_Read->Set_value_in_modbus_json(DATA_Read->getValueAt(CSV_read::Set_Modbus, i , parameter_column) ,  setst);

..........

DATA_Read->Set_value_in_modbus_json is my QJsonObject update line you can see this function

void CSV_read::Set_value_in_modbus_json(QString name, QJsonValue value)
{
    Modbus_register_mutex.lock();
    Modbus_register[name] = value;
    Modbus_register_mutex.unlock();
}

Modbus_register_mutex and the Modbus_register is the class variable

    QMutex Modbus_register_mutex;
    QJsonObject Modbus_register;

if I comment DATA_Read->Set_value_in_modbus_json line then the memory in the task manager is stable and when I uncomment (run this line in code ) then the memory in the task manager increases.

Do you have any idea about it...

Thanks and Regards,
Ashish Upara

[J.Hilk]

@Ashish-Epsilon ok, it seems to be a Problem with the QJsonObject

are you sure that name is always a pre-existing valid key of the QJsonObject ? And why don't you use insert here ?

also do you clear that jsonobject eventually ? as in write it to file or something ? otherwise it might be growing uncontrollably.