The mystery of the leaking ui object

[Perdrix]

I have a class:

class ProgressDlg : public QDialog, public ProgressBase				
{
	Q_OBJECT

	private:
		Ui::ProgressDlg* ui;
                // etc ...

	public:
		ProgressDlg(QWidget* parent = nullptr);
		~ProgressDlg();

the ctor reads:

ProgressDlg::ProgressDlg(QWidget* parent) :
	QDialog { parent },
	ProgressBase{ },
	ui{ new Ui::ProgressDlg },
	m_cancelInProgress{ false }
{
	ui->setupUi(this);   // etc...

The dtor reads:

ProgressDlg::~ProgressDlg()
{
	Close();
	if (nullptr != ui)
	{
		delete ui; ui = nullptr;
	}
}

I had to add the if nullptr != ui stuff to the dtor because every time an instance of the class was instantiated and then deleted, it leaked the ui object!

I thought that the Qt parent/child object ownership stuff would prevent the need for me to add code like that to the dtor?

Can someone smarter than I explain how this can happen?

Thanks, David

[JonB]

@Perdrix said in The mystery of the leaking ui object:

I had to add the if nullptr != ui stuff to the dtor

What does this have to do with checking if ui == nullptr? You can see that is initialized in your ProgressDlg() constructor.

I thought that the Qt parent/child object ownership stuff would prevent the need for me to add code like that to the dtor?

You don't have to add the nullptr check if you don't want to --- and don't forget delete nullptr is perfectly legal in C++ anyway --- but you do have to delete it since you newed it. Where does ui have any "Qt parent/child object ownership" stuff? It doesn't, and doesn't have any parentage.

[Perdrix]

@JonB Put it all down to a "not enough coffee morning"

D.

[SimonSchroeder]

There are two things you can learn from modern C++.

1. Use default initializers directly when declaring member variables: Ui::ProgressDlg* ui = nullptr;. If you forget to initialize the variable in one constructor it will use this default value for initialization.

2. Even better is to use std::unique_ptr<Ui::ProgressDlg> ui; in this case. Then you don't have to handle the delete inside the destructor. unique_ptr will work with the forward declaration of your ui class.

[Chris Kawa]

@SimonSchroeder said:

Even better is to use std::unique_ptr<Ui::ProgressDlg> ui; in this case.

The caveat being that you have to have a non-inline destructor for your class in that case, so you have to define it in your cpp even if it's empty. That's because the unique_ptr has to have a complete definition of the pointed to type at the time of instantiating its destructor, so can't be inlined in a header that only has forward declaration. Not a big deal if you do other stuff in the destructor, but kinda spoils the benefit of using RAII pointer if you don't.