Memory Leak

[SemiKebab]

Basically every second, you are creating a new XMLHttpRequest object (which is not wrong, provided it is done properly), and my guess is that its "onloadend" listener prevents it from being garbage collected.

Out of curiosity, what results do you get with the following attempts?

Code: Select all

  xhr.onloadend = function(){
    document.getElementById('dst').textContent = xhr.response + (num++);
    xhr = null; // <-- deleting the reference to the XHR object
    setTimeout(onTimer, 1000);
  };

Code: Select all

  xhr.onloadend = function(){
    document.getElementById('dst').textContent = xhr.response + (num++);
    xhr.onloadend = null; // <-- deleting the event listener, which contains a reference to the XHR object
    setTimeout(onTimer, 1000);
  };

Code: Select all

  xhr.onloadend = function(){
    document.getElementById('dst').textContent = xhr.response + (num++);
    xhr.onloadend = null; // <-- combining the 2 previous codes. first, deleting the event listener...
    xhr = null;           // <-- ... then the reference to the XHR object
    setTimeout(onTimer, 1000);
  };

Code: Select all

  xhr.onloadend = function(){
    document.getElementById('dst').textContent = 'DUMMY' + (num++); // removing reference to the XHR object in the event listener
    setTimeout(onTimer, 1000);
  };

You may also be interested by this article: https://nullprogram.com/blog/2013/02/08/

[SemiKebab]

Also refs'ing this question on Stack Overflow: Does Ajax (XMLHttpRequest) objects cleanup after use?

From what I'm understanding, the browsers are theoretically smart enough to manage the codes posted in this thread…

[Moonchild]

Also refs'ing this question on Stack Overflow

Standard use of asynchronous requests and objects is indeed fine. it happens all the time. However, if you attach active code to a method of an XHR, like a timeout with a callback, it will not be cleaned up until that use is not longer active. Trace depth for that is also pretty decent but if you get to the point where you just keep daisy-chaining the next callback with a new XHR with a new timeout with a new callback with... I think you can understand that that is a potential recipe for disaster, even if the XHR request eventually fails (because if the trace depth exceeds what the garbage collector handles, then it will not be collected at all unless there's an actual teardown happening of the owning document after that occurs, and even then "it's complicated")

[vannilla]

From what I'm understanding, the browsers are theoretically smart enough to manage the codes posted in this thread…

Normally they are and the "leak" caused by the XHRs go unnoticed, especially since eventually the browser fixes them on its own.
However in this specific case the "leaking" script us ran on a system with little memory and that's a problem onto itself.

I'd say from this thread we learned that:

• XHR objects should never be created within timers or intervals
• if the callback does not "close over" some variables (i.e everything used by the callback is either locally defined or global like "document") then it should not be an anonymous function

After all, even if the XHR itself is not a problem, the event handler's closure might be!

[Kris_88]

However, if you attach active code to a method of an XHR, like a timeout with a callback, it will not be cleaned up until that use is not longer active.

• XHR objects should never be created within timers or intervals
• if the callback does not "close over" some variables (i.e everything used by the callback is either locally defined or global like "document") then it should not be an anonymous function

Gentlemen, you say strange things, with all my respect for you...
The setTimeout(onTimer, 1000) registers global function as a callback.
This callback will be called in the window context, not in the context of its previous instance.
It is in no way connected with its previous instance.

When the onTimer callback is completed, the entire construction is held in memory because of the running XHR. When the XHR finished then the XHR object with its anonymous callback and the onTimer instance will be freed.

[Moonchild]

oh right, we're no longer talking about Interval here.

It's still a tricky and potentially dangerous construction though.

[SemiKebab]

Indeed, I'm considering Kris_88's version that uses setTimeout(). Using setTimeinterval() is considered an anti-pattern for non-trivial tasks, which may take some time to execute. The recommended pattern is to call the function again using a setTimeout().

And Kris_88 you are right, the inner setTimeout(onTimer, 1000) doesn't persist its parent scope. However, we are relying on the browser to terminate the XHR object, so that its onloadend listener is removed, thus in turn all the references to the XHR object have disappeared, so this object can be garbage collected. Normally, for a long time, browsers are able to do this. If it wasn't the case, a lot of websites would suffer from memory leaks as it's a very common pattern!

That's why I would be interested by the results with the snippets I posted above. Maybe, there is no leak from the beginning and it's just that the GC didn't occur yet. Maybe, manually freeing the references might help the GC to do its job. Thus, results might provide some insight about this.

[Kris_88]

However, we are relying on the browser to terminate the XHR object, so that its onloadend listener is removed, thus in turn all the references to the XHR object have disappeared, so this object can be garbage collected.

A little different. The XHR object+onloadend callback+onTimer instance are unreachable from the root (although they form a cyclic reference). The JS environment could free all this before the XHR completed and the cyclic reference is not an obstacle. However, there is an important reason why it will not be released:
https://xhr.spec.whatwg.org/#garbage-collection
3.2. Garbage collection
An XMLHttpRequest object must not be garbage collected if its state is either opened with the send() flag set, headers received, or loading, and it has one or more event listeners registered whose type is one of readystatechange, progress, abort, error, load, timeout, and loadend.
If an XMLHttpRequest object is garbage collected while its connection is still open, the user agent must terminate the XMLHttpRequest object’s fetch controller.