os-fpga / FOEDAG

Framework Open EDA Gui
https://foedag.readthedocs.io
Other
58 stars 29 forks source link

[memory leak case] reportsWidget is not deleted when it's tab is closed #1372

Open w0lek opened 10 months ago

w0lek commented 10 months ago

steps to reproduce: 1) switch to branch associated with current ticket custom TWidget class with log messages is added to illustrate issue. 2) build and run foedag 3) run project compilation 4) activate "Synthesis - Design Statistics" 5) see that we create TWidget constructor printed in the log message 6) close tab report, here we must see destructor message ~TWidget, but it doesn't occur. 7) if we repeat the steps 5) and 6) we will continuously dynamically allocate the widget and don't release memory

KochynVolodymyr commented 10 months ago

tabWidget->addTab(reportsWidget, reportName); takes the ownership of the reportsWidget.

w0lek commented 7 months ago

@volodymyrkochyn @ravikiranchollangi For Interactive Path Analysis tool we have following code, the place when we create NCriticalPathWidget and insert it as the tab.

NCriticalPathWidget* viewWidget = new NCriticalPathWidget(compiler);
viewWidget->setProperty("deleteOnCloseTab", true);

tabWidget->addTab(viewWidget, NCRITICALPATH_UI_NAME);
tabWidget->setCurrentWidget(viewWidget);

Here i am using custom property "deleteOnCloseTab" just to provide exclusive behaviour for a tab which is NCriticalPathWidget, and not touch other types of tabs (other reports)

And then, on tab close event we have following code

    void TextEditorForm::SlotTabCloseRequested(int index) {
....

  // TODO: with removing property "deleteOnClose" it may become possible solution for https://github.com/os-fpga/FOEDAG/issues/1372
  QWidget* page = m_tab_editor->widget(index);
  if (page) {
    QVariant value = page->property("deleteOnCloseTab");
    if (value.isValid() && value.toBool()) {
      page->deleteLater();
    } 
  }

For generic fix, we may just remove setup and check custom property "deleteOnCloseTab". And do deleteLater() on any tab widget which is closed. But i am not sure if this fix is applicable for all tabs (maybe some report context is reused after it's close etc).

volodymyrkochyn commented 7 months ago

@w0lek If you go to method bool TextEditorForm::TabCloseRequested(int index)

It should delete the editor. If not, that delete is missing here I guess:

Editor *tabItem = qobject_cast<Editor *>(m_tab_editor->widget(index));
  if (!tabItem) {
    m_tab_editor->removeTab(index);
    return true;
  }

Please add deleting of widget properly and not workaround.