Excessive memory use in QNetworkAccessManager after network disruption

nyakacs

Hi, I have a system with an unreliable mobile network connection. I make https posts of about 100K data sporadically using QNetworkAccessManager::post. The QNetworkReply objects are deleted after every post using deleteLater(), so is the QIODevice containing the data as well as the QNetworkAccessManager itself.

In normal operation the system works as it should and memory usage is very stable. However, sometimes, the post just hangs due to network disruptions. I have a timer to detect this and call QNetworkReply::abort() to abort the post delete all the objects and try in at a later time. I see no excess memory taken when this happens.

However after an abort was done every subsequent post will start using up more and more memory. The data transmitted will be correct, but the application will eventually eat up all the available memory. The amount of memory lost per post varies.

A cut down bits of the code are given below. The idea is that I want the post to be blocking so I emit a QueuedConnection signal from the Send() function to the slotSend slot which lives in it's own thread with its own event loop.

The QNetworkAccessManagers finished() signal is directly connected to the slotResponseReceived() slot when processing followed by clean-up is done.

When the timeout happens, the timer started when the post was made trigger the slotTimeout(), aborts the post. This triggers the finished() signal form the QNetworkAccessManagers and the clean-up happens. Until this point everything works. The problem is that subsequent posts will start accumulating more and more memory.

The system is running windows 7 with Qt 5.3.1 and open-ssl 1.0.1q.

I have no idea on how to skin this one, would appreciate any idea.

bool Connection::Send(const RelayPayload& payload)
{
  if(m_wait_on_receipt.available()>0)
    m_wait_on_receipt.acquire(m_wait_on_receipt.available());

  QMutexLocker locker(&m_send_mutex);

  QByteArray to_send;

//Put some data into the array

  QBuffer* buffer_to_send = new QBuffer;
  buffer_to_send->setData(to_send);
  buffer_to_send->open(QIODevice::ReadOnly);

  QUrl url;
  //add URL

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

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

  if(m_is_error==COM_OK)
    return true; //we got a receipt

  return false;
}

void Connection::slotResponseReceived(QNetworkReply* reply)
{
  //in the Network manager thread
  m_is_error = ERROR;

  if(reply!=nullptr)
  {
    //process response
    m_is_error = COM_OK;
    reply->manager()->deleteLater();
    reply->deleteLater();
  }

   m_wait_on_receipt.release(1);
}

void SendPrivate::slotSend(QUrl url, QIODevice* 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());
  }

  QNetworkAccessManager *manager = new QNetworkAccessManager();
  connect(manager, SIGNAL(finished(QNetworkReply*)), this, SLOT(slotResponseReceived(QNetworkReply*)));

  m_reply = manager->post(upload_req, data);
  data->setParent(m_reply);

  connect(m_reply, SIGNAL(error(QNetworkReply::NetworkError)), SLOT(slotError(QNetworkReply::NetworkError)));
  
  m_timeout_timer.start();
}

void PostPrivate::slotTimeout()
{
  m_reply->abort();
}

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?

Important: Please read the Qt Code of Conduct - https://forum.qt.io/topic/113070/qt-code-of-conduct

Excessive memory use in QNetworkAccessManager after network disruption