[PyQt] Memory leak

[Giovanni Bajo]

On Sun, 2008-07-06 at 17:29 +0100, Phil Thompson wrote:
> On Sun, 06 Jul 2008 15:18:10 +0200, Giovanni Bajo <rasky at develer.com>
> wrote:
> > On Sat, 2008-07-05 at 23:04 +0100, Phil Thompson wrote:
> >> On Thu, 03 Jul 2008 16:04:42 +0200, Giovanni Bajo <rasky at develer.com>
> >> wrote:
> >> > Hi Phil,
> >> > 
> >> > with SIP 4.7.6, PyQt 4.2.2, Qt 4.4.0:
> >> > 
> >> > ========================================
> >> > import sip
> >> > import weakref
> >> > from PyQt4.Qt import *
> >> > 
> >> > class MyWidget(QWidget):
> >> >      def sizeHint(self):
> >> >          return QSize(900, 700)
> >> > 
> >> > app = QApplication([])
> >> > 
> >> > ws = MyWidget(None)
> >> > wr = weakref.ref(ws)
> >> > 
> >> > L = QVBoxLayout(None)
> >> > L.addWidget(ws)
> >> > L.activate()
> >> > del L
> >> > del ws
> >> > 
> >> > import gc
> >> > gc.collect()
> >> > 
> >> > assert wr() is None
> >> > ========================================
> >> > 
> >> > The assert triggers, meaning that the object of type MyWidget is not 
> >> > released.
> >> 
> >> This appears to be a Qt problem. Although the docs say that a layout
> > takes
> >> ownership of the widget when addWidget() is called it leaves the
> >> destruction of the widget to the eventual owner of the layout and
> > doesn't
> >> call the widget's dtor itself.
> >> 
> >> If the layout is never used (ie. never passed as an argument to
> >> QWidget.setLayout()) then all the widgets in the layout will leak.
> >> 
> >> An equivalent C++ version behaves in the same way.
> >> 
> >> I could change addWidget() so that the layout doesn't take ownership of
> > the
> >> widget (ie. to match the implementation rather than the documentation)
> > but
> >> that will break any code that creates a populated layout and returns it
> >> from a function.
> > 
> > I believe that it's more correct if you fix this code not to have any
> > memory leak.
> 
> I disagree, I don't work around Qt bugs in PyQt.

Depends on whether you consider it a bug in the Qt documentation or a
bug in the Qt implementation. If it's the former, probably something
should be changed in PyQt as well. If it's the latter, then yes, leave
it to the Trolltech guys.

> > Either that, or you get smarter wrt when the ownership is
> > transferred from Python to C++ (that is, when the layout is reparented
> > to a widget, if ever).
> 
> The reason I'm not going to do this is that it would break lots of code (at
> least, lots of my code).

You can wait a major version to apply this breaker, but it's still
worthwhile. I think that explicitly breaking code is better than
implicitly causing memory leaks which hard to find and debug.

> > Anyway, it turns out that I had reduced the testcase a little too much:
> > it was not the actual memory leak I was seeing in my application.
> > 
> > The correct testcase is this one:
> 
> Fixed in tonight's SIP snapshot.

Many thanks!
-- 
Giovanni Bajo
Develer S.r.l.
http://www.develer.com