[pywin32-bugs] [ pywin32-Bugs-1489690 ] Memory Leak: PyCWnd objects are never freed

[SourceForge.net <no...@so...>]

Bugs item #1489690, was opened at 2006-05-17 02:42
Message generated for change (Comment added) made by mhammond
You can respond by visiting: 
https://sourceforge.net/tracker/?func=detail&atid=551954&aid=1489690&group_id=78018

Please note that this message will contain a full copy of the comment thread,
including the initial issue submission, for this request,
not just the latest update.
Category: pythonwin
Group: None
>Status: Closed
>Resolution: Fixed
Priority: 7
Submitted By: kxroberto (kxroberto)
>Assigned to: Mark Hammond (mhammond)
Summary: Memory Leak: PyCWnd objects are never freed

Initial Comment:
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


----------------------------------------------------------------------

>Comment By: Mark Hammond (mhammond)
Date: 2006-07-11 15:19

Message:
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


----------------------------------------------------------------------

Comment By: kxroberto (kxroberto)
Date: 2006-05-17 06:34

Message:
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 )


----------------------------------------------------------------------

Comment By: kxroberto (kxroberto)
Date: 2006-05-17 02:46

Message:
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


----------------------------------------------------------------------

You can respond by visiting: 
https://sourceforge.net/tracker/?func=detail&atid=551954&aid=1489690&group_id=78018