QModbusTcpClient use RAM more and more.

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.