KAutoPointer: a new autoptr class for QPointer

[Lubos Lunak]

Dne středa 08 července 2009 19:19:43 David Jarvie napsal(a):
> There is a widespread issue in KDE applications, described by Frank
> Osterfeld at http://www.kdedevelopers.org/node/3919, where if an
> application is displaying a modal dialog when it receives a quit signal,
> the dialog will be deleted by its parent's destructor and after the exec()
> call returns, the application is likely to crash due either to double
> deletion of the dialog, or use of the dangling pointer.
>
> A new template class for kdelibs, KAutoPointer, is proposed in
> http://reviewboard.kde.org/r/955/, which provides a combination of a
> QPointer and and autoptr so that double deletion can be avoided, and
> dangling pointers can be detected.

 I disagree with both the analysis of the problem and the proposed solution.

 I would argue that the problem in http://www.kdedevelopers.org/node/3919 is a 
problem on its own and as such the solution is DONOTDOTHAT. I believe that 
quitting an application with a modal dialog open should not quit, but instead 
should require the user to close the dialog first.

 It appears to be also technically wrong, since when exec() in 
ParentWidget::slotDoSomething() is called, the control flow should continue by 
returning from it. That part actually confuses me a bit, since I don't know 
what exactly happens behind the scenes, but I assume that quit() somehow 
destroys some data including the dialog and then returns control, and in that 
case this is just patching over a small problem and asking for a much bigger 
one. If quit() really does this, then it is a bug. First 
ParentWidget::slotDoSomething() should return control to the main loop and 
from there the exit and cleanup should really happen. What seems to happen now 
is completely backwards and with this proposal we will just end up with 
randomly patching places with KAutoPointer until going insane.

 And I have also design issues with KAutoPointer. It appears to me as a random 
mix of do-everything-pointer. QPointer is a class that watches a QObject that 
belongs to something else and that's it, simple. std::auto_ptr or 
boost:scoped_ptr are classes that take ownership of the pointer and take care 
of deleting the object when the time comes. Now, KAutoPointer is what, a class 
that takes ownership of the object, but not really, since it's fine if 
somebody else wipes it out too? That's not design, that's a hack.


 That said, I would strongly support it if KDE finally embraced the 90's and 
started using some kind of a scoped pointer. By which is mean something that 
basically just takes ownership of the object and does delete in the dtor. Qt 
API somehow discourages the usage of the stack and especially less 
experienced C++ developers ofter write stuff like:
void foo()
{
Object* o = new Object;
o->doSomething();
delete o;
}

Which is completely unnecessary and we have numbers of leaks because of this, 
as soon as the control flow becomes even slightly more complicated. I would of 
course rather see this with the object simply allocated on the stack, but 
using a scoped pointer here is still much better. The same with classes, which 
should rather use scoped pointers for objects they own rather than listing 
delete calls in the dtor. Really, people, the 80's called and wanted their 
manual memory management back.

 Could people who know other libraries suggest something, so that we don't 
have to completely reinvent the wheel?

-- 
Lubos Lunak
KDE developer
--------------------------------------------------------------
SUSE LINUX, s.r.o.   e-mail: l.lunak at suse.cz , l.lunak at kde.org
Lihovarska 1060/12   tel: +420 284 028 972
190 00 Prague 9      fax: +420 284 028 951
Czech Republic       http://www.suse.cz