Bugs item #787137, was opened at 2003-08-12 03:36
Message generated for change (Comment added) made by xwisdom
You can respond by visiting:
https://sourceforge.net/tracker/?func=detail&atid=105757&aid=787137&group_id=5757
Category: DynAPI 3 API
Group: None
Status: Open
Resolution: None
Priority: 5
Submitted By: Andrew Gillett (agillett)
Assigned to: Nobody/Anonymous (nobody)
Summary: Memory leak in DynElement.deleteChild()
Initial Comment:
The DynElement.deleteChild() function is implemented
thus:
p.deleteChild = function(c) {
c.removeFromParent();
c._delete();
};
Although the _delete() function is documented in
quickref.dynelement.html, it is not implemented
_anywhere_ in the DynAPI source code, except for
being set to dynapi.functions.Null in event.js.
If you have a page that repeatedly adds and removes
DynElements, it will leak memory. If you replace the call
to _delete() with a call to _destroy(), the memory leak
goes away:
p.deleteChild = function(c) {
c.removeFromParent();
c._destroy();
};
----------------------------------------------------------------------
>Comment By: Raymond Irving (xwisdom)
Date: 2003-08-14 13:02
Message:
Logged In: YES
user_id=696242
Thanks for the feedback
Ok. Will keep the delete functions them _destroy() the layers
as well.
----------------------------------------------------------------------
Comment By: L W (warp9pnt9)
Date: 2003-08-14 12:30
Message:
Logged In: YES
user_id=706287
My view on the matter.
I think the naming convention would be clearer if the
function that removes children from parents was called
removeChild / removeFromParent, but not destroying the
layer. I think this is how it works now. I think the
function that removes the child from the parent AND destroys
the layer should be called deleteChild / deleteFromParent or
maybe destroyChild / destroyFromParent. I think it's
confusing to add a new name "dispose" when it doesn't
intuitively correspond to an underlying "dispose" function.
It's a little vague.
----------------------------------------------------------------------
Comment By: Raymond Irving (xwisdom)
Date: 2003-08-14 02:37
Message:
Logged In: YES
user_id=696242
I see what's happening. IMO deleteChild() and removeChild
means the same thing, correct?
When a child layer is removed from the parent it's actually
deleted from the parent, correct? The layer is not completly
destroyed as a reference to it still exists within DynObject.all
collection.
Calling _destroy() during a deleteChild() function will not only
delete the child from its parent but it will also dispose of the
child object.
I think we should change deleteChild/AllChildren/etc to
disposeChild/AllChildren/etc wihich will clearly define what is
been done to the object. The dispose functions will complete
remove and destroy the object.
Are we all in agreement with changing delete to dispose? Or
should we add the _destroy() function to deleteChild and
keep the delete functions?
(Note: If we keep the delete functions we'll have to
document that it will completely destroy and remove the layer
from the system.)
----------------------------------------------------------------------
Comment By: Andrew Gillett (agillett)
Date: 2003-08-14 00:31
Message:
Logged In: YES
user_id=134108
> Are you using the lastest snapshot?
Yes, I check out the code directly from CVS.
> What browsers are experiencing this memory leak?
IE6 & Mozilla1.4, but that is not really relevant.
On 29/07/2003 12:26 AM (my time), in response to my post
about a bug in deleteAllChildren(), you wrote:
>To fix the memory and layer problem use the following:
>
>p.deleteAllChildren = function() {
> var c,l = this.children.length;
> for(var i=0;i<l;i++) {
> c=this.children[i];
> if(c) {
> c._destroy();
> c._created = c.isChild = false;
> };
> delete this.children[i];
> }
> this.children.length = 0;
>};
>
This fix worked perfectly. The _destroy() method is called on
each child of the layer.
However this is not what was put into CVS. In the current
version of event.js, deleteAllChildren() calls deleteFromParent
() which calls deleteChild() which calls _delete() instead of
_destroy().
It is the call to _delete() that I have a problem with. There
is simply no such function. In line 184 of event.js there is:
... = p._delete = p._destroy = dynapi.functions.Null;
but nowhere else in the entire source tree is _delete()
defined.
Try the attached example. On my Windows XP system, IE
grows by about 6MB each time the "Memory Leak" button is
pressed. But if I replace the call to _delete() with a call to
_destroy() in the deleteChild function, then IE doesn't leak at
all.
----------------------------------------------------------------------
Comment By: Raymond Irving (xwisdom)
Date: 2003-08-13 13:53
Message:
Logged In: YES
user_id=696242
Are you using the lastest snapshot?
What browsers are experiencing this memory leak?
You can get the lastest snapshot here:
http://dynapi.sourceforge.net/snapshot/?N=D
----------------------------------------------------------------------
You can respond by visiting:
https://sourceforge.net/tracker/?func=detail&atid=105757&aid=787137&group_id=5757
-------------------------------------------------------
This SF.Net email sponsored by: Free pre-built ASP.NET sites including
Data Reports, E-commerce, Portals, and Forums are available now.
Download today and enter to win an XBOX or Visual Studio .NET.
http://aspnet.click-url.com/go/psa00100003ave/direct;at.aspnet_072303_01/01
_______________________________________________
Dynapi-Dev mailing list
[EMAIL PROTECTED]
http://www.mail-archive.com/[EMAIL PROTECTED]/
