#282 Memory Leak: PyCWnd objects are never freed

closed-fixed
Mark Hammond
pythonwin (176)
7
2006-07-11
2006-05-16
kxroberto
No

Memory leak: PyCWnd objects are never free'd.

( and minor bug: If Window is never CreateWindow'ed +
Destroy'Ed, the HookXXX'ed function-objects are also
never free'd. Maybe the "Py_INCREF(hookedObject);" in
win32cmd.cpp/Line228 (and associated DODECREF) are
unnecessary : the Window/Hooklist must hold the hooked
method, but not the reverse? )

I post examples in follow up - as sf main post destroys
indentation.

-robert

Discussion

  • kxroberto
    kxroberto
    2006-05-16

    Logged In: YES
    user_id=972995

    The bug manifests in long running win32ui apps and can be
    made visible: The memory consumption goes infinitely up
    doing this (both - with and without
    .CreateWindow/.DestroyWindow called :

    >>> for i in range(10000):
    ... w=win32ui.CreateWnd()
    ...
    #w.CreateWindow(None,"wef",wc.WS_OVERLAPPEDWINDOW,(0,0,100,100),win32ui.GetMainFrame(),1111)
    ... #w.DestroyWindow()
    ...

    strange: the refcount of a created Window is always minimum 2:

    >>> sys.getrefcount(w)
    2
    >>>

    -robert

     
  • kxroberto
    kxroberto
    2006-05-16

    • labels: --> pythonwin
    • priority: 5 --> 6
     
  • kxroberto
    kxroberto
    2006-05-16

    • priority: 6 --> 7
     
  • kxroberto
    kxroberto
    2006-05-16

    Logged In: YES
    user_id=972995

    The minor bug exposed another really serious bug ( the cause
    of e.g.
    http://groups.google.de/group/comp.lang.python/browse_frm/thread/3aaece8b5f4359cb
    ), which is probably best fixed at the most central spot in
    Python_do_callback:

    For example when in Python_check_message
    Python_callback(method, msg) is done, there is currently no
    extra INCREF to 'method' done - and currently also nowhere
    else down that call until it goes into Python code. The
    Python message handler can now DECREF 'method' (e.g. by
    another HookMessage or by DestroyWindow or ...) and if no
    one else holds a ref to method => the executing function
    looses its own ground!, its frame/closure/cell's =>
    'SystemError:
    C:\\\\sf\\\\python\\\\dist23\\\\src\\\\Objects\\\\cellobject.c:22:
    bad argument to internal
    function\\n\'

    The following example shows, that there is only one ref from
    the current frame to the function, when the installed and
    running function (itself) is stored in _ :

    ----

    import win32ui,win32con, gc

    def WmTimerExec(wnd,f):
    def ON_WM_TIMER(msg):
    wnd.KillTimer(123)
    _ = wnd.HookMessage(None,win32con.WM_TIMER)
    print gc.get_referrers(_)
    f()
    wnd.HookMessage(ON_WM_TIMER,win32con.WM_TIMER)
    wnd.SetTimer(123, 50)

    def f():
    print "f()"

    WmTimerExec(win32ui.GetMainFrame(), f)

    ----

    => if the storage in _ is not done, the closure and cells
    are recycled while the function is still executing!

    Maybe the following patch is necessary - also in attchment
    ( or alternatively many such INCREF's whenever a 'method' is
    used from a xxx_hook_list) :

    --- win32uimodule.cpp.orig 2006-01-10 14:38:00.000000000 +0100
    +++ win32uimodule.cpp 2006-05-16 22:22:24.000000000 +0200
    @@ -773,8 +773,11 @@
    PyObject *newarglst =
    Py_BuildValue("(OO)",themeth,thearglst);
    result = gui_call_object( pCallbackCaller, newarglst );
    DODECREF(newarglst);
    - } else
    + } else {
    + Py_XINCREF(themeth);
    result = gui_call_object( themeth, thearglst );
    + Py_XDECREF(themeth);
    + }
    DODECREF(thearglst);
    if (result==NULL) {
    TRACE("Python_do_callback: callback failed with
    exception\n");

    =============

    ( this doesn't solve the other leak problem; therfore most
    probably the Py_INCREF(hookedObject) / DODECREF.. has to be
    removed )

     
  • kxroberto
    kxroberto
    2006-05-16

     
    Attachments
  • Mark Hammond
    Mark Hammond
    2006-07-11

    Logged In: YES
    user_id=14198

    I see how we need to add the extra reference to the method
    as your patch shows, and I'll check that in. I also worked
    out that win32ui.CreateWnd() leaked a CWnd object (but *not*
    the PyObject associated with it). I'll check both changes
    in and close this bug - if you still think the management of
    hookedObject is incorrect, please reopen (or open a new one)
    with more details.

    Thanks!
    Checking in win32cmd.cpp;
    new revision: 1.2; previous revision: 1.1
    Checking in win32uimodule.cpp;
    new revision: 1.32; previous revision: 1.31
    Checking in win32virt.cpp;
    new revision: 1.3; previous revision: 1.2
    Checking in win32win.cpp;
    new revision: 1.10; previous revision: 1.9

     
  • Mark Hammond
    Mark Hammond
    2006-07-11

    • assigned_to: nobody --> mhammond
    • status: open --> closed-fixed
     
  • Mark Hammond
    Mark Hammond
    2006-07-12

    Logged In: YES
    user_id=14198

    I also meant to mention:
    > strange: the refcount of a created Window is always
    > minimum 2:
    > >>> sys.getrefcount(w)
    > 2

    This is a feature of getrefcount - the refcount is always 1
    more than the real value due to a reference held by the
    function itself. Eg, try "l=[]; sys.getrefcount(l)"