Qt layout memory issue

[bunjee]

Hello,

I have an issue with Qt layout.

That piece of code is working :


ZeTextWidget * textWidget = new ZeTextWidget ("Salut"); // Creating a new Widget
ZeTopListWidget * topListWidget1 = new ZeTopListWidget(*textWidget); // Adding the widget to the layout
topListWidget1->Clear(); // Clearing the object / layout
delete topListWidget1; // Deleting the object containing the layout


That one isn't :



ZeTextWidget textWidget("Salut"); // Creating a local variable
ZeTopListWidget * topListWidget1 = new ZeTopListWidget(textWidget); // Adding the widget to the layout
topListWidget1->Clear(); // Clearing the object / layout
delete topListWidget1; // Deleting the object containing the layout


When creating my "ZeTopListWidget " I'm doing this :



mLayout.addWidget(&widget);
mWidget = &widget;


When clearing my "ZeTopListWidget" I'm doing this :



if (mWidget)
{
mLayout.removeWidget(mWidget);
mWidget = NULL;
}


When I'm:
- Adding an allocated variable to the layout.
- Clearing the layout (removeWidget).
- Deleting the object containing the layout.
-> It Works.

BUT if I'm:

- Adding a local variable to the layout.
- Clearing the layout (removeWidget).
- Deleting the object containing the layout.
-> Crash on the delete line.

Obviously something is going wrong when the Layout default dtor is called.
Provided the fact that I remove the assigned widget before deleting my object and thus its member's layout: it should work fine.

Is there something I'm missing ?

[marcel]

You're creating textWidget on the stack, so probably gets deleted.
Do it as in the first case. Why does it bother you.

Actually, you also have a problem in Clear function: you make the widget null, although you should let its parent delete it. It gets reparented when you add it to a layout.

Also, I think the biggest problem is mWidget=NULL; You cannot do this with something that is allocated on the stack.

Regards

[bunjee]

Thanks marcel,

Actually mWidget is a pointer on the widget.
Assigning it to NULL doesn't change a thing.

another example :

This works :


ZeTextWidget * textWidget = new ZeTextWidget("Salut");
QHBoxLayout * layout = new QHBoxLayout();
layout->addWidget(textWidget);
delete layout;
delete textWidget;

This one crashes :


ZeTextWidget textWidget("Salut");
QHBoxLayout * layout = new QHBoxLayout();
layout->addWidget(&textWidget);
delete layout;


Thos two pieces of code are all the same for me.

- In one case :
- I'm declaring a self allowed variable in local
- Assigning it to the layout.
- Deleting the layout
- the self allowed variable is/should be deleted at the end of the function locally.

- In another case :
- I'm declaring a pointer variable
- Allocating it myself.
- Assigning it to the layout.
- Deleting the layout
- Deleting the pointer manually.

Imho the result is/should be exactly the same.

[fullmetalcoder]

Did you check WHERE the crash occurs? Run your code under gdb and see if the backtrace indicates that Qt code is faulty or if the error comes from yours...

[marcel]

Once again I am telling you, although it it is common knowledge: you can't play with pointers like that. Take the follwoing example and test it:


void deletePtr(int* x)
{
delete x;
x = NULL;
}

int main(void)
{
int varAllocatedOnStack = 5;
deletePtr(&varAllocatedOnStack);
}


This will cause a segfault/mem violation. You can't actually delete something you allocated on the stack.

That is what is happening in your examples. If you take a closer look at how widget parent/child relations work, you will see that.

Regards

[jpn]

There are only a few acceptable situations when it's ok to allocate a widget on the stack:

a top level window in main() (since QApp::exec() blocks)
a modal dialog anywhere (since its exec() blocks)
perhaps some other I didn't think of

Otherwise you should likely NEVER allocate a widget on the stack. This only causes problems because a QObject automatically deletes its children. This leads to a double free attempt; 1) by parent 2) when going out of scope => crash.

[iw2nhl]

Hi!
You wrote:


This works :


ZeTextWidget * textWidget = new ZeTextWidget("Salut");
QHBoxLayout * layout = new QHBoxLayout();
layout->addWidget(textWidget);
delete layout;
delete textWidget;

This one crashes :


ZeTextWidget textWidget("Salut");
QHBoxLayout * layout = new QHBoxLayout();
layout->addWidget(&textWidget);
delete layout;


Thos two pieces of code are all the same for me.
[...]
Imho the result is/should be exactly the same.
You are partially right: the 2 pieces of code are _almost_ the same (at least for a little code like this).
In fact both works perfectly! I don't know how your second code could crash...
Can you recheck it please? May be there is something wrong with your compiler/Qt version/O.S./...?

I've checked with Qt 4.3.1 and Linux 2.6.18: no crashes at all!

[iw2nhl]

I made some tests and it seems that a child widget is automatically deleted by the destructor only by other widgets.
QLayout is not QWidget, so when you delete the QLayout, the child QWidgets are not deleted.
This is right because if you add a QWidget to a QLayout, the parent() of the QWidget is not QLayout, but equals to QLayout->parent().

[iw2nhl]

Anyway, as already said by others, don't mix stack and heap variables like that! It's very dangerous!

Sample of crashing code:


QLineEdit textWidget("Salut");
QHBoxLayout * layout = new QHBoxLayout();
layout->addWidget(&textWidget);
QGroupBox * groupBox = new QGroupBox();
groupBox->setLayout(layout);
qDebug() << textWidget.parent(); // => groupBox
delete groupBox;

Just before the crash, you see that textWidget.parent() is groupBox, so it's destructor deletes textWidget too, but textWidget is on the stack and the application crashes.

Warning that also a wrong delete can give the same result, also if everything is in the heap!
Sample of crashing code:


QLineEdit * textWidget = new QLineEdit("Salut");
QHBoxLayout * layout = new QHBoxLayout();
layout->addWidget(textWidget);
QGroupBox * groupBox = new QGroupBox();
groupBox->setLayout(layout);
qDebug() << textWidget->parent(); // => groupBox
delete groupBox;
delete textWidget;


This code, instead, is not crashing!


QLineEdit * textWidget = new QLineEdit("Salut");
QHBoxLayout * layout = new QHBoxLayout();
layout->addWidget(textWidget);
qDebug() << textWidget->parent(); // => NULL
QGroupBox * groupBox = new QGroupBox();
groupBox->setLayout(layout);
qDebug() << textWidget->parent(); // => groupBox
layout->removeWidget(textWidget);
qDebug() << textWidget->parent(); // => groupBox
delete groupBox;

See the comments for the output of qDebug(). Note how QHBoxLayout (add and remove functions) does not change the parent of textWidget.

In case of doubt, there is a simple rule: use only objects in the heap and store their pointers with a QPointer (it's always better to understand what you are doing, but this may help if you can't find what is crashing or if you have a shared object):


QPointer< QLineEdit > textWidget = new QLineEdit("Salut");
QPointer< QHBoxLayout > layout = new QHBoxLayout();
layout->addWidget(textWidget);
QPointer< QGroupBox > groupBox = new QGroupBox();
groupBox->setLayout(layout);
qDebug() << textWidget->parent(); // => groupBox
delete groupBox; // this deletes textWidget and sets the QPointer to NULL
delete textWidget; // textWidget now is NULL so no crash

A QPointer becomes NULL when it's object is deleted (works only with QObjects).
And remember you don't need to write code like this:

if (ptr != 0) delete ptr;
because

delete 0
is valid code!

[bunjee]

Thanks everybody,

Well well,
The problem was coming from my side :

this :


ZeTextWidget * textWidget = new ZeTextWidget("Salut");
QHBoxLayout * layout = new QHBoxLayout();
layout->addWidget(textWidget);
delete layout;
delete textWidget;

and this :


ZeTextWidget textWidget("Salut");
QHBoxLayout * layout = new QHBoxLayout();
layout->addWidget(&textWidget);
delete layout;

Perfectly works.
When deleted, a layout doesn't delete the assigned objects.

Concerning the setting the pointer to NULL :

As marcel said, this doesn't work :


void deletePtr(int* x)
{
delete x;
x = NULL;.
}

int main(void)
{
int varAllocatedOnStack = 5;
deletePtr(&varAllocatedOnStack);
}

But I'm more doing something like that :


void deletePtr(int* x)
{
delete x;
}

int main(void)
{
int * newVar = new int(5);

int * newPtr = newVar;

(...)

newPtr = NULL;.

deletePtr(varAllocatedOnStack);
}

As iw2nhl said setting a pointer to NULL is a valid code.
Setting a pointer to NULL is a way knowing if a pointer has been assigned to something or not.

And correct me if I'm wrong but the heap will never create a 0 pointer variable.
Setting a pointer to NULL is like saying : okay the adress of my pointer isn't valid anymore.

And I don't see why it would crash anything in my case, and it doesn't.