PDA

View Full Version : problem with memory leak



kuzynpiy
16 Oct 2006, 11:55 AM
Hello,

I?ve got the problem with memory leak. I use IE engine and when I refresh the page (clicking F5 in IE) the browser has increased the memory by about 500kB to 1MB. Its terrible.
When I remove Your extension to YUI ? I mean the line:

<script></script>
memory is stable and does not leak.
Have You any idea of how to resolve this problem ?

Peter

jack.slocum
16 Oct 2006, 12:02 PM
Just because the memory increases when you reload a page doesn't indicate a leak. It could indicate IE is caching things. 95% Leaks in IE are created by putting JS Objects on DOM nodes and I never do that. However, if you can point me to a link with the problem, I'd be happy to test it for leaks.

kuzynpiy
16 Oct 2006, 12:32 PM
The test page is located at:
http://interpc.pl/~pwywiol/test/splitter.htm
It's almost an empty page (with dblClick on splitter :) implemented by code You sent me).

Try refresh the page to see the memory used by the browser increase. I test it only on IE engine.

I add the purge function to the javascript code. Now the memory leak slowly.

Peter

jack.slocum
16 Oct 2006, 12:54 PM
It could be because you are using legacy event handlers. Try using YAHOO.util.Event for load and unload and see if that changes anything.



YAHOO.util.Event.on(window, 'load', yourLoadFcn);
YAHOO.util.Event.on(window, 'unload', yourUnloadFcn);


Inline load and unload handlers aren't cleaned up and could prevent normal cleanup.

fab
30 Nov 2006, 8:10 AM
first of all, this stuff really *rocks*
i came across this site searching for a grid widget and i found a lot more.

now, i tested:
http://www.jackslocum.com/blog/examples/layout1.htm
with drip to check MSIE memory.

drip does not find any leak, however memory usage increment on every reload by 1M.
it goes like this (Mb): 16,17,18,19...100,101,102.... etc, etc. and memory get freed only restarting drip.

if it is not a leak, why IE keep eating memory on every refresh ?

jbowman
30 Nov 2006, 9:07 AM
could IE be caching each instance of the JS pulled, due to the fact it's being served via php?

on a side note:



95% Leaks in IE are created by putting JS Objects on DOM nodes


Does that include using onClick for dom nodes? If so looks like I am going to need to refactor a lot of my work.

tryanDLS
30 Nov 2006, 9:18 AM
I think that depends on how you handle adding the onclick event. I think there was some discussion about how the yui-ext event model deals with this - not sure if on the blog or here. Here's a article that talks about some of the issues too.

http://javascript.crockford.com/memory/leak.html

This site has a lot of good articles about javascript - Crockford is the guy who created the JSON notation.

fab
30 Nov 2006, 9:25 AM
could IE be caching each instance of the JS pulled, due to the fact it's being served via php?
drip reports the same problem on a local copy with normal .js serving.

jack.slocum
30 Nov 2006, 1:56 PM
As I said, it is probably IE caching page state. Most modern browsers (FireFox and Safari too) not only cache the scripts in the page but also the state of those scripts. This is how they can do quick forward backward navigation etc. If drip says no leaks, chances are there probably aren't any leaks. :)

jack.slocum
30 Nov 2006, 1:58 PM
jbowman, using legacy event handlers like onclick aren't directly a source of leaks. But if you assign them in javascript where the handler's closure could also be wrapping the same DOM node, there's a high probability for a leak. For example:


function foo(node){
node.onclick = function(){
alert(node.id + ' was clicked.';
}
}

That creates a circular reference that is bad. The onclick function has a closure containing the node, and the node has a property (onclick) that contains the function. This is where the big closure scare came from.

jbowman
30 Nov 2006, 2:53 PM
Ah ok... I downloaded drip. None of my onclick elements seem to have leaks. They are just used to show a dialog. My gridpanel elements which get assigned the class ygrid-wrap-body are showing up as leaks though?

jack.slocum
30 Nov 2006, 3:34 PM
Did you assign anything to them?

jbowman
30 Nov 2006, 3:43 PM
custom toolbar item with callback, that lauches a dialog that does something.... oh heck, here's the code block.



showCat: function() {
var catCols = [{
header: "Category",
width: 255,
editor: new YAHOO.ext.grid.TextEditor()
}];
var catCm = new YAHOO.ext.grid.DefaultColumnModel(catCols);
catCm.defaultSortable = true;
var catDm = new YAHOO.ext.grid.JSONDataModel({
root: 'Categories',
totalProperty: "totalCount",
id: 'id',
fields: ['category']
});
catDm.initPaging('/ci/admin/profile_fields_categories', 10);
catDm.setDefaultSort(catCm, 0, 'ASC');
catGrid = new YAHOO.ext.grid.EditorGrid('admin_user_fields_categories', catDm, catCm);
catGrid.getSelectionModel().clicksToActivateCell = 2;
catGrid.render();

var toolbar = catGrid.getView().getPageToolbar();
toolbar.addButton({
className: 'add',
text: "",
click: YAHOO.whasit.newUserFields.addCat
});

YAHOO.whasit.newUserFields.adminFieldsLayout.beginUpdate();
YAHOO.whasit.newUserFields.adminFieldsLayout.add('center', new YAHOO.ext.GridPanel(catGrid, {title: 'Categories', fitToFrame: true}));
YAHOO.whasit.newUserFields.adminFieldsLayout.endUpdate();
catDm.loadPage(1);
},
addCat: function() {
if ( typeof(newCatDialog) == "undefined" ) {
if ( !document.getElementById('newCatDialog')) {
var dlgDiv = document.createElement("div");
dlgDiv.id = "newCatDialog";
document.body.appendChild(dlgDiv);
}
newCatDialog = new YAHOO.ext.LayoutDialog("newCatDialog", {
modal:true,
autoScroll:true,
width:300,
height:100,
shadow:true,
minWidth:300,
minHeight:100,
center: {
autoScroll:true,
alwaysShowTabs: false,
autoCreate: true
}
});
var layout = newCatDialog.getLayout();
newCatDialog.beginUpdate();
if ( !newCatPane ) {
var newCatPane = layout.add('center', new YAHOO.ext.ContentPanel('newCatPane', {
fitToFrame: true,
autoCreate: true }));
}
newCatDialog.endUpdate();
newCatDialog.header.update('New Category');
newCatDialog.addKeyListener(27, newCatDialog.hide, newCatDialog);
newCatDialog.addButton('Close', newCatDialog.hide, newCatDialog);
submitButton = newCatDialog.addButton('Submit', YAHOO.whasit.newUserFields.submitCat, YAHOO.whasit.newUserFields.showCat);
newCatDialog.syncBodyHeight();
}
getEl('newCatPane').getUpdateManager().update({url: '/ci/admin/newcat', loadScripts: true});
newCatDialog.show();
},
submitCat: function(){
getEl('newCatPane').getUpdateManager().loadScripts = true;
getEl('newCatPane').getUpdateManager().update({url: '/ci/admin/newcat', params: {catName: document.getElementById('newCatName').value}});
},



I'm also trying to figure out why IE errors on "Object doesn't support this property or method" when that toolbar item is clicked. So far it seems to be something with the line - if ( typeof(newCatDialog) == "undefined" ) { - but I haven't figured it out yet.

jack.slocum
30 Nov 2006, 4:18 PM
Try removing the parens from the typeof.

I don't see any specific code that looks bad.

jbowman
30 Nov 2006, 4:46 PM
The parens is fine... In fact looking at it, I realized that I wasn't using the autoCreate dialog code you were nice enough to make for me. However, turning that on, I realized that it appears to be broken? Or maybe I upgraded wrong. What I did was use the RC2 yui-ext.js and then after that loaded the new BasicDialog.js to overload it.

If you go to http://whasit.com/ci/ and click on Login and Register in the header, and Logger in the footer, you'll see the craziness.

jbowman
30 Nov 2006, 5:33 PM
I synchronized with the latest svn version and rebuilt my yui-ext.js and still have the same problem btw.

jbowman
30 Nov 2006, 8:49 PM
I figured it out and posted the fix, as well as a suggestion for show() here - http://www.yui-ext.com/forum/viewtopic.php?t=1124

my site will be updated with the latest version later when I'm ready to push code updates again

fab
1 Dec 2006, 1:42 AM
As I said, it is probably IE caching page state. Most modern browsers (FireFox and Safari too) not only cache the scripts in the page but also the state of those scripts. This is how they can do quick forward backward navigation etc. If drip says no leaks, chances are there probably aren't any leaks. :)
i know it's MSIE fault 'cause firefox does not have this problem, however:

yui http://img295.imageshack.us/img295/1573/dripnormalyf9.gif yui-ext http://img237.imageshack.us/img237/870/dripyuiextac5.gif

the graph shows that in yui-ext memory usage grows linearly with IE.
it must be something in the library that prevent crappy MS browser to release memory.

fab
1 Dec 2006, 4:31 AM
ok, here is the test

<html>
<head>
<title>leak test</title>
<script type="text/javascript" src="/yui/build/utilities/utilities.js"></script>
<script type="text/javascript">
YAHOO.util.Event.on(window, "load",
function () {
var display = YAHOO.util.Dom.getStyle(YAHOO.util.Dom.get('test'), 'display');
}
);
</script>
</head>
<body>
<div id="test">test</div>
</body>
</html>
http://img46.imageshack.us/img46/5536/dripnormaliq1.gif
just adding

<script type="text/javascript" src="/yui-ext.0.33-rc2/yui-ext.js"></script>
http://img85.imageshack.us/img85/6346/dripyuiextrv8.gif

jack.slocum
1 Dec 2006, 6:32 AM
just adding <script type="text/javascript" src="/yui-ext.0.33-rc2/yui-ext.js"></script>

Just including a JS file isn't going to create a leak. Just to verify though, I created this page which I can refresh all day long in IE7 and IE6 without any memory growth. I also listen for onDocumentReady and do something worthless.

http://www.yui-ext.com/playpen/leak.php

fab
1 Dec 2006, 7:09 AM
Just including a JS file isn't going to create a leak. Just to verify though, I created this page which I can refresh all day long in IE7 and IE6 without any memory growth. I also listen for onDocumentReady and do something worthless.
jack, have you tried my test ?
i'm not just including yui-ext, i'm also calling

var display = YAHOO.util.Dom.getStyle(YAHOO.util.Dom.get('test'), 'display');
and that is going to eat memory as soon as i include yui-ext.

jack.slocum
1 Dec 2006, 7:23 AM
Same results:

http://www.yui-ext.com/playpen/leak.php

fab
1 Dec 2006, 8:00 AM
Same results:
http://www.yui-ext.com/playpen/leak.php
yeah, it does not leak, but your yui-ext is different from mine (0.33-rc2)
is that a more updated version ?

jack.slocum
1 Dec 2006, 8:22 AM
OK, I found it.

Apparently extending the Function prototype is creating a closure around the window object in IE (or at least this is my guess). Then when you attach an event listener to the window object that isn't cleaned up properly, this creates a circular reference. I looked through the event.js file in yui, but it is twisting and turning so much I can't find the source.

Anyway, in the meantime this stopped the growth.


YAHOO.util.Event.on(window, 'unload', function(){
delete Function.prototype.createSequence;
delete Function.prototype.defer;
delete Function.prototype.createDelegate;
delete Function.prototype.createCallback;
delete Function.prototype.createInterceptor;
});

I've added this to the dev copy of yutil.js and I will continue to hunt for the true source of the leak.

Thanks for your persistence.

fab
1 Dec 2006, 8:43 AM
yeah, your yui-ext-1128.php was leaking on my test and i was like :shock:
it took me 30m to understand why..... 'couse you don't have id='test' in your code....

i'm very glad you have found the source of the problem.


Thanks for your persistence.
jack, thx for your code :wink:

jack.slocum
1 Dec 2006, 8:51 AM
Please let me know if that fixes it in your environment.

I will probably make a blog post about this, as a lot of other frameworks extend the Function prototype as well and it's sure to be a common problem. After more testing it doesn't appear to be a typical leak. There is no detectable circular reference. That's probably why drip and other tools are unable to pick up on the leak.

fab
1 Dec 2006, 8:56 AM
Please let me know if that fixes it in your environment.
yes, MSIE stopped eating all my memory.

fab
1 Dec 2006, 10:03 AM
john, in my previous testing i had found out that instanceof was causing massive leaks.
so i came out with this simple test that eats like 1mb on every reload

<html>
<head>
<title>leak test</title>
<script type="text/javascript" src="/yui/build/utilities/utilities.js"></script>
<script type="text/javascript" src="/yui-ext.0.33-rc2/yui-ext.js"></script>
<script type="text/javascript">
function leak(el) {
var leaker = el instanceof Array;
}
</script>
</head>
<body>
<div id="test">test</div>
<script type="text/javascript">
leak(document.getElementById('test'));
</script>

</body>
</html>
removing prototypes fixes the leak.
hope it helps.

fab
1 Dec 2006, 10:36 AM
this is crazy.... and has nothing to do with yui or yui-ext...

<html>
<head>
<title>leak test</title>
<script type="text/javascript" src="/leaker.js"></script>
<script type="text/javascript">
function leak(el) {
var leaker = el instanceof Array;
}
</script>
</head>
<body>
click me and i will eat your ram (test.php)
<script type="text/javascript">
leak(document.getElementById('test'));
</script>

</body>
</html>
leaker.js:


Function.prototype.foo = function(){ return false};

/* just add 2mb of comments...... */

add more comments and MSIE will eat more ram.....

jack.slocum
1 Dec 2006, 10:44 AM
IE is leakmaster 2000. :)

fab
4 Dec 2006, 10:15 AM
just in case, i've put the leak test here:

http://makoomba.googlepages.com/home

mschering
28 Jun 2007, 2:31 AM
Can anyone tell me if this will cause leaks:


tb.add(new Ext.Toolbar.Button({
id: 'delete',
icon: GOimages['delete'],
text: GOlang['cmdDelete'],
cls: 'x-btn-text-icon',
handler: function(){
var selectedRows = grid.selModel.selections.keys;

if(selectedRows.length)
{
var conn = new Ext.data.Connection();
conn.request({
url: 'action.php',
params: {task: 'delete', selectedRows: Ext.encode(selectedRows)},
callback: function(options, success, response)
{
if(!success)
{
Ext.MessageBox.alert('Failed', response.result.errors);
}else
{

ds.reload();
}
}
});
}
}
})


I mean putting a function directly to a handler. In the beginning of these thread I read that this could cause leaks but I find this a very convenient way of coding.