Qt Forum

    • Login
    • Search
    • Categories
    • Recent
    • Tags
    • Popular
    • Users
    • Groups
    • Search
    • Unsolved

    Update: Forum Guidelines & Code of Conduct


    Qt World Summit: Early-Bird Tickets

    Solved QNetworkAccessManager seems to leak with every retry

    General and Desktop
    qnetworkaccessm networking memory
    4
    10
    1520
    Loading More Posts
    • Oldest to Newest
    • Newest to Oldest
    • Most Votes
    Reply
    • Reply as topic
    Log in to reply
    This topic has been deleted. Only users with topic management privileges can see it.
    • S
      Smaankers last edited by

      Hi,

      I am implementing a client using QNetworkAccessManager. When the signal finished is received and the reply indicates an error, a timer is started and the reply marked for deletion. When the timer elapses, a retry is performed.

      For testing purposes, I have set the retry timer to a very short amount of time (5 ms to shorten test time). To make sure there are no memory leaks when retrying, I am using a script on the commandline to watch the available memory: free -m | awk '/Mem:/{print $4}'

      When I run the application with the network disconnected, I can see the free memory dropping about 1 megabyte per second. Even though retrying every 5 milliseconds is not what you would typically do, it does seem to demonstrate something is wrong.

      My questions are:

      • Am I doing something wrong in the code or in my testing?
      • What can I do so fix either the code or the testing?

      Hardware: Lenovo Thinkpad w/ Intel(R) Core(TM) i5-8250U
      System: Linux Mint,
      uname -a: Linux 4.10.0-38-generic #42~16.04.1-Ubuntu SMP Tue Oct 10 16:32:20 UTC 2017 x86_64 x86_64 x86_64 GNU/Linux
      Qt version: 5.6.3
      GCC: gcc (Ubuntu 5.4.0-6ubuntu1~16.04.10) 5.4.0 20160609
      G++: g++ (Ubuntu 5.4.0-6ubuntu1~16.04.10) 5.4.0 20160609

      Below is the code of the test. This version of the code was stripped down to make it easier to grasp, and it does show the behavior described above.

      int main(int argc, char *argv[])
      {
        QCoreApplication app (argc, argv);
      
        ::testing::InitGoogleTest (&(argc), (argv));
        RUN_ALL_TESTS();
      }
      
      TEST(SmsNotifierStressTest, GIVEN_notifier_WHEN_notify_messages_THEN_successfully_sends_message)
      {
          // initialization
          Sms_notifier notifier(true, 5);
      
          // state
          bool completion_handler_called = false;
          bool retry_handler_called = false;
      
          notifier.notify(
              "+0123456789",
                "Lorem Ipsum is simply dummy text of the printing and typesetting industry. ",
                [&completion_handler_called](Notification::SMS::Response_code){
                    completion_handler_called = true;
                },
                [&retry_handler_called](){
                    retry_handler_called = true;
                });
      
          qWarning() << "done, looping";
      
          while(true)
          {
            QCoreApplication::exec();
          }
      }
      

      And this is the code of the client:

      QSemaphore Sms_notifier::queue_count(0);
      
      Sms_notifier::Sms_notifier(bool test, int interval_length_milliseconds)
      
          :  QObject                       (NULL)
            ,m_test                        (test)
            ,m_interval_length_milliseconds(interval_length_milliseconds)
            ,m_manager                     ()
            ,m_timer                       ()
            ,m_addressee()
            ,m_payload()
            ,m_completion_handler()
            ,m_retry_handler()
      {
        m_timer.setSingleShot(true);
      
        QObject::connect(&m_manager, &QNetworkAccessManager::finished,
                         this,       &Sms_notifier::on_nam_finished,
                         Qt::QueuedConnection);
      
        QObject::connect(&m_timer, &QTimer::timeout,
                         this,     &Sms_notifier::on_timer_elapsed,
                         Qt::QueuedConnection);
      
      }
      
      Sms_notifier::~Sms_notifier()
      {
      }
      
      bool Sms_notifier::notify(
          const std::string addressee,
          const std::string payload,
          const std::function<void(Notification::SMS::Response_code)> completion_handler,
          const std::function<void()> retry_handler
      )
      {
          this->queue_count.release();
      
          m_retry_handler      = retry_handler;
          m_completion_handler = completion_handler;
          m_addressee          = addressee;
          m_payload            = payload;
      
          return notify();
      }
      
      int Sms_notifier::get_queue_count()
      {
        return queue_count.available();
      }
      
      bool Sms_notifier::notify()
      {
          // set up API query
          QUrlQuery params = prepare_request_parameters(m_addressee, m_payload, m_test);
          QUrl request_url(QString::fromStdString(Notification::SMS::SMS_API_URL));
          QNetworkRequest request(request_url);
          request.setHeader(QNetworkRequest::ContentTypeHeader, "application/x-www-form-urlencoded");
      
          // ssl configuration
          QSslConfiguration configuration = request.sslConfiguration();
          configuration.setProtocol(QSsl::TlsV1_2OrLater);
          request.setSslConfiguration(configuration);
      
          // make the API call
          this->m_manager.post(request, params.toString(QUrl::FullyEncoded).toUtf8());
      
          return false;
      }
      
      void Sms_notifier::on_nam_finished(QNetworkReply* reply)
      {
          if (reply->error() != QNetworkReply::NetworkError::NoError)
          {
              if (m_retry_handler)
              {
                  m_retry_handler();
              }
      
              m_timer.start(m_interval_length_milliseconds);
          }
          else
          {
              queue_count.acquire();
      
              // call completion handler with response code
              if (m_completion_handler)
              {
                  int response_code_number = (QString(reply->readAll())).mid(0, 3).toInt();
                  auto response_code       = static_cast<Notification::SMS::Response_code>(response_code_number);
      
                  m_completion_handler(response_code);
              }
      
              m_completion_handler = nullptr;
              m_retry_handler      = nullptr;
              m_addressee.clear();
              m_payload.clear();
          }
      
          reply->deleteLater();
      }
      
      void Sms_notifier::on_timer_elapsed()
      {
        notify();
      }
      
      QUrlQuery Sms_notifier::prepare_request_parameters(
          const std::string& addressee,
          const std::string& payload,
          bool test
      )
      {
          QUrlQuery params;
      
          params.addQueryItem("UID", "someid"));
          
          // ... some more params here
      
          return params;
      }
      
      
      Pablo J. Rogina 1 Reply Last reply Reply Quote 0
      • Pablo J. Rogina
        Pablo J. Rogina @Smaankers last edited by

        @Smaankers I guess you need to change

        // make the API call
        this->m_manager.post(request, params.toString(QUrl::FullyEncoded).toUtf8());

        into

        QNetworkReply *reply = this->m_manager.post(request, params.toString(QUrl::FullyEncoded).toUtf8());
        

        Upvote the answer(s) that helped you solve the issue
        Use "Topic Tools" button to mark your post as Solved
        Add screenshots via postimage.org
        Don't ask support requests via chat/PM. Please use the forum so others can benefit from the solution in the future

        S 1 Reply Last reply Reply Quote 0
        • S
          Smaankers @Pablo J. Rogina last edited by

          @Pablo-J.-Rogina

          Thank you for your answer.

          I do not understand how saving the pointer and then not using it would change any behavior. As far as I can tell, the compiler would probably consider it as an unused variable and optimize it away.

          Can you please explain what behavior you would expect to be different?

          Pablo J. Rogina 1 Reply Last reply Reply Quote 1
          • VRonin
            VRonin last edited by

            @Smaankers said in QNetworkAccessManager seems to leak with every retry:

            Am I doing something wrong in the code or in my testing?

            Is this all your code?
            Are you using multithreading?
            Are you blocking the event loop somewhere else in your code?

            "La mort n'est rien, mais vivre vaincu et sans gloire, c'est mourir tous les jours"
            ~Napoleon Bonaparte

            On a crusade to banish setIndexWidget() from the holy land of Qt

            S 1 Reply Last reply Reply Quote 1
            • Pablo J. Rogina
              Pablo J. Rogina @Smaankers last edited by

              @Smaankers said in QNetworkAccessManager seems to leak with every retry:

              Can you please explain what behavior you would expect to be different?

              I expect that Qt framework can handle the pointer/memory management right now that there's a pointer assigned and it is not lost as you did previously.

              Upvote the answer(s) that helped you solve the issue
              Use "Topic Tools" button to mark your post as Solved
              Add screenshots via postimage.org
              Don't ask support requests via chat/PM. Please use the forum so others can benefit from the solution in the future

              JonB 1 Reply Last reply Reply Quote 0
              • JonB
                JonB @Pablo J. Rogina last edited by JonB

                @Pablo-J.-Rogina

                this->m_manager.post(request, params.toString(QUrl::FullyEncoded).toUtf8());
                

                versus

                QNetworkReply *reply = this->m_manager.post(request, params.toString(QUrl::FullyEncoded).toUtf8());
                

                Changing the line in the OP's code to assign to a (pointer) local variable, which is then not used, will not trigger anything in the Qt framework. It's just as "lost" with that code as the original --- it's purely a compiler issue. Unless you mean the OP to use that variable, or keep it somewhere and later use it.

                P.S.
                Though if I were @Smaankers I'd spend 5 seconds to try it, just in case... :) I don't know if compilers will warn on unused variable....

                S 1 Reply Last reply Reply Quote 1
                • S
                  Smaankers @VRonin last edited by Smaankers

                  @VRonin

                  "Is this all your code?"
                  Yes, this is all the code. I stripped it down to the minimum to make sure there was nothing else causing the leak.

                  "Are you using multithreading?"
                  No, there is just the main thread that you see in the code.

                  "Are you blocking the event loop somewhere else in your code?"
                  No, at the end of the test it executes the event loop without any interruptions.

                  1 Reply Last reply Reply Quote 0
                  • S
                    Smaankers @JonB last edited by

                    @JonB

                    "Changing the line in the OP's code to assign to a (pointer) local variable, which is then not used, will not trigger anything in the Qt framework."

                    That was exactly my thinking.

                    "Though if I were @Smaankers I'd spend 5 seconds to try it, just in case... :)"

                    I did, but there is no change in behavior.

                    JonB 1 Reply Last reply Reply Quote 1
                    • JonB
                      JonB @Smaankers last edited by JonB

                      @Smaankers
                      :)

                      Now, I'm not a Qt C++-er, I'm a PyQt Pythonista, so (given its auto reference counting) I don't get your memory leaks or have to think about freeing objects....

                      Following on from the theme of @Pablo-J-Rogina's post/concern/suggestion:

                      • Each time round your timer you set off a fresh notify(); call.

                      • That does a new this->m_manager.post(request, params.toString(QUrl::FullyEncoded).toUtf8());.

                      • http://doc.qt.io/qt-5/qnetworkaccessmanager.html#post says

                      Sends an HTTP POST request to the destination specified by request and returns a new QNetworkReply object opened for reading that will contain the reply sent by the server.

                      • Note the word "new". I'm thinking/wondering whether that doesn't mean you are responsible for freeing that object, or what do you think will do so for you?

                      • You would free your reply object in on_nam_finished, but that's where you do your retry, and start off the timer again....

                      I'm getting a bit lost, but are you thereby leaking a QNetworkReply object each time? Can you verify that for each post() made the corresponding QNetworkReply returned is indeed deleted?

                      1 Reply Last reply Reply Quote 0
                      • S
                        Smaankers last edited by Smaankers

                        Okay I found the culprit in an unexpected corner.

                        I decided to completely strip it to the absolute bare minimum:

                        int main(int argc, char *argv[])
                        {
                          QCoreApplication app (argc, argv);
                        
                          // initialization
                          Sms_notifier notifier(true, 5);
                        
                          notifier.notify(
                              "+0123456789",
                              "Lorem Ipsum is simply dummy text of the printing and typesetting industry. ");
                        
                          qWarning() << "done, looping";
                        
                          while(true)
                          {
                            QCoreApplication::exec();
                          }
                        
                        }
                        
                        Sms_notifier::Sms_notifier(bool test, int interval_length_milliseconds) 
                         
                            :  QObject                       (NULL) 
                              ,m_test                        (test) 
                              ,m_interval_length_milliseconds(interval_length_milliseconds) 
                              ,m_manager                     () 
                              ,m_timer                       () 
                              ,m_addressee() 
                              ,m_payload() 
                        { 
                          m_timer.setSingleShot(true); 
                         
                          QObject::connect(&m_manager, &QNetworkAccessManager::finished, 
                                           this,       &Sms_notifier::on_nam_finished); 
                         
                          QObject::connect(&m_timer, &QTimer::timeout, 
                                           this,     &Sms_notifier::on_timer_elapsed); 
                         
                        } 
                         
                        Sms_notifier::~Sms_notifier() 
                        { 
                        } 
                         
                        bool Sms_notifier::notify( 
                            const std::string addressee, 
                            const std::string payload 
                        ) 
                        { 
                            m_addressee = addressee; 
                            m_payload   = payload; 
                         
                            return notify(); 
                        } 
                         
                        bool Sms_notifier::notify() 
                        { 
                            QNetworkRequest request; 
                            QByteArray data; 
                        
                            m_manager.post(request, data); 
                            return false; 
                        } 
                         
                        void Sms_notifier::on_nam_finished(QNetworkReply* reply) 
                        { 
                            QNetworkReply::NetworkError error = reply->error(); 
                            reply->deleteLater(); 
                         
                            if (error != QNetworkReply::NetworkError::NoError) 
                            { 
                                m_timer.start(m_interval_length_milliseconds); 
                            } 
                            else 
                            { 
                                qWarning() << "success"; 
                         
                                m_addressee.clear(); 
                                m_payload.clear(); 
                            } 
                        } 
                         
                        void Sms_notifier::on_timer_elapsed() 
                        { 
                          notify(); 
                        } 
                        

                        It turns out it was still leaking while there was no network. So I stripped away all libraries that were linked, and it still leaked.

                        Eventually my eye struck this in the .pro file:

                        QMAKE_CXXFLAGS += -fsanitize=address
                        QMAKE_CFLAGS   += -fsanitize=address
                        QMAKE_LFLAGS   += -fsanitize=address -lasan
                        

                        This was added to detect memory leaks and report them when the application quits. After removing this, the excessive "memory leak" was gone.

                        I am assuming that the address sanitizer allocates memory for each allocation done by the application, for its own administration purposes. I suspect that when the application releases the allocated memory, the sanitizer holds on to the respective administration data until the application quits. This could explain why when I remove the sanitizer it also removes the leak.

                        Well, thanks everyone for your input!

                        1 Reply Last reply Reply Quote 1
                        • First post
                          Last post