QModbusTcpClient use RAM more and more.

[neeme]

But when use QModbusRtuSerialMaster everything is OK!

#ifndef CMODBUS_H
#define CMODBUS_H

#include <QModbusTcpClient>
#include <CSerialPortConfig.h>
#include <QModbusRtuSerialMaster>

class CModBus : public CSerialPortConfig
{
    Q_OBJECT
public:
    explicit CModBus(const QString &capstr);
    ~CModBus();

    void portOpen();
    void portClose();
    bool portIsOpen();

    void read(const QModbusDataUnit &mdu, const int &address);
    void write(const QModbusDataUnit &mdu, const int &address);

signals:
    void linkState(const int &address, const bool &st);
    void dataReady(const int &address, const QModbusDataUnit &mdu);

private:
    void readReady();
    void writeFinished();
    void newSetttings(const Settings &ns);
    void errorOccurred(const QModbusDevice::Error &error);

    int m_lastAddress = 0;
    QModbusClient *m_device = nullptr;
};

#endif // CMODBUS_H

#include "CModBus.h"

CModBus::CModBus(const QString &capstr) : CSerialPortConfig(capstr)
{    
    newSetttings(curSettings());
    connect(this, &CSerialPortConfig::sigNewSetttings, this, &CModBus::newSetttings);
    connect(m_device, &QModbusClient::errorOccurred, this, &CModBus::errorOccurred);
    m_device->connectDevice();

    if(m_device->state() != QModbusDevice::ConnectedState)
    {
        errorAppend(capstr + " : " + portName() + tr(" : port error!"));
    }
}

CModBus::~CModBus()
{

}

void CModBus::newSetttings(const Settings &ns)
{
    bool isopen = false;

    if(m_device != nullptr)
    {
        isopen = (m_device->state() == QModbusDevice::ConnectedState);
        delete m_device;
        m_device = nullptr;
    }

    if(ns.tcpEnable)
    {
        m_device = new QModbusTcpClient(this);
        const QUrl url = QUrl::fromUserInput(ns.tcpHost);
        m_device->setConnectionParameter(QModbusDevice::NetworkPortParameter, url.port());
        m_device->setConnectionParameter(QModbusDevice::NetworkAddressParameter, url.host());
    }
    else
    {
        m_device = new QModbusRtuSerialMaster(this);
        m_device->setConnectionParameter(QModbusDevice::SerialPortNameParameter, ns.portName);
        m_device->setConnectionParameter(QModbusDevice::SerialParityParameter, ns.parity);
        m_device->setConnectionParameter(QModbusDevice::SerialBaudRateParameter, ns.baud);
        m_device->setConnectionParameter(QModbusDevice::SerialDataBitsParameter, ns.dataBits);
        m_device->setConnectionParameter(QModbusDevice::SerialStopBitsParameter, ns.stopBits);
    }

    m_device->setTimeout(ns.responseTime);
    m_device->setNumberOfRetries(ns.numberOfRetries);

    if(isopen)
    {
        m_device->connectDevice();
    }

}

void CModBus::read(const QModbusDataUnit &mdu, const int &address)
{
    if((!m_device) || (m_device->state() != QModbusDevice::ConnectedState))
    {
        emit linkState(address, false);
        return;
    }

    QModbusReply *reply = m_device->sendReadRequest(mdu, address);

    if(reply)
    {
        m_lastAddress = address;

        if(!reply->isFinished())
        {
            connect(reply, &QModbusReply::finished, this, &CModBus::readReady);
        }
        else
        {
            emit linkState(address, false);
            delete reply;
        }
    }
    else
    {
        emit linkState(address, false);
    }
}

void CModBus::write(const QModbusDataUnit &mdu, const int &address)
{
    if((!m_device) || (m_device->state() != QModbusDevice::ConnectedState))
    {
        emit linkState(address, false);
        return;
    }

    QModbusReply *reply = m_device->sendWriteRequest(mdu, address);

    if(reply)
    {
        m_lastAddress = address;

        if(!reply->isFinished())
        {
            connect(reply, &QModbusReply::finished, this, &CModBus::writeFinished);
        }
        else
        {
            emit linkState(address, false);
            delete reply;
        }
    }
    else
    {
        emit linkState(address, false);
    }
}

void CModBus::readReady()
{
    QModbusReply *reply = qobject_cast<QModbusReply *>(sender());

    if(!reply)
    {
        emit linkState(m_lastAddress, false);
        return;
    }

    const int address = reply->serverAddress();
    const QModbusDataUnit mdu = reply->result();

    if(reply->error() == QModbusDevice::NoError)
    {
        emit dataReady(address, mdu);
        emit linkState(address, true);
    }
    else
    {
        emit linkState(m_lastAddress, false);
    }

    reply->deleteLater();
}

void CModBus::writeFinished()
{
    auto *reply = qobject_cast<QModbusReply *>(sender());

    if(!reply)
    {
        emit linkState(m_lastAddress, false);
        return;
    }

    if(reply->error() == QModbusDevice::NoError)
    {
        emit linkState(reply->serverAddress(), true);
    }
    else
    {
        emit linkState(m_lastAddress, false);
    }

    reply->deleteLater();
}

void CModBus::errorOccurred(const QModbusDevice::Error &error)
{
    Q_UNUSED(error)
    emit linkState(m_lastAddress, false);
}

void CModBus::portOpen()
{
    m_device->connectDevice();
}

void CModBus::portClose()
{
    m_device->disconnectDevice();
}

bool CModBus::portIsOpen()
{
    return (m_device->state() == QModbusDevice::ConnectedState);
}

[sierdzio]

Please wrap your code in code tags, otherwise it's unreadable on the forum:

```
// code goes here
```

Check your app with address sanitizer, if there is a leak it will show up.

[neeme]

I do it, now is OK. Thank you!

[Lari]

@neeme , did you fix the leak? if you did it, could you tell how to fix it, please. I have the leak too when using QModbusTcpClient.

[neeme]

@Lari said in QModbusTcpClient use RAM more and more.:

did you fix the leak?

I can't find the reason！

[J.Hilk]

the first point to change would be, to not rely on sender and consecutively object_casts.

Use a QModbusReply * member pointer or a queue of pointers, don't expect to magically get the pointer to the object back at one or an other point in time!

[Lari]

Hi,@J-Hilk.

Could you explain that a little bit more?

[KroMignon]

@Lari said in QModbusTcpClient use RAM more and more.:

Could you explain that a little bit more?

Suggestion would be to do this:

• change readReady() to readReady(QModbusReply * reply)
• change writeFinished() to writeFinished(QModbusReply * reply)
• change connect statement as follow

// in read()
connect(reply, &QModbusReply::finished, this, [reply, this]() {
        readReady(reply);
    });

// in write()
connect(reply, &QModbusReply::finished, this, [reply, this]() {
        writeFinished(reply);
    });

[Lari]

@KroMignon , thank you.

I tested your decision, but it didn't help. Memory leaks away.
but if you disconnect from the server, then this memory is deleted.

[SGaist]

Hi,

Are you properly deleting the replies you get ?

[KroMignon]

@Lari said in QModbusTcpClient use RAM more and more.:

but if you disconnect from the server, then this memory is deleted.

This is strange.
It's look like you do not delete the replies, and they are cleaned-up with parent destruction.

In your code, you are using deleteLater(), you could add traces (with qDebug()) to ensure those lines are executed.

deleteLater() will delay the deletion on next enter in thread event loop.
As you signals/slots seems to work, this should also work.

I have no explanation :(

[Christian Ehrlicher]

See https://bugreports.qt.io/browse/QTBUG-92072

[Lari]

I modified the Qt example (\serialbus\modbus\master) a little to organize a continuous exchange with a frequency of 250ms. I read 10 holding registers and see a leak from 5-7 minutes and it grows. I changed only these two functions.

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

    if (reply->error() == QModbusDevice::NoError) 
	{
        ;
		
//        const QModbusDataUnit unit = reply->result();
//        for (int i = 0, total = int(unit.valueCount()); i < total; ++i) {
//            const QString entry = tr("Address: %1, Value: %2").arg(unit.startAddress() + i)
//                                     .arg(QString::number(unit.value(i),
//                                          unit.registerType() <= QModbusDataUnit::Coils ? 10 : 16));
//            ui->readValue->addItem(entry);
//        }
    } 
	else if (reply->error() == QModbusDevice::ProtocolError) 
	{
        statusBar()->showMessage(tr("Read response error: %1 (Mobus exception: 0x%2)").
                                    arg(reply->errorString()).
                                    arg(reply->rawResult().exceptionCode(), -1, 16), 5000);
    } else 
	{
        statusBar()->showMessage(tr("Read response error: %1 (code: 0x%2)").
                                    arg(reply->errorString()).
                                    arg(reply->error(), -1, 16), 5000);
    }

    reply->deleteLater();
}

 void MainWindow::onReadButtonClicked()
{
    if (!modbusDevice)
        return;
    ui->readValue->clear();
    statusBar()->clearMessage();

    mStart = (!mStart) ? true : false;
    if (mStart)
        ui->readButton->setText("Stop read");
    else
        ui->readButton->setText("Read");
		
		
    while (mStart)
    {
        if (auto *reply = modbusDevice->sendReadRequest(readRequest(), ui->serverEdit->value())) {
        if (!reply->isFinished())
        {
            QObject::connect(reply, &QModbusReply::finished, this, [reply, this]() {
                  onReadReady(reply);
            });
        }
        else
            delete reply; // broadcast replies return immediately
        } 
		else 
		{
			statusBar()->showMessage(tr("Read error: ") + modbusDevice->errorString(), 5000);
        }
        for (int i=0; i<25; i++)  // I do this to organize a delay of 250 ms
        {
            QThread::msleep(10);
            QCoreApplication::processEvents();
        }
    }
    return;
}

This simple use case shows a leak in both Windows and Linux after 5-7 minutes. And it grows constantly. Checked by the 2 days of work of the example.

[Christian Ehrlicher]

@Lari The bug was already confirmed - see my link.

[Lari]

@Christian-Ehrlicher said in QModbusTcpClient use RAM more and more.:

he bug was already confirmed - see my link.

Christian-Ehrlicher,
Thank you. I created this bug in bugreports. But Qt answers for a long time, and I am in uncertainty - what am I doing wrong? and how to make the QModbusClient object work correctly?

[Christian Ehrlicher]

@Lari Either fix the code by yourself or find someone who does for you.

[Lari]

@Christian-Ehrlicher,
ok. thank you.

[Christian Ehrlicher]

Since it seems to work with 5.14 (at least it looks like in the bug report) I would start checking what changed between 5.14 and 5.15 for this module/class.

[Lari]

@Christian-Ehrlicher said in QModbusTcpClient use RAM more and more.:

Since it seems to work with 5.14 (at least it looks like in the bug report) I would start checking what changed between 5.14 and 5.15 for this module/class.

Christian-Ehrlicher,
Thank you for help.

[Lari]

5.13 and 5.12 version - there is no memory leak in this module. 5.14 - already has memory leak.