Excessive memory use in QNetworkAccessManager after network disruption

Christian Ehrlicher

Don't use threads - QNetworkAccessManager is async and creating threads internally when needed.
Don't create more than one QNetworkAccessManager - RTM: https://doc.qt.io/qt-5/qnetworkaccessmanager.html#details

nyakacs

Hello Christian,

I use a semaphore to block. This also blocks the event loop. I seem to remember that there was a deadlock issue because of this, but I can certainly investigate removing the access to QNetworkAccessManager methods form its own thread. However the actual original call is form already a thread
I don't there is only one at a time. I had a previous version when the QNetworkAccessManager was destroyed so there was only ever one. I have just changed in an attempt to clean up all internal status. It made no difference.

How these two related to the issue of excessive memory use after network disruption but perfect operation otherwise?
Is it that methods of QNetworkAccessManager can only be called form the GUI thread and must live in the GUI thread?
Serialisation is already ensure by a mutex. There is only one call at a time, so is it the thread affinity you think is the problem?

Christian Ehrlicher

Again: don't use any sort of threading when trying to get something with QNetworkAccessManager and only create it once and not on every new request again. If you modified your code this way and still getting errors then show us your (compilable) code.

nyakacs

I did just that. The QNetworkAccessManager now live sin the main application thread. It is created at startup and not destroyed until exit. The trigger for the send comes from a worker thread but I queued connected it so the actaul network bit is all inthe main thread. It changed nothing at all. I will get togherther some compilable code soon.

nyakacs

And this is the code sanitised:

#include <QtNetwork/QNetworkReply>
#include <QtNetwork/QNetworkRequest>
#include <QtNetwork/qnetworkaccessmanager.h>
#include <QMutex>
#include <QSemaphore>
#include <QTimer>
#include <QUrl>

#define TIMEOUT  10*1000

class SendClass: public QObject
{
    Q_OBJECT
  public:
    SendClass();

    bool Send(QUrl url, const QByteArray& payload);

  signals:
    void signalSend(QUrl, QByteArray, QMap<QByteArray, QByteArray>);

  private slots:
    void slotResponseReceived(QNetworkReply* reply);
    void slotSend(QUrl url, QByteArray data, QMap<QByteArray, QByteArray> header);
    void slotTimeout();

  private :
    QNetworkAccessManager m_networkManager;
    QTimer m_timeout_timer;
    QNetworkReply* m_reply;
    QMutex m_send_mutex;
    QSemaphore m_wait_on_receipt;
    bool m_is_error;
};

SendClass::SendClass()
{
  qRegisterMetaType< QMap<QByteArray,QByteArray> > ( "QMap<QByteArray,QByteArray>" );

  m_timeout_timer.setSingleShot(true);
  m_timeout_timer.setInterval(TIMEOUT);
  connect(&m_timeout_timer,SIGNAL(timeout()), this, SLOT(slotTimeout()));
  connect(&m_networkManager, SIGNAL(finished(QNetworkReply*)), this, SLOT(slotResponseReceived(QNetworkReply*)));
  connect(this, SIGNAL(signalSend(QUrl, QByteArray, QMap<QByteArray, QByteArray>)), this, SLOT(slotSend(QUrl, QByteArray, QMap<QByteArray, QByteArray>)), Qt::QueuedConnection);
}

bool SendClass::Send(QUrl url, const QByteArray& payload)
{
  //called from worker thread

  QMutexLocker locker(&m_send_mutex);
 
  QMap<QByteArray, QByteArray> header;
  header.insert("Content-Type", "text/xml");

  m_is_error=false;
  emit signalSend(url, payload, header);
  //------ Await the completion of the send ------
  m_wait_on_receipt.acquire(1);

  return !m_is_error;
}

void SendClass::slotSend(QUrl url, QByteArray data, QMap<QByteArray, QByteArray> header)
{
  QNetworkRequest upload_req(url);
  upload_req.setAttribute(QNetworkRequest::DoNotBufferUploadDataAttribute, true);
  upload_req.setHeader(QNetworkRequest::ContentLengthHeader, data.size());

  QMapIterator<QByteArray, QByteArray> it(header);
  while (it.hasNext())
  {
    it.next();
    upload_req.setRawHeader(it.key(), it.value());
  }

  m_reply = m_networkManager.post(upload_req, data);
  m_timeout_timer.start();
}

void SendClass::slotResponseReceived(QNetworkReply* reply)
{
    m_is_error = true;

    if(reply!=nullptr)
    {
      QVariant statusCode = reply->attribute( QNetworkRequest::HttpStatusCodeAttribute );
      if ( statusCode.isValid() )
      {
        int status = statusCode.toInt();
        if(status==200)
        {
          QByteArray replyData = reply->readAll();
          //do sothing with result
          m_is_error = false;
        }
      }

      reply->deleteLater();
    }
    m_wait_on_receipt.release(1);
}

void SendClass::slotTimeout()
{
  m_is_error=true;
  m_reply->abort();
}

Christian Ehrlicher

@nyakacs said in Excessive memory use in QNetworkAccessManager after network disruption:

m_reply = m_networkManager.post(upload_req, data);

When you send more than one request before the previous is finished (timed out) this will leak.

nyakacs

Because of the QMutexLocker at the beginning of Send() no new call can come through until Send() is finished. The m_wait_on_receipt semaphore will not allow Send() to finish before slotResponseReceived() cleans up. If slotResponseReceived() is not called then the whole thing will freeze and wait on the semaphore indefinatelly. This is not the case.

The timeout only calls QNetworkReply::abort(). Abort will trigger QNetworkAccessManager to emit the finished() signal which is connected to slotResponseReceived() and cleans up.

Unless I really miss something big, how can I have more than one request before the previous finished and where is the leak?

Christian Ehrlicher

So you block the main thread until the request is finished? Mhhh
Why do you need all this mutex stuff? I don't see a reason. You know when a request is on the way (m_request is != nullptr) anyway so you can react properly.

nyakacs

It implements a blocking send. The Send() function does not return until we know that the send succeeded or failed. The worker thread needs to behave differetly regarding the success of the send because the data needs to be sent in strict order. The Mutex garantees that the Send() function cannot be reentered until the full network cycle is finished.

In the mean time I have increased the network timeout to 20sec. This ment that there is no more timeout but the accumulating memory use is still there. This just proves that the issue is not related to the aborting of the send but what happens to the network many layesr below.

I have two identical machines, one is always accunmulating memory, the other never. The difference is the relyabilty of connection. They are the same hardware and software in every detail.

nyakacs

After a lot more testing and comparing between systems with and without the issue, is does not seem to be related to network reliability after all.

System sending regularly and constantly are OK systems spending sporadically take up ever more memory. When about a 100K is sent every 10-20 minutes and additional 40Mb is used up daily compared to sending ever minute. This memory in not released if QNetworkAccessManager is destroyed.

Memory is taken during QnetworkAccessManager::post() and the amount can be anything between 2-300K to 3MB at a time and is not related to the amount of data sent.

Does anyone have experience with cacheing in QNetworkAccessManager? What is cached by default and how to control it?