Possible bug in WPopupWidget
Added by Thomas Suckow about 11 years ago
I had been using a WDialog and setting a parent, which in hindsight was unnecessary because during a session the parent is never removed. However, I can see use cases with a WPopupWidget where the parent may be removed and the popup should go away as well.
That said.
It appears that the setParent override in WPopupWidget is causing a SEGFAULT when the parent of the WPopupWidget is called if the WPopupWidget has already been destroyed.
Lets step through a use of two widgets PARENT and POPUP.
- POPUP is newed with new WPopupWidget( PARENT );
- In the constructor, WPopupWidget calls PARENT->addChild( POPUP )
- addChild calls POPUP->setParent( PARENT );
- WPopupWidgets override of setParent falls through because PARENT is neither 0, nor the DOM_ROOT
- POPUP now has no parent
- POPUP is destroyed
- PARENT is destroyed and SEGFAULTS trying to delete POPUP (Use after free)
You even made a note of this in WObject \" // exception: WPopupWidget \" but did not appear to correctly handle it.
Granted I don't have a bullet proof test case, but based on the vague results valgrind is giving me, this is what I believe is happening.
If I don't set a parent, then everything is fine.
Replies (7)
RE: Possible bug in WPopupWidget - Added by Koen Deforche about 11 years ago
Hey,
Dmitriy just noted the same issue on the mailing list, and I've committed a fix for this in latest git. It would be great if you could give it a try ?
Regards,
koen
RE: Possible bug in WPopupWidget - Added by Thomas Suckow about 11 years ago
Possibly unrelated to this change specifically, but I am seeing the content of the dialog being rendered in the titlebar.
I need to do more testing to be sure about parenting issues.
RE: Possible bug in WPopupWidget - Added by Koen Deforche about 11 years ago
Hey Thomas,
That sounds bad (and very weird). A screenshot might ring a bell ?
Regards,
koen
RE: Possible bug in WPopupWidget - Added by Thomas Suckow about 11 years ago
Sorry for the delay, got tied up with other things.
The fix works, everything is fine. Some object files just were not getting recompiled and were linked against the old Wt 3.3.0 instead of the latest git.
-
Thomas
RE: Possible bug in WPopupWidget - Added by Thomas Suckow about 11 years ago
I spoke too soon. This causes memory leaks when a WPopupWidget has a "fake parent".
The WDateValidator created on line 104 in WDatePicker is getting leaked.
The WPopupWidget is parented (not really) to the composite widget, upon destruction of the composite widget the following test is false in WObject.C:
if (c->parent_ == this) // exception: WPopupWidget
c->setParent(0);
But then when you delete the WPopupWidget the new code:
if (fakeParent_)
fakeParent_->WObject::removeChild(this);
Modifies the composite widgets child vector and the for loop is no longer valid, thus the second child (the WDateValidator) leaks.
Honestly, ~Wobject should probably just be doing something like (pseudo code, untested):
if (children_) {
while(children_->size() > 0) {
WObject *c = (*children_)[0];
c->setParent(0);
delete c;
}
delete children_;
}
RE: Possible bug in WPopupWidget - Added by Koen Deforche almost 11 years ago
Hey,
Indeed that is something we need to take into account. I've implemented this. Sorry for the delay, my excuse is that I become father once more ... :-)
koen
RE: Possible bug in WPopupWidget - Added by Thomas Suckow almost 11 years ago
Apparently you didn't learn the first time. You must just like only getting three hours of sleep.
Also, did you push? I don't see the fix on github, I did test out your one commit you put up but it didn't change anything.
Congrats,
Thomas