SitePoint Sponsor

User Tag List

Page 1 of 2 12 LastLast
Results 1 to 25 of 30
  1. #1
    Employed Again Viflux's Avatar
    Join Date
    May 2003
    Location
    London, On.
    Posts
    1,127
    Mentioned
    0 Post(s)
    Tagged
    0 Thread(s)

    AJAX Memory Issues

    I have a script that is (via "AJAX") making a request to the server every second.

    I have these lines...

    Code:
    request.onreadystatechange = function()
    {
         //do stuff here
    }
    This creates a memory leak in Internet Explorer.

    I've read all over about closures and how they can create memory leaks in a situation like this, and that they should be avoided.

    However, I have yet to stumble across an explanation of how to avoid it (so that it still works, obviously). I've tried pretty much everything I can think of and I'm certain that this is such a simple problem.

    Any help is much appreciated.

  2. #2
    SitePoint Wizard chris_fuel's Avatar
    Join Date
    May 2006
    Location
    Ventura, CA
    Posts
    2,750
    Mentioned
    0 Post(s)
    Tagged
    0 Thread(s)
    There are a number of things that could cause this. However, we need to see more code to verify that.

  3. #3
    Employed Again Viflux's Avatar
    Join Date
    May 2003
    Location
    London, On.
    Posts
    1,127
    Mentioned
    0 Post(s)
    Tagged
    0 Thread(s)
    Code:
    	request.onreadystatechange = function()
    	{
    		if ( request.readyState == 4 )
    		{
    			if ( request.status == 200 )
    			{
    				pings++;
    			}
    			else
    			{
    				errors++;
    			}
    		}
    	}
    If I leave this running in IE, the memory used keeps climbing and climbing, eventually crashing the app.

  4. #4
    SitePoint Wizard chris_fuel's Avatar
    Join Date
    May 2006
    Location
    Ventura, CA
    Posts
    2,750
    Mentioned
    0 Post(s)
    Tagged
    0 Thread(s)
    How is your actual ajax request constructed?

  5. #5
    Employed Again Viflux's Avatar
    Join Date
    May 2003
    Location
    London, On.
    Posts
    1,127
    Mentioned
    0 Post(s)
    Tagged
    0 Thread(s)
    Here's the whole function...

    Code:
    function pingServer()
    {
    	var page;
    	var request;
    	
    	page = url + 'ping.php?userid=' + escape( userID );
    	request = getXMLHttpRequestObject();
    	
    	request.onreadystatechange = function()
    	{
    		if ( request.readyState == 4 )
    		{
    			if ( request.status == 200 )
    			{
    				pings++;
    			}
    			else
    			{
    				errors++;
    			}
    		}
    		
    		updateStatus();
    	}
    	
    	request.open( 'GET' , page , true );
    	request.send( null );
    }

  6. #6
    Employed Again Viflux's Avatar
    Join Date
    May 2003
    Location
    London, On.
    Posts
    1,127
    Mentioned
    0 Post(s)
    Tagged
    0 Thread(s)
    The memory leak is the same if I remove the updateStatus() call so I'm reasonably confident that it is not the culprit.

  7. #7
    Employed Again Viflux's Avatar
    Join Date
    May 2003
    Location
    London, On.
    Posts
    1,127
    Mentioned
    0 Post(s)
    Tagged
    0 Thread(s)
    Figured I'd try posting more code

    I'm fairly certain that the leak is caused by the closure in the onreadystatechange function. What I'm not sure of, however, is how to remove it.

    Thoughts?

    Code:
    var pings = 0;
    var url = '---';
    
    function initializeChat()
    {
    	updateStatus();
    	startPing();
    }
    
    function initializeRequestObject()
    {
        var requestObject;
    		
    	try
    	{
    		requestObject = new ActiveXObject( 'Msxml2.XMLHTTP' );
    	}
    	catch( e )
    	{
    		try
    		{
    			requestObject = new ActiveXObject( 'Microsoft.XMLHTTP' );
    		}
    		catch( oc )
    		{
    			requestObject = null;
    		}
    	}
    				
    	if( !requestObject && typeof XMLHttpRequest != 'undefined' )
    		requestObject = new XMLHttpRequest();
    					
    	if( !requestObject )
    		debug( 'Could not create the request object.' );
    					
    	return requestObject;
    }
    
    function handleReturn( sender , call )
    {
        if( sender.readyState != 4 )
    	    return;
    	
    	if( sender.status == 200 )
    	{
    	    if( call != '' )
    	        call( sender.responseText );
    	    
    	    sender.onreadystatechange = function(){}
    	    return;
    	}
    	
    	else
    	{
    		errors++;
    		return;
    	}
    }
    
    function startPing()
    {
    	var content;
    	var page;
    	var sender;
    	
    	content = 'userid' + userid;
    	page = url + 'ping.php';
    	sender = initializeRequestObject();
    	sender.open( 'POST' , page , true );
    	sender.setRequestHeader( 'Method' , 'POST ' + page + ' HTTP/1.1' );
    	sender.setRequestHeader( 'Content-Type' , 'application/x-www-form-urlencoded' );
    	
    	sender.onreadystatechange = function()
    	    {
    		if( sender.readyState != 4 )
    			return;
    
    		if( sender.status == 200 )
    		{
    			pings++;
    		}
    	}
    
    	sender.send( content );
    	setTimeout( 'startPing()' , 2000 );
    }
    
    function updateStatus()
    {
    	document.getElementById( 'pings' ).innerHTML = pings;
    }

  8. #8
    SitePoint Zealot logitron's Avatar
    Join Date
    Feb 2006
    Posts
    144
    Mentioned
    0 Post(s)
    Tagged
    0 Thread(s)
    Code:
    	sender.onreadystatechange = function()
    	    {
    		if( sender.readyState != 4 )
    			return;
    
    		if( sender.status == 200 )
    		{
    			pings++;
    		}
    	}
    I'm noticing that you don't do a return if the sender.status == 200. Shouldn't it be returning after the pings++?

    I'm not sure if that's your problem, but if you think it's in the onreadystatechange, that might be it.
    Patrick Smith
    PHP Programmer

  9. #9
    SitePoint Addict dek's Avatar
    Join Date
    Oct 2004
    Location
    UK
    Posts
    352
    Mentioned
    0 Post(s)
    Tagged
    0 Thread(s)
    It is, as you say, the closure. More specifically, it's the fact that with the closure, you've created a circular reference (sender has a reference to the function, which has a reference to sender)

    This is a problem here, because in IE, I believe I'm right in saying, a RequestObject is handled by a different garbage collector from a function. And they don't communicate too well.

    Luckily, there's an easy thing you could try:

    Code:
    function startPing()
    {
    	var content;
    	var page;
    	var sender;
    	
    	content = 'userid' + userid;
    	page = url + 'ping.php';
    	sender = initializeRequestObject();
    	sender.open( 'POST' , page , true );
    	sender.setRequestHeader( 'Method' , 'POST ' + page + ' HTTP/1.1' );
    	sender.setRequestHeader( 'Content-Type' , 'application/x-www-form-urlencoded' );
    	
    	sender.onreadystatechange = myOnReadyStateChange;
    
    	sender.send( content );
    	setTimeout( 'startPing()' , 2000 );
    }
    
    function myOnReadyStateChange()
    
    {
       if (this.readyState != 4)
    	  return;
    
       if (this.status == 200)
    	  pings++;
    }
    This should (touch wood) sort the problem out.
    Only dead fish go with the flow

  10. #10
    SitePoint Wizard Pepejeria's Avatar
    Join Date
    Jan 2005
    Location
    Too far up north
    Posts
    1,566
    Mentioned
    0 Post(s)
    Tagged
    0 Thread(s)
    Microsoft has written an article on how to spot and avoid memory leaks in IE.

    Article was written almost 5 years after they released IE 6, funny that they didnt actually fix the leaks instead during this time...

  11. #11
    SitePoint Addict dek's Avatar
    Join Date
    Oct 2004
    Location
    UK
    Posts
    352
    Mentioned
    0 Post(s)
    Tagged
    0 Thread(s)
    Quote Originally Posted by Pepejeria
    Microsoft has written an article on how to spot and avoid memory leaks in IE.

    Article was written almost 5 years after they released IE 6, funny that they didnt actually fix the leaks instead during this time...
    Those loveable scallywags...

    I don't suppose.... just possibly.... anyone knows if they've sorted this stuff out in IE7? Just maybe?

    Four years ago, before I started working with this stuff, I had hair...
    Only dead fish go with the flow

  12. #12
    SitePoint Wizard Pepejeria's Avatar
    Join Date
    Jan 2005
    Location
    Too far up north
    Posts
    1,566
    Mentioned
    0 Post(s)
    Tagged
    0 Thread(s)
    They claim they fixed some memory leaks, but I haven't had the time to verify it. When I do I will post a simple example here with my results.

  13. #13
    SitePoint Wizard Pepejeria's Avatar
    Join Date
    Jan 2005
    Location
    Too far up north
    Posts
    1,566
    Mentioned
    0 Post(s)
    Tagged
    0 Thread(s)
    Ok, maybe offtopic, but this example leaks really bad in IE 6 (just reload a couple of times)
    Code:
    <script type="text/javascript">
    function Test(iNumber)
    {
    	this.elm = null;
    	this.name = "div nr. " + iNumber;
    	this.init();
    }
    
    Test.prototype.init = function()
    {
    	var oThis = this;
    
    	this.elm = document.createElement("div");
    	this.elm.style.cssText = "width:40px; height:40px; background:orange;";
    	this.elm.onclick = function(){ oThis.handleClick(); }
    	document.body.appendChild(this.elm);
    }
    
    Test.prototype.handleClick = function()
    {
    	alert(this.name);
    }
    
    var aObjects = [];
    
    for(var i=0; i<1000; i++)
    {
    	aObjects[i] = new Test(i);
    }
    </script>
    Good new is doesn't leak in IE 7 beta 2

  14. #14
    SitePoint Addict dek's Avatar
    Join Date
    Oct 2004
    Location
    UK
    Posts
    352
    Mentioned
    0 Post(s)
    Tagged
    0 Thread(s)
    Quote Originally Posted by Pepejeria
    Ok, maybe offtopic, but this example leaks really bad in IE 6 (just reload a couple of times)
    <snip />

    Good new is doesn't leak in IE 7 beta 2
    Offtopic or no, this is very good news.
    Thanks
    Only dead fish go with the flow

  15. #15
    Employed Again Viflux's Avatar
    Join Date
    May 2003
    Location
    London, On.
    Posts
    1,127
    Mentioned
    0 Post(s)
    Tagged
    0 Thread(s)
    It's fixed in IE7 because the closure/circular reference bug has to do with the way IE tracks memory consumed by ActiveX objects.

    With IE7 implementing XMLHTTPRequest natively, it's not an issue.

  16. #16
    SitePoint Addict dek's Avatar
    Join Date
    Oct 2004
    Location
    UK
    Posts
    352
    Mentioned
    0 Post(s)
    Tagged
    0 Thread(s)
    Quote Originally Posted by Viflux
    It's fixed in IE7 because the closure/circular reference bug has to do with the way IE tracks memory consumed by ActiveX objects.

    With IE7 implementing XMLHTTPRequest natively, it's not an issue.
    Hey ho. Pepejeria's example does not use XMLHTTPRequest, so this is not really relevant.

    I don't know whether it's fixed because MS have fixed the GC handling, or because they've changed the way DOM elements are implemented (I'm hoping for the former) but as Pepejeria is referencing what is probably the most common form of IE memory leakage, it's still good news. Now if only they're release a patch for IE 6 which did the same thing (and then forced the whole world to update) my happiness would be ... improved.
    Only dead fish go with the flow

  17. #17
    Employed Again Viflux's Avatar
    Join Date
    May 2003
    Location
    London, On.
    Posts
    1,127
    Mentioned
    0 Post(s)
    Tagged
    0 Thread(s)
    Quote Originally Posted by dek
    Hey ho. Pepejeria's example does not use XMLHTTPRequest, so this is not really relevant.

    I don't know whether it's fixed because MS have fixed the GC handling, or because they've changed the way DOM elements are implemented (I'm hoping for the former) but as Pepejeria is referencing what is probably the most common form of IE memory leakage, it's still good news. Now if only they're release a patch for IE 6 which did the same thing (and then forced the whole world to update) my happiness would be ... improved.
    Sorry, I didn't look at the example, I just assumed it was based off the original post.

  18. #18
    SitePoint Addict dek's Avatar
    Join Date
    Oct 2004
    Location
    UK
    Posts
    352
    Mentioned
    0 Post(s)
    Tagged
    0 Thread(s)
    Quote Originally Posted by Viflux
    Sorry, I didn't look at the example, I just assumed it was based off the original post.
    No worries. The burning question is, of course, is the original problem now a thing of the past?
    Only dead fish go with the flow

  19. #19
    SitePoint Wizard Pepejeria's Avatar
    Join Date
    Jan 2005
    Location
    Too far up north
    Posts
    1,566
    Mentioned
    0 Post(s)
    Tagged
    0 Thread(s)
    A testcase and having the beta installed would answer that

  20. #20
    Employed Again Viflux's Avatar
    Join Date
    May 2003
    Location
    London, On.
    Posts
    1,127
    Mentioned
    0 Post(s)
    Tagged
    0 Thread(s)
    Quote Originally Posted by dek
    No worries. The burning question is, of course, is the original problem now a thing of the past?
    Yes...

    For some reason...

    Code:
    request.onreadystatchange = function(){}
    Didn't work. I suspect that a reference to the request is still maintained by the function due to the scope in which it is declared.

    Code:
    request.onreadystatechange = null;
    Didn't work either. I don't know why.

    Code:
    request.onreadystatechange = BaseClasse.emptyFunction();
    Somewhat stolen from Prototype and it works.

    I don't know nearly enough JS theory to do what my bosses want

  21. #21
    SitePoint Addict dek's Avatar
    Join Date
    Oct 2004
    Location
    UK
    Posts
    352
    Mentioned
    0 Post(s)
    Tagged
    0 Thread(s)
    Interesting. So my own suggestion didn't work?
    Only dead fish go with the flow

  22. #22
    SitePoint Wizard chris_fuel's Avatar
    Join Date
    May 2006
    Location
    Ventura, CA
    Posts
    2,750
    Mentioned
    0 Post(s)
    Tagged
    0 Thread(s)
    Code:
    Test.prototype.init = function()
    {
    	var oThis = this;
    
    	this.elm = document.createElement("div");
    	this.elm.style.cssText = "width:40px; height:40px; background:orange;";
    	this.elm.onclick = function(){ oThis.handleClick(); }
    	document.body.appendChild(this.elm);
    }
    one of the issue I see here is that oThis is declared as a local function variable. Anonymous functions don't have variable closure, thus when oThis is looked up when the anonymous function is actually called, it isn't properly found. I'm almost curious as to whether nuking the var before oThis would solve it.

  23. #23
    SitePoint Wizard Pepejeria's Avatar
    Join Date
    Jan 2005
    Location
    Too far up north
    Posts
    1,566
    Mentioned
    0 Post(s)
    Tagged
    0 Thread(s)
    I am not sure I follow you. The above code does work. With "would solve it", what do you refer to? The memory leak?

  24. #24
    SitePoint Wizard chris_fuel's Avatar
    Join Date
    May 2006
    Location
    Ventura, CA
    Posts
    2,750
    Mentioned
    0 Post(s)
    Tagged
    0 Thread(s)
    I thought you said that code was broke in IE6, and yes, I meant the memory leak.

  25. #25
    SitePoint Addict dek's Avatar
    Join Date
    Oct 2004
    Location
    UK
    Posts
    352
    Mentioned
    0 Post(s)
    Tagged
    0 Thread(s)
    Quote Originally Posted by chris_fuel
    one of the issue I see here is that oThis is declared as a local function variable.
    Which is the whole point. Without that, the code wouldn't work.
    Quote Originally Posted by chris_fuel
    Anonymous functions don't have variable closure,
    Since when?
    Quote Originally Posted by chris_fuel
    thus when oThis is looked up when the anonymous function is actually called, it isn't properly found.
    It is found, and it's the reference to oThis that causes the leak.
    Say you create one of these objects with:
    var x = new Test(1);

    x will have a reference to a div (x.elm)
    x.elm.onclick refers to the anonymous function. Which has a reference to x (in the form of oThis)

    That's the circular reference. SO. If you deference x (x = null) - because one of the objects in the circle is a DOM object, and the others are not, the GC screws up and doesn't collect any of them. Leak. Rinse, repeat, now your misery is complete.

    Quote Originally Posted by chris_fuel
    I'm almost curious as to whether nuking the var before oThis would solve it.
    Unclear on what you mean by that. What var?
    Only dead fish go with the flow


Bookmarks

Posting Permissions

  • You may not post new threads
  • You may not post replies
  • You may not post attachments
  • You may not edit your posts
  •