Thalia
Thalia

Reputation: 14605

Are destructors necessary in QDialogs?

I am following Qt examples (like TabDialog) and I notice that all the UI items are created as pointers - yet I see no delete and no destructor.

Is that right ? Will that not lead to memory leak ?

I am trying to add destructors

~TabDialog()
{
    delete tabWidget;
    delete buttonBox;
}

and on caller

TabDialog *tabDialog = new TabDialog();
tabDialog->setAttribute(Qt::WA_DeleteOnClose);
tabDialog->exec();

But the program crashes when I close the dialog.

Are the destructors, and delete all pointer items, unnecessary or am I doing it wrong ?

Upvotes: 2

Views: 1373

Answers (3)

Jablonski
Jablonski

Reputation: 18504

I think that you are confused because of these lines:

tabWidget = new QTabWidget;//and so on

You don't see explicit parent (like new QTabWidget(this);), but it is not necessary here. Take a look here:

QVBoxLayout *mainLayout = new QVBoxLayout;
mainLayout->addWidget(tabWidget);
mainLayout->addWidget(buttonBox);
setLayout(mainLayout);

setLayout will reparent your QVBoxLayout and QVBoxLayout will reparent all widgets inside it, so now your widgets has a parent and they will be destroyed after your dialog.

As doc said:

When you use a layout, you do not need to pass a parent when constructing the child widgets. The layout will automatically reparent the widgets (using QWidget::setParent()) so that they are children of the widget on which the layout is installed.

Note: Widgets in a layout are children of the widget on which the layout is installed, not of the layout itself. Widgets can only have other widgets as parent, not layouts.

Upvotes: 4

I'm sorry, but this just doesn't reproduce. The test case is below. You'd probably need to add some extra code to make it reproduce.

Qt's memory management will take care of everything, since all of the widgets end up having parents. Namely:

  1. Tabs are parented as soon as they are passed to addTab.
  2. tabWidget and buttonBox are parented as soon as they are added to the layout.

Since you delete the tabWidget and buttonBox before Qt attempts to delete them, everything is fine. As soon as you delete them, QObject's memory management is informed and they are removed from the TabDialog's child list. I've made this point explicit in the destructor code.

The meaning of Q_ASSERT is: "At this point during runtime, the following must be true". If we're wrong, the debug build will abort. Since it doesn't, the assertions are correct. Thus, before delete tabWidget, the dialog has both QTabWidget and QDialogButtonBox children. After delete tabWidget, the dialog should not have any QTabWidget children anymore. And so on.

#include <QApplication>
#include <QDialog>
#include <QTabWidget>
#include <QDialogButtonBox>
#include <QVBoxLayout>

class TabDialog : public QDialog
{
   QTabWidget *tabWidget;
   QDialogButtonBox *buttonBox;
public:
   TabDialog() :
      tabWidget(new QTabWidget),
      buttonBox(new QDialogButtonBox(QDialogButtonBox::Ok | 
                                     QDialogButtonBox::Cancel))
   {
      tabWidget->addTab(new QWidget, tr("General"));
      tabWidget->addTab(new QWidget, tr("Permissions"));
      tabWidget->addTab(new QWidget, tr("Applications"));
      QVBoxLayout *layout = new QVBoxLayout;
      layout->addWidget(tabWidget);
      layout->addWidget(buttonBox);
      setLayout(layout);
      connect(buttonBox, SIGNAL(accepted()), this, SLOT(accept()));
      connect(buttonBox, SIGNAL(rejected()), this, SLOT(reject()));
   }
   ~TabDialog() {
      Q_ASSERT(findChild<QTabWidget*>());
      Q_ASSERT(findChild<QDialogButtonBox*>());
      delete tabWidget;
      Q_ASSERT(! findChild<QTabWidget*>());
      Q_ASSERT(findChild<QDialogButtonBox*>());
      delete buttonBox;
      Q_ASSERT(! findChild<QTabWidget*>());
      Q_ASSERT(! findChild<QDialogButtonBox*>());
   }
};

int main(int argc, char *argv[])
{
   QApplication a(argc, argv);
   TabDialog *tabDialog = new TabDialog();
   tabDialog->setAttribute(Qt::WA_DeleteOnClose);
   tabDialog->exec();
   return 0;
}

The only way it'd crash is if you tried the following:

int main(int argc, char *argv[])
{
   QApplication a(argc, argv);
   TabDialog *tabDialog = new TabDialog();
   tabDialog->setAttribute(Qt::WA_DeleteOnClose);
   tabDialog->exec();
   // At this point `tabDialog` is a dangling pointer.
   delete tabDialog; // crash
   return 0;
}

Unfortunately, Qt examples are a case of pointless premature pessimization. Qt's classes utilize the PIMPL idiom extensively. Thus the size of, say a QTabWidget is not much larger than that of QObject (48 vs. 16 bytes on my 64 bit platform). By allocating the fixed members of your class on the heap, you're performing two heap allocations: a small one for the QObject-derived class, and then another one for its PIMPL. You're doubling the number of allocations for no good reason.

Here's how to avoid this pessimization:

#include <QApplication>
#include <QDialog>
#include <QTabWidget>
#include <QDialogButtonBox>
#include <QVBoxLayout>

class TabDialog : public QDialog
{
   QVBoxLayout m_layout;
   QTabWidget m_tabWidget;
   QDialogButtonBox m_buttonBox;
   QWidget m_generalTab, m_permissionsTab, m_applicationsTab;
public:
   TabDialog() :
      m_layout(this),
      m_buttonBox(QDialogButtonBox::Ok | QDialogButtonBox::Cancel)
   {
      m_tabWidget.addTab(&m_generalTab, tr("General"));
      m_tabWidget.addTab(&m_permissionsTab, tr("Permissions"));
      m_tabWidget.addTab(&m_applicationsTab, tr("Applications"));
      m_layout.addWidget(&m_tabWidget);
      m_layout.addWidget(&m_buttonBox);
      connect(&m_buttonBox, &QDialogButtonBox::accepted, this, &QDialog::accept);
      connect(&m_buttonBox, &QDialogButtonBox::rejected, this, &QDialog::reject);
   }
};

int main(int argc, char *argv[])
{
   QApplication app(argc, argv);
   auto tabDialog = new TabDialog();
   tabDialog->setAttribute(Qt::WA_DeleteOnClose);
   tabDialog->show(); // NOT tabDialog->exec()!!
   return app.exec();
}

The less explicit heap allocations, the better. This way you can't even be tempted to delete anything in the destructor, since there are no pointers involved. The compiler generates necessary destructor calls for you automatically.

Furthermore, if you're showing only one window in main, there's no point to explicit heap allocation either:

int main(int argc, char *argv[])
{
   QApplication app(argc, argv);
   TabDialog tabDialog;
   tabDialog.show();
   return app.exec();
}

Upvotes: 3

Jean Pierre Dudey
Jean Pierre Dudey

Reputation: 665

Memory handling on Qt

Qt handles the widgets as a tree, every widget has a parent and every parent has the obligation to free the children memory, if a widget has no parent you should delete it manually with the operator delete.

Upvotes: 1

Related Questions