Memory leak using QThread issue



  • Hi! I have made a changelog feature to my application. I have created worker class and connect all signals and slots with QThread. The problem is when I start a thread to update data more then one time it begin to leak RAM memory.

    Code:

     appCheckUpdate = new CheckUpdates();
     appCheckUpdatesThread = new QThread();
     appCheckUpdate->moveToThread(appCheckUpdatesThread);
     connect(appCheckUpdatesThread, &QThread::started, appCheckUpdate, &CheckUpdates::checkUpdate);
     connect(appCheckUpdate, &CheckUpdates::updateData, this, &Changelog::getUpdateData);
     connect(appCheckUpdate, &CheckUpdates::networkError, this, &Changelog::changeLogUpdateError);
     connect(appCheckUpdate, &CheckUpdates::localeError, this, &Changelog::updateLocaleError);
     connect(appCheckUpdate, &CheckUpdates::finished, appCheckUpdatesThread, &QThread::quit, Qt::DirectConnection);
     appCheckUpdatesThread->start();
    

    When I create this on the stack:

      appCheckUpdate.moveToThread(&appCheckUpdatesThread);
      connect(&appCheckUpdatesThread, &QThread::started, &appCheckUpdate, &CheckUpdates::checkUpdate);
      connect(&appCheckUpdate, &CheckUpdates::updateData, this, &Changelog::getUpdateData);
      connect(&appCheckUpdate, &CheckUpdates::networkError, this, &Changelog::changeLogUpdateError);
      connect(&appCheckUpdate, &CheckUpdates::localeError, this, &Changelog::updateLocaleError);
      connect(&appCheckUpdate, &CheckUpdates::finished, &appCheckUpdatesThread, &QThread::quit, Qt::DirectConnection);
      appCheckUpdatesThread.start();
    

    It duplicates data and also leak is still present. How to fix this issue? Thanks in advance.


  • Lifetime Qt Champion

    Hi,

    In the first case, you don't seem to delete appCheckUpdate nor appCheckUpdateThread.

    As for the leak, how do you know you are leaking something ?

    Are you properly handling all resource in your custom classes ?



  • @SGaist

    I have checked the memory leak by Task Manager.

    I have some questions:

    1. Qt does not delete objects when use connect(appCheckUpdate, &CheckUpdates::finished, appCheckUpdatesThread, &QThread::quit, Qt::DirectConnection);?

    I thought when thread has been deleted than the object appCheckUpdate should be also deleted or not?

    1. So where do I have to delete those objects (in destructor)?

    Thanks in advance for your help.


  • Lifetime Qt Champion

    A thread calling finished doesn't delete itself like that, it's your job to know when it should be deleted. If it's when CheckUpdates::finished then connect the deleteLater function to it.



  • @Cobra91151 as an additional note,

    Using Qt::DirectConnection instead of Qt::QueuedConnection is asking for trouble!
    Signal/Slot Connections are threadsave because they are automatically queued, accpet if you give it Qt::DirectConnection as a 5th parameter.



  • @SGaist
    @J.Hilk

    I will try it. Thanks for the suggestions.



  • I use connect(appCheckUpdate, &CheckUpdates::finished, appCheckUpdatesThread, &QThread::deleteLater); but then I get warning: QThread: Destroyed while thread is still running.

    So should I connection like this:

    connect(appCheckUpdate, &CheckUpdates::finished, appCheckUpdatesThread, &QThread::quit);
    connect(appCheckUpdatesThread, &CheckUpdates::destroyed, appCheckUpdatesThread, &QThread::deleteLater);
    

    Or it will not work because the thread is not destroyed?



  • @Cobra91151 said in Memory leak using QThread issue:

    So should I connection like this:

    connect(appCheckUpdate, &CheckUpdates::finished, appCheckUpdatesThread, &QThread::quit);
    connect(appCheckUpdatesThread, &CheckUpdates::destroyed, appCheckUpdatesThread, &QThread::deleteLater);
    

    Or it will not work because the thread is not destroyed?

    No, this is exactly the correct way to do it.



  • @J.Hilk

    I have changed code to:

     appCheckUpdate = new CheckUpdates();
     appCheckUpdatesThread = new QThread();
     appCheckUpdate->moveToThread(appCheckUpdatesThread);
     connect(appCheckUpdatesThread, &QThread::started, appCheckUpdate, &CheckUpdates::checkUpdate);
     connect(appCheckUpdate, &CheckUpdates::updateData, this, &Changelog::getUpdateData);
     connect(appCheckUpdate, &CheckUpdates::networkError, this, &Changelog::changeLogUpdateError);
     connect(appCheckUpdate, &CheckUpdates::localeError, this, &Changelog::updateLocaleError);
     connect(appCheckUpdate, &CheckUpdates::finished, appCheckUpdatesThread, &QThread::quit);
     connect(appCheckUpdatesThread, &CheckUpdates::destroyed, appCheckUpdatesThread, &QThread::deleteLater);
     appCheckUpdatesThread->start();
    

    But the problem still exists, it leaks memory. So this - connect(appCheckUpdatesThread, &CheckUpdates::destroyed, appCheckUpdatesThread, &QThread::deleteLater); - deletes only appCheckUpdatesThread (QThread) object.

    So should I also delete appCheckUpdate by calling delete appCheckUpdate?



  • @Cobra91151
    yes of course,
    at the end of your threaded operation, you'll have to delete your QThread object and your Worker object, seperately.

    In case you haven't seen it, heres an wiki-example



  • @J.Hilk

    Thanks for the link. I will try it.


  • Moderators

    @Cobra91151 It should be:

    connect(appCheckUpdatesThread, &QThread::finished, appCheckUpdatesThread, &QThread::deleteLater);
    

    In http://doc.qt.io/qt-5.8/qthread.html#finished
    "This signal can be connected to QObject::deleteLater(), to free objects in that thread."



  • @J.Hilk

    Thank you. I have fixed the memory leak issue with changelog, but I also have the memory leak when I reload some WMI data during language change of my app. I will try to fix it and reply.



  • Hi! I have some WMI memory leak issue.

    Code:

    //Initialization
    IWbemLocator *pLocator = 0;
    IWbemServices *pService = 0;
    IEnumWbemClassObject* pEnumerator = NULL;
    IWbemClassObject *pclsObj = NULL;
    
    while (pEnumerator)
    {
         hres = pEnumerator->Next(WBEM_INFINITE, 1, &pclsObj, &uReturn);
    
          VARIANT processName;
          pclsObj->Get(L"Name", 0, &processName, 0, 0);
          QString userProcessName;
          userProcessName = QString::fromWCharArray(processName.bstrVal);
           
          emit testData(userProcessName);
          VariantClear(&processName);
    }
    
    //Cleanup
     pService->Release();
     pLocator->Release();
     //pEnumerator->Release(); - Clang Static Analyzer displays issue - called C++ object pointer is null
     //pclsObj->Release(); - Clang Static Analyzer displays issue - called C++ object pointer is null
    

    Every time I change language in my app it takes 1 - 5 MB RAM. How to fix this issue? Thanks in advance.


  • Moderators

    @Cobra91151 This isn't Qt code.. You should probably post this question on a win32 API message board.

    Also are you sure it's actually leaking? Just because the size of memory for your running application grows does not mean it's leaking. You need something like valgrind to test for leaks on exit.



  • @ambershark

    I have checked it in Task Manager. For example, my app takes 25 MB RAM, when changing languages it grows to 30 MB and 35, 40... and never release it. So should I consider this as not a leak?


  • Lifetime Qt Champion

    What do you mean by changing languages ? What do you do to make that happen ?



  • @SGaist

    I have all WMI data in QTreeWidgetItems (QTreeWidget). I have two columns:
    1 - Property
    2 - Data

    Property and data I get from a worker class which connected with signals and slots to thread. So to change property to another language I call clear function on QTreeWidget to delete all property and data, then I call function which connects worker to thread and get this data as QTreeWidgetItems in chosen (translated) language.


  • Lifetime Qt Champion

    Are you loading any language file in order to provide translations ?



  • @SGaist

    Yes, but when QComboBox index changes I call this function to load appropriate translation file:

    Some code:

    appTranslator = new QTranslator(this);
    qApp->removeTranslator(appTranslator);
    appTranslator->load(":Localization/Test_ru.qm");
     qApp->installTranslator(appTranslator);
    

    and when apply button is clicked I retranslate string and reload WMI data using my loadTraslatedText function


  • Lifetime Qt Champion

    Your code has one tiny problem: you are removing the translator you just created and then you add it again after loading the translation file, so basically you never remove anything and just keep adding new ones.



  • @SGaist

    I have changed code:

    appTranslator = new QTranslator(this);
    appTranslator->load(":Localization/Test_ru.qm");
     qApp->installTranslator(appTranslator);
    

    But the leak is still exists.


  • Lifetime Qt Champion

    Isn't it the same code as before ?



  • @SGaist

    Check again. I have copied the wrong one.

    I don't think that the problem is with language change function itself, because when I have commented the getWMIData function no leak is found.

    So the problem is with actual getWMIData which contains other functions such as osData, cpuData.... which lead to worker class which has about 10 thousand lines of code.

    I think the only way to fix this, is to figure out how to change all properties to appropriate language without reloading their data.


  • Lifetime Qt Champion

    You are still still adding one new translator each time you change the language, start by fixing that. Remove the one currently installed and then load the new one.



  • @SGaist

    So I put initialization of translator (appTranslator = new QTranslator(this);) in the constructor.

    appTranslator->load(":Localization/Test_ru.qm");
    qApp->installTranslator(appTranslator);
    

    Then I have added qApp->removeTranslator(appTranslator); at the end to loadTraslatedText function, but now my other windows are not translated (when I open them). The problem is with qApp->removeTranslator(appTranslator); So where to remove the one currently installed?


  • Moderators

    @ambershark said in Memory leak using QThread issue:

    @Cobra91151 This isn't Qt code.. You should probably post this question on a win32 API message board.

    Also are you sure it's actually leaking? Just because the size of memory for your running application grows does not mean it's leaking. You need something like valgrind to test for leaks on exit.

    Yea checking active running memory does not show you leaks. If it changes in task manager that is ok, if when you exit the app there are still pointers to memory that didn't get cleaned up then those are leaks.

    If as you watch your program run and be used it is constantly growing while running in the task manager that can be a leak too, but not just because you did something like allocate a new language.

    The way memory usually works is block are allocated and freed as they are needed. So if you allocate a new block, even though you delete your old one, the OS doesn't necessarily reclaim that memory unless it is needed. So in task manager you would see it change unless you were out of memory then you would see it reclaimed.

    So unless you see it constantly growing and growing, out of control, while running it's probably not leaking. That is why you need a tool like valgrind to detect actual leaks.


  • Lifetime Qt Champion

    1. Remove the currently loaded translator
    2. Delete it
    3. Create the new translator
    4. Install it


  • @ambershark

    Thanks, but the problem is with WMI. And I'm working to fix it.



  • @SGaist

    OK. I will try it and reply.



  • Thank you. I have fixed the WMI memory leak issue by releasing resources.

    Code:

                VariantClear(&serviceName);
                VariantClear(&servicePath);
                VariantClear(&serviceID);
                VariantClear(&serviceType);
                VariantClear(&serviceState);
                VariantClear(&serviceStatus);
                VariantClear(&serviceErrorControl);
                VariantClear(&serviceStartMode);
                VariantClear(&serviceWaitHint);
                VariantClear(&serviceExitCode);
                pclsObj->Release();
            }
        }
    
        // Cleanup
        pService->Release();
        pLocator->Release();
        pEnumerator->Release();
        CoUninitialize();
        emit finished();
        return 0;   // Program successfully completed.
    }
    


  • @SGaist

    I have change appTranslator to stacked object, because I can't find the appropriate place to delete it. So now my function where I load local. files looks like this:

    Code:

    void Test::changeLanguage(int languageIndex)
    {
        languageChanged = true;
        QString systemLocale = QLocale::system().name();
    
        switch (languageIndex) {
            case 0:
                if (systemLocale == "en_US") {
                    qApp->removeTranslator(&appTranslator);
                    appTranslator.load(":Localization/Test_en.qm");
                    qApp->installTranslator(&appTranslator);
                }
    
                if (systemLocale == "ru_RU") {
                    qApp->removeTranslator(&appTranslator);
                    appTranslator.load(":Localization/Test_ru.qm");
                    qApp->installTranslator(&appTranslator);
                }
    
                if (systemLocale == "uk_UA") {
                    qApp->removeTranslator(&appTranslator);
                    appTranslator.load(":Localization/Test_uk.qm");
                    qApp->installTranslator(&appTranslator);
                }
            break;
    
            case 1:
                qApp->removeTranslator(&appTranslator);
                appTranslator.load(":Localization/Test_en.qm");
                qApp->installTranslator(&appTranslator);
            break;
    
            case 2:
                qApp->removeTranslator(&appTranslator);
                appTranslator.load(":Localization/Test_ru.qm");
                qApp->installTranslator(&appTranslator);
            break;
    
            case 3:
                qApp->removeTranslator(&appTranslator);
                appTranslator.load(":Localization/Test_uk.qm");
                qApp->installTranslator(&appTranslator);
            break;
    
            default:
                QMessageBox::critical(this, QObject::tr("Error"), QObject::tr("Can't change localization!"), QMessageBox::Ok);
        }
    }
    

    I think this should work now?


  • Moderators

    @Cobra91151 It should work depending on where appTranslator is declared.
    Back to what @SGaist suggested:

    qApp->removeTranslator(appTranslator); // Remove old translator
    delete appTranslator; // Delete old translator
    appTranslator = new QTranslator(this);
    appTranslator->load(":Localization/Test_ru.qm");
     qApp->installTranslator(appTranslator);
    

    That's it.
    Documentation actually says that removing translator does not delete it, see http://doc.qt.io/qt-5/qcoreapplication.html#removeTranslator
    In your code you actually was removing the new translator instead of the old one:

    appTranslator = new QTranslator(this); // You create new one
    qApp->removeTranslator(appTranslator); // You try to remove the new one
    appTranslator->load(":Localization/Test_ru.qm");
     qApp->installTranslator(appTranslator);
    


  • @jsulm

    Thanks for code example. I have declared it in a header file as stacked object.



  • Issue is solved. Thanks all for the help.


Log in to reply
 

Looks like your connection to Qt Forum was lost, please wait while we try to reconnect.