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

Does QMenuBar::clear() release memory of the action created with ?



  • Hi,

    I update the menu bar of a QMainWindow periodically (let say every 2s). To do this, I call the clear method before populating the menu bar again. Although working, this lead to memory leaks.

    Here is a code snipset showing the trouble:

        this->menuBar()->clear();
        this->bm = this->menuBar()->addMenu(QObject::tr("Menu"));
       auto *const qmm = this->bm->addMenu(QObject::tr("Items"));
        for (const auto &mn : items) {
          auto *const qmma = qmm->addAction(mn.name);
          qmma->setIcon(mn.icon);
          // associate an action with qmma
        }
    

    Would somebody tell me what I am doing wrong or if there is a proper way to delete all the items associated with my menu bar.

    Regards,

    Thomas Helfer


  • Lifetime Qt Champion

    Hi,

    Not knowing what else your application is doing can't ensure it's the menu bar that's the culprit.

    In any case, it's a bit strange to completely clear the menu bar so often especially if it's only to update one submenu. You should rather only manipulate what's really needed.



  • @SGaist Hi,

    You are perfecly right on the fact that I shall only remove what really needs to be updated.

    However, the main point is that there is a memory leak to I need to solve.

    If I remove the update of the QMenuBar, no more memory leak. If I remove the call to the setIcon method, the memory leak is much lower, so I do suspect that the memory leak is related to the fact that the clear method does not do what I believed.

    BTW, I use Qt 5.7.1 under debian jessie.

    Thanks for you answer.


  • Lifetime Qt Champion

    Can you check whether bm is still valid after the call to clear and before overwriting its content ?



  • @SGaist How would I ? this->bm is a pointer, there is no way that the clear method would change it, so could I check if it's still valid ?


  • Lifetime Qt Champion

    Use a QPointer



  • @SGaist Thanks for the tip ! I totally forgot about QPointer. I'll do this tomorrow.



  • Well, using QPointer shows that this->bm is not released by QMenuBar::clear(). Shall we consider

    • that this is normal and one shall release each items (QMenu and QActions) by hand (though I did not think about how to do it in pratice)
    • this is a bug in QMenuBar
      What do you think ?
      Regards

  • Qt Champions 2019

    QMenu *QMenuBar::addMenu(const QString &title)

    Appends a new QMenu with title to the menu bar. The menu bar takes ownership of the menu. Returns the new menu.

    So when destroying the menu, the submenus will be gone. There is no constraint that those menus are destroyed on clear() esp. since the the documenation does not state anything releated to this:

    Removes all the actions from the menu bar.



  • @Christian-Ehrlicher Since the QMenuBar takes the ownership of the sub-menus, I do not feel that deleting them by hand is legal.

    So, if clear is not meant to release the memory, how to do it ?

    Even the documentation of the removeAction method does not state that the action is deleted. And there is no removeMenu method that would work on submenus.

    Any advice is welcomed.


  • Lifetime Qt Champion

    @thelfer
    Hi
    Since Actions can be shared between toolbar and menubar, its reasonable that clear() does not
    free them.
    Its a bit unusual use case but you could do like.

    
    void enumerateMenu(QMenu* menu) {
      foreach (QAction* action, menu->actions()) {
        if (action->isSeparator()) {
          qDebug("this action is a separator");
        } else if (action->menu()) {
          qDebug("action: %s", qUtf8Printable(action->text()));
          qDebug(">>> this action is associated with a submenu, iterating it recursively...");
          enumerateMenu(action->menu());
          delete action->menu(); // KILL IT
          qDebug("<<< finished iterating the submenu");
        } else {
          qDebug("action: %s", qUtf8Printable(action->text()));
          delete action; // KILL IT
        }
      }
    }
    
    void MainWindow::on_btRemove_released() {
      enumerateMenu(bm);
      delete bm; // bm is member
    }
    

    Seems to clean up as demostrated in this sample where i subclass Aaction to write to qDebug in dtor.
    https://www.dropbox.com/s/jwlpvpva2mx6cy9/FreeMe.zip?dl=0

    I also instrumented QIcon but is assigned by copy so that was not informative.

    enumerateMenu code credits to
    https://stackoverflow.com/questions/9399840/how-to-iterate-through-a-menus-actions-in-qt


  • Qt Champions 2019

    @thelfer said in Does QMenuBar::clear() release memory of the action created with ?:

    So, if clear is not meant to release the memory, how to do it ?

    It's clearly documented:

    The menu bar takes ownership of the menu.

    --> When the QMenuBar is destroyed, the QActions will be deleted too.


  • Lifetime Qt Champion

    Hi
    Fooling around with it, maybe just total nuke works best

    void MainWindow::on_btDelete_released() {
    delete ui->menuBar;
    this->setMenuBar( new QMenuBar(this) );
    }

    it dont even flashes and clearly delete all.



  • Well, I tested @mrjj approach, and it led to a segfault right from the start. I'll investigate.


  • Lifetime Qt Champion

    @thelfer
    Oh.
    enumerateMenu crashed ?
    I tested it with several subitems.

    Do you have multiple "items" ?

    alt text

    Did sample crash for you? or was in first when used in real code?



  • Well, I found the reason of the segfault: I hold one action as a member. This action was deleted by enumateMenu and the code crashes when this action was added again, which is perfectly normal. I think that I have to refactor my code.
    Thanks to everyone.


  • Lifetime Qt Champion

    @thelfer
    Ok. good found.
    I assume its a no go to wipe whole menubar and recreate ?
    You are keeping some when u "refresh" ?
    delete ui->menuBar;
    this->setMenuBar( new QMenuBar(this) );


Log in to reply