Skip to content
  • Categories
  • Recent
  • Tags
  • Popular
  • Users
  • Groups
  • Search
  • Get Qt Extensions
  • Unsolved
Collapse
Brand Logo
  1. Home
  2. Qt Development
  3. General and Desktop
  4. Executing QProcess in QThread: memory leak
Forum Updated to NodeBB v4.3 + New Features

Executing QProcess in QThread: memory leak

Scheduled Pinned Locked Moved Unsolved General and Desktop
79 Posts 7 Posters 21.1k Views 2 Watching
  • 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.
  • K KroMignon
    19 Nov 2020, 11:14

    @JonB said in Executing QProcess in QThread: memory leak:

    I would not write production (or even development) code like this, and certainly not for distribution. It is asking for an "unseen hang" to occur, one day. I would at minimum hook onto QProcess::errorOccurred, maybe stateChanged() too. And I would put in some sort of timer/timeout, so that if something goes badly wrong you get out of the blocking loop (with perhaps an error flag) instead of waiting for Hell to freeze over....

    Yes, this is a good advice.

    S Offline
    S Offline
    sitesv
    wrote on 19 Nov 2020, 11:38 last edited by sitesv
    #64

    @JonB said in Executing QProcess in QThread: memory leak:

    You are writing code which will call QEventLoop::exec(), blocking whatever calls it. It relies on hitting the QEventLoop::exit() statement, which you only have in response to the QProcess::finished signal. If for whatever reason that does not get hit, your event loop will never be exited.

    Maybe my first variant was good (QThread + QProcess)?

    if(!myProcess) myProcess = new QProcess(this);
    myProcess->start(exe_path, arguments);
    myProcess->waitForFinished(500);
    output = myProcess->readAll();
    output_str = codec->toUnicode(output);
    output_strlst = output_str.split("\r\n");
    myProcess->close();
    ...
    

    There is QProcess "waitForFinished" with timeout.
    The app wasn't freezing with this approach.

    K J 2 Replies Last reply 19 Nov 2020, 11:46
    0
    • S sitesv
      19 Nov 2020, 11:38

      @JonB said in Executing QProcess in QThread: memory leak:

      You are writing code which will call QEventLoop::exec(), blocking whatever calls it. It relies on hitting the QEventLoop::exit() statement, which you only have in response to the QProcess::finished signal. If for whatever reason that does not get hit, your event loop will never be exited.

      Maybe my first variant was good (QThread + QProcess)?

      if(!myProcess) myProcess = new QProcess(this);
      myProcess->start(exe_path, arguments);
      myProcess->waitForFinished(500);
      output = myProcess->readAll();
      output_str = codec->toUnicode(output);
      output_strlst = output_str.split("\r\n");
      myProcess->close();
      ...
      

      There is QProcess "waitForFinished" with timeout.
      The app wasn't freezing with this approach.

      K Offline
      K Offline
      KroMignon
      wrote on 19 Nov 2020, 11:46 last edited by
      #65

      @sitesv said in Executing QProcess in QThread: memory leak:

      There is QProcess "waitForFinished" with timeout.
      The app wasn't freezing with this approach.

      Why not, but you have to wait ping finished before starting next.
      If it is what you want, then go with it.

      It is an old maxim of mine that when you have excluded the impossible, whatever remains, however improbable, must be the truth. (Sherlock Holmes)

      1 Reply Last reply
      0
      • S sitesv
        19 Nov 2020, 11:38

        @JonB said in Executing QProcess in QThread: memory leak:

        You are writing code which will call QEventLoop::exec(), blocking whatever calls it. It relies on hitting the QEventLoop::exit() statement, which you only have in response to the QProcess::finished signal. If for whatever reason that does not get hit, your event loop will never be exited.

        Maybe my first variant was good (QThread + QProcess)?

        if(!myProcess) myProcess = new QProcess(this);
        myProcess->start(exe_path, arguments);
        myProcess->waitForFinished(500);
        output = myProcess->readAll();
        output_str = codec->toUnicode(output);
        output_strlst = output_str.split("\r\n");
        myProcess->close();
        ...
        

        There is QProcess "waitForFinished" with timeout.
        The app wasn't freezing with this approach.

        J Offline
        J Offline
        jsulm
        Lifetime Qt Champion
        wrote on 20 Nov 2020, 06:46 last edited by
        #66

        @sitesv I'm still wondering why you think you need a local event loop...

        https://forum.qt.io/topic/113070/qt-code-of-conduct

        S 1 Reply Last reply 20 Nov 2020, 09:38
        1
        • J jsulm
          20 Nov 2020, 06:46

          @sitesv I'm still wondering why you think you need a local event loop...

          S Offline
          S Offline
          sitesv
          wrote on 20 Nov 2020, 09:38 last edited by
          #67

          @jsulm said in Executing QProcess in QThread: memory leak:

          @sitesv I'm still wondering why you think you need a local event loop...

          Hi! I have done variant as you recommend. It's working!

          void PingTester::doPing(){
              QStringList ip_list = {"192.168.0.1", "192.168.0.2"};
              int success_count = 0;
              int pingsToDo = ip_list.count();
              int ipCnt = pingsToDo;
              foreach(auto ip, ip_list)
              {
                  QProcess ping;
                  ping.setProcessChannelMode(QProcess::MergedChannels);
                  connect(&ping,
                          QOverload<int, QProcess::ExitStatus>::of(&QProcess::finished),
                          [this, &ping, &success_count, &pingsToDo, ipCnt]() {
                              --pingsToDo;
                              QString output(ping.readAll());
                              if(output.contains("ttl",Qt::CaseInsensitive)){
                                  success_count++;
                              }
                              if(!pingsToDo){
                                  if(success_count == ipCnt) emit setStatus(true);
                                  else                       emit setStatus(false);
                                  m_timer->start();
                              }
                          });
                  ping.start("/bin/ping", QStringList() << ip << "-c" << "1" << "-w" << "1");
              }
          }
          

          I made another experiment:

          1. Created a QThread
          2. In QThread::run() method::
          • made a QTimer* object,
          • set him as "one shot kind",
          • ran QTimer, and in final
            * ran QThread::exec() (for run local EventLoop)...
          1. There is timer->start() in timeout slot, It executed, but QTimer doesn't start again.

          Any ideas?

          J K 2 Replies Last reply 20 Nov 2020, 09:44
          0
          • S sitesv
            20 Nov 2020, 09:38

            @jsulm said in Executing QProcess in QThread: memory leak:

            @sitesv I'm still wondering why you think you need a local event loop...

            Hi! I have done variant as you recommend. It's working!

            void PingTester::doPing(){
                QStringList ip_list = {"192.168.0.1", "192.168.0.2"};
                int success_count = 0;
                int pingsToDo = ip_list.count();
                int ipCnt = pingsToDo;
                foreach(auto ip, ip_list)
                {
                    QProcess ping;
                    ping.setProcessChannelMode(QProcess::MergedChannels);
                    connect(&ping,
                            QOverload<int, QProcess::ExitStatus>::of(&QProcess::finished),
                            [this, &ping, &success_count, &pingsToDo, ipCnt]() {
                                --pingsToDo;
                                QString output(ping.readAll());
                                if(output.contains("ttl",Qt::CaseInsensitive)){
                                    success_count++;
                                }
                                if(!pingsToDo){
                                    if(success_count == ipCnt) emit setStatus(true);
                                    else                       emit setStatus(false);
                                    m_timer->start();
                                }
                            });
                    ping.start("/bin/ping", QStringList() << ip << "-c" << "1" << "-w" << "1");
                }
            }
            

            I made another experiment:

            1. Created a QThread
            2. In QThread::run() method::
            • made a QTimer* object,
            • set him as "one shot kind",
            • ran QTimer, and in final
              * ran QThread::exec() (for run local EventLoop)...
            1. There is timer->start() in timeout slot, It executed, but QTimer doesn't start again.

            Any ideas?

            J Offline
            J Offline
            JonB
            wrote on 20 Nov 2020, 09:44 last edited by JonB
            #68

            @sitesv
            I don't know how/whether your issue relates this, but in your code: you set off ping.start(), but QProcess ping; is a local variable in your foreach loop and so immediately goes out of scope (not to mention, you also re-use the same local variable for each time round the loop, overwriting/destroying the previous one). Nothing should work (it might actually "crash"), I don't understand how you say it does.

            On top of all of this: I think we've said it already here above, but goodness only knows why you are using a thread, with all the complications that involves? If you want to run a QProcess and the main thread to know when it's finished, it's asynchronous anyway, it would be a whole lot simpler not to have any thread. I think I said this earlier, but up to you.

            And finally, while I'm on a roll: I think you are just using a /bin/ping in order to read the textual output, parse it, and see whether something is there/alive. In which case I'd be tempted to just write it myself in Qt instead of running some external command, I would have thought it's only a few lines of code. Your "parsing" of the output is beyond hokey, not even sure what you think it tells you....

            1 Reply Last reply
            0
            • S sitesv
              20 Nov 2020, 09:38

              @jsulm said in Executing QProcess in QThread: memory leak:

              @sitesv I'm still wondering why you think you need a local event loop...

              Hi! I have done variant as you recommend. It's working!

              void PingTester::doPing(){
                  QStringList ip_list = {"192.168.0.1", "192.168.0.2"};
                  int success_count = 0;
                  int pingsToDo = ip_list.count();
                  int ipCnt = pingsToDo;
                  foreach(auto ip, ip_list)
                  {
                      QProcess ping;
                      ping.setProcessChannelMode(QProcess::MergedChannels);
                      connect(&ping,
                              QOverload<int, QProcess::ExitStatus>::of(&QProcess::finished),
                              [this, &ping, &success_count, &pingsToDo, ipCnt]() {
                                  --pingsToDo;
                                  QString output(ping.readAll());
                                  if(output.contains("ttl",Qt::CaseInsensitive)){
                                      success_count++;
                                  }
                                  if(!pingsToDo){
                                      if(success_count == ipCnt) emit setStatus(true);
                                      else                       emit setStatus(false);
                                      m_timer->start();
                                  }
                              });
                      ping.start("/bin/ping", QStringList() << ip << "-c" << "1" << "-w" << "1");
                  }
              }
              

              I made another experiment:

              1. Created a QThread
              2. In QThread::run() method::
              • made a QTimer* object,
              • set him as "one shot kind",
              • ran QTimer, and in final
                * ran QThread::exec() (for run local EventLoop)...
              1. There is timer->start() in timeout slot, It executed, but QTimer doesn't start again.

              Any ideas?

              K Offline
              K Offline
              KroMignon
              wrote on 20 Nov 2020, 09:48 last edited by
              #69

              @sitesv said in Executing QProcess in QThread: memory leak:

              Hi! I have done variant as you recommend. It's working!

              I am not sure this is really working!
              I think you have to (re)learn C++ object life cycle.
              You create a QProcess local instance in the for loop, this object will be destroyed at loop end.

              It is an old maxim of mine that when you have excluded the impossible, whatever remains, however improbable, must be the truth. (Sherlock Holmes)

              1 Reply Last reply
              1
              • S Offline
                S Offline
                sitesv
                wrote on 20 Nov 2020, 09:51 last edited by sitesv
                #70

                @JonB @KroMignon
                Agree with you, guys.
                But it works...
                I can reimplement to QProcess pointers of PingTester class...

                K 1 Reply Last reply 20 Nov 2020, 10:00
                0
                • S sitesv
                  20 Nov 2020, 09:51

                  @JonB @KroMignon
                  Agree with you, guys.
                  But it works...
                  I can reimplement to QProcess pointers of PingTester class...

                  K Offline
                  K Offline
                  KroMignon
                  wrote on 20 Nov 2020, 10:00 last edited by
                  #71

                  @sitesv said in Executing QProcess in QThread: memory leak:

                  But it works...

                  It don't, "it seems to work" would be the correct answer ;)

                  If you try to ping a not valid/accessible IP address, I am pretty sure it will not work.

                  If you want to do it sequentially, you have to be consistent in your choice:

                  foreach(auto ip, ip_list)
                  {
                      QProcess ping;
                      ping.setProcessChannelMode(QProcess::MergedChannels);
                      ping.start("/bin/ping", QStringList() << ip << "-c" << "1" << "-w" << "1");
                      ping.waitForFinished(5000); // wait up to 5 seconds
                      QString output(ping.readAll());
                      if(output.contains("ttl",Qt::CaseInsensitive)){
                          success_count++;
                      }
                  }
                  

                  It is an old maxim of mine that when you have excluded the impossible, whatever remains, however improbable, must be the truth. (Sherlock Holmes)

                  S J 2 Replies Last reply 20 Nov 2020, 10:03
                  0
                  • K KroMignon
                    20 Nov 2020, 10:00

                    @sitesv said in Executing QProcess in QThread: memory leak:

                    But it works...

                    It don't, "it seems to work" would be the correct answer ;)

                    If you try to ping a not valid/accessible IP address, I am pretty sure it will not work.

                    If you want to do it sequentially, you have to be consistent in your choice:

                    foreach(auto ip, ip_list)
                    {
                        QProcess ping;
                        ping.setProcessChannelMode(QProcess::MergedChannels);
                        ping.start("/bin/ping", QStringList() << ip << "-c" << "1" << "-w" << "1");
                        ping.waitForFinished(5000); // wait up to 5 seconds
                        QString output(ping.readAll());
                        if(output.contains("ttl",Qt::CaseInsensitive)){
                            success_count++;
                        }
                    }
                    
                    S Offline
                    S Offline
                    sitesv
                    wrote on 20 Nov 2020, 10:03 last edited by
                    #72

                    @KroMignon said in Executing QProcess in QThread: memory leak:

                    If you try to ping a not valid/accessible IP address, I am pretty sure it will not work.

                    There is "-w 1" key. It helps with unaccessible IP.

                    K 1 Reply Last reply 20 Nov 2020, 10:07
                    0
                    • K KroMignon
                      20 Nov 2020, 10:00

                      @sitesv said in Executing QProcess in QThread: memory leak:

                      But it works...

                      It don't, "it seems to work" would be the correct answer ;)

                      If you try to ping a not valid/accessible IP address, I am pretty sure it will not work.

                      If you want to do it sequentially, you have to be consistent in your choice:

                      foreach(auto ip, ip_list)
                      {
                          QProcess ping;
                          ping.setProcessChannelMode(QProcess::MergedChannels);
                          ping.start("/bin/ping", QStringList() << ip << "-c" << "1" << "-w" << "1");
                          ping.waitForFinished(5000); // wait up to 5 seconds
                          QString output(ping.readAll());
                          if(output.contains("ttl",Qt::CaseInsensitive)){
                              success_count++;
                          }
                      }
                      
                      J Offline
                      J Offline
                      JonB
                      wrote on 20 Nov 2020, 10:05 last edited by JonB
                      #73

                      @KroMignon
                      I would think doing it sequentially, with waitForFinished(), for his multiple IP addresses would be a poor way to do things. He has a number of IP addresses to check, these should be done in parallel....

                      1 Reply Last reply
                      0
                      • S sitesv
                        20 Nov 2020, 10:03

                        @KroMignon said in Executing QProcess in QThread: memory leak:

                        If you try to ping a not valid/accessible IP address, I am pretty sure it will not work.

                        There is "-w 1" key. It helps with unaccessible IP.

                        K Offline
                        K Offline
                        KroMignon
                        wrote on 20 Nov 2020, 10:07 last edited by
                        #74

                        @sitesv said in Executing QProcess in QThread: memory leak:

                        There is "-w 1" key. It helps with unaccessible IP.

                        AFAIK "-w 1" will wait up to 1 second.
                        I am sure you will got QProcess warnings about killing a process which is still running.

                        It is an old maxim of mine that when you have excluded the impossible, whatever remains, however improbable, must be the truth. (Sherlock Holmes)

                        S 1 Reply Last reply 20 Nov 2020, 10:15
                        0
                        • K KroMignon
                          20 Nov 2020, 10:07

                          @sitesv said in Executing QProcess in QThread: memory leak:

                          There is "-w 1" key. It helps with unaccessible IP.

                          AFAIK "-w 1" will wait up to 1 second.
                          I am sure you will got QProcess warnings about killing a process which is still running.

                          S Offline
                          S Offline
                          sitesv
                          wrote on 20 Nov 2020, 10:15 last edited by sitesv
                          #75

                          @KroMignon said in Executing QProcess in QThread: memory leak:

                          AFAIK "-w 1" will wait up to 1 second.

                          ...and QProcess will emit a "finished" signal. Why you are writing about "killing" process?

                          K 1 Reply Last reply 20 Nov 2020, 10:21
                          0
                          • S sitesv
                            20 Nov 2020, 10:15

                            @KroMignon said in Executing QProcess in QThread: memory leak:

                            AFAIK "-w 1" will wait up to 1 second.

                            ...and QProcess will emit a "finished" signal. Why you are writing about "killing" process?

                            K Offline
                            K Offline
                            KroMignon
                            wrote on 20 Nov 2020, 10:21 last edited by KroMignon
                            #76

                            @sitesv said in Executing QProcess in QThread: memory leak:

                            ...and QProcess will emit a "finished" signal. Why you are writing about "killing" process?

                            This is the way I would implement multiple pings in parallel:

                            int PingTester::doPing(const QStringList &ip_list)
                            {
                                // to avoid issues ;)
                                if(ip_list.isEmpty())
                                    return 0;
                            
                                int success_count = 0;
                                int pingsToDo = ip_list.count();
                                QEventLoop l;
                                QTimer timer;
                                foreach(auto ip, ip_list)
                                {
                                    auto ping = new QProcess();
                                    ping->setProcessChannelMode(QProcess::MergedChannels);
                                    connect(timer, &QTimer::timeout, ping, &QProcess::kill);
                                    connect(ping, &QProcess::stateChanged,
                                            [&l, ping, &success_count, &pingsToDo](QProcess::ProcessState newState) {
                                                if(newState != QProcess::NotRunning)
                                                    return;
                                                --pingsToDo;
                                                QString output(ping->readAll());
                                                if(output.contains("ttl",Qt::CaseInsensitive))
                                                    success_count++;
                                                // free memory
                                                ping->deleteLater();
                                                // exit loop when done.
                                                if(!pingsToDo)
                                                    l.exit();
                                            });
                                    ping->start("/bin/ping", QStringList() << ip << "-c" << "1" << "-w" << "1");
                                }
                                timer.start(1500); // kill process after 1.5 seconds if still running
                                // wait all pings done
                                l.exec();
                                return success_count;
                            }
                            

                            It is an old maxim of mine that when you have excluded the impossible, whatever remains, however improbable, must be the truth. (Sherlock Holmes)

                            S 1 Reply Last reply 20 Nov 2020, 11:32
                            1
                            • S Offline
                              S Offline
                              sitesv
                              wrote on 20 Nov 2020, 10:34 last edited by sitesv
                              #77

                              @jsulm There is a local QEventLoop...
                              @KroMignon I really don't understand why you are checking state changing of QProcess...
                              QProcess "Finished" method is not suitable?
                              And why kill a process?

                              K 1 Reply Last reply 20 Nov 2020, 10:43
                              0
                              • S sitesv
                                20 Nov 2020, 10:34

                                @jsulm There is a local QEventLoop...
                                @KroMignon I really don't understand why you are checking state changing of QProcess...
                                QProcess "Finished" method is not suitable?
                                And why kill a process?

                                K Offline
                                K Offline
                                KroMignon
                                wrote on 20 Nov 2020, 10:43 last edited by KroMignon
                                #78

                                @sitesv said in Executing QProcess in QThread: memory leak:

                                QProcess "Finished" method is not suitable?

                                I found it easier to use QProcess::stateChanged(), because there is no overload of this slot, and I am not sure if QProcess::finished(int, QProcess::ExitStatus) is triggered if process is killed.

                                And why kill a process?

                                This is a security, when calling an external application, many things can happen: execution right issues, application not exist, network issues and so on.
                                99.9% of time it will not be useful, but it will ensure QEventLoop will exit at the end!

                                It is an old maxim of mine that when you have excluded the impossible, whatever remains, however improbable, must be the truth. (Sherlock Holmes)

                                1 Reply Last reply
                                0
                                • K KroMignon
                                  20 Nov 2020, 10:21

                                  @sitesv said in Executing QProcess in QThread: memory leak:

                                  ...and QProcess will emit a "finished" signal. Why you are writing about "killing" process?

                                  This is the way I would implement multiple pings in parallel:

                                  int PingTester::doPing(const QStringList &ip_list)
                                  {
                                      // to avoid issues ;)
                                      if(ip_list.isEmpty())
                                          return 0;
                                  
                                      int success_count = 0;
                                      int pingsToDo = ip_list.count();
                                      QEventLoop l;
                                      QTimer timer;
                                      foreach(auto ip, ip_list)
                                      {
                                          auto ping = new QProcess();
                                          ping->setProcessChannelMode(QProcess::MergedChannels);
                                          connect(timer, &QTimer::timeout, ping, &QProcess::kill);
                                          connect(ping, &QProcess::stateChanged,
                                                  [&l, ping, &success_count, &pingsToDo](QProcess::ProcessState newState) {
                                                      if(newState != QProcess::NotRunning)
                                                          return;
                                                      --pingsToDo;
                                                      QString output(ping->readAll());
                                                      if(output.contains("ttl",Qt::CaseInsensitive))
                                                          success_count++;
                                                      // free memory
                                                      ping->deleteLater();
                                                      // exit loop when done.
                                                      if(!pingsToDo)
                                                          l.exit();
                                                  });
                                          ping->start("/bin/ping", QStringList() << ip << "-c" << "1" << "-w" << "1");
                                      }
                                      timer.start(1500); // kill process after 1.5 seconds if still running
                                      // wait all pings done
                                      l.exec();
                                      return success_count;
                                  }
                                  
                                  S Offline
                                  S Offline
                                  sitesv
                                  wrote on 20 Nov 2020, 11:32 last edited by sitesv
                                  #79

                                  @KroMignon said in Executing QProcess in QThread: memory leak:

                                  This is the way I would implement multiple pings in parallel:

                                  This code doesn't work. Only one iteration.

                                  1 Reply Last reply
                                  0

                                  73/79

                                  20 Nov 2020, 10:05

                                  • Login

                                  • Login or register to search.
                                  73 out of 79
                                  • First post
                                    73/79
                                    Last post
                                  0
                                  • Categories
                                  • Recent
                                  • Tags
                                  • Popular
                                  • Users
                                  • Groups
                                  • Search
                                  • Get Qt Extensions
                                  • Unsolved