Memory Leaks in IE7, Prototype or Prototip2?

[mr_justin]

Hey all,
We've been using Prototype for a few years now, always staying up-to-
date with the latest versions and never have really run into any major
memory leaks. We recently integrated Prototip2 into our product
( http://www.nickstakenburg.com/projects/prototip2/ ) and at the same
time we have now started having major problems with memory leaks in
IE7. By major, I mean leaking approximately 2MB for every page
refresh.

I have contacted Nick (author of Prototip), but he has pointed at
Prototype as the source of our leaks, so I figured I'd post here to
see if anyone else is either using Prototip and has experienced
similar issues or maybe can agree with Nick that the problem is indeed
Prototype-related.

Obviously in a large application with lots of JavaScript, there can be
a variety of sources to cause memory leaks and we certainly didn't
rule our own code out as the culprit until A LOT of testing.

Using a memory inspection tool such as Process Explorer, if you
navigate to the Prototip2 demo page (linked above) while using IE7,
every time you refresh the page you should see the memory usage
increasing by approximately 2MB per refresh. I cannot reproduce this
leak on other sites that are using only Prototype (no Prototip).

It was recently brought to my attention that the Element cache in
Prototype does not get cleared out on page unload, so I added code to
do just that. Unfortunately, the leak persists.

Any ideas or help would be greatly appreciated. We have spent a lot of
man hours on this already with several developers and are no closer
now at solving it then when we started. It's extremely frustrating to
say the least.

Thanks guys,
-justin

ps, we are using Prototype 1.6.0.3 and Prototip 2.0.1.3. Due to some
incompatibility issues, we cannot run the more recent version of
Prototip at this point.

[kangax]

On Jan 14, 3:22 pm, mr_justin <gro...@jperkins.otherinbox.com> wrote:

[...]

> It was recently brought to my attention that the Element cache in
> Prototype does not get cleared out on page unload, so I added code to
> do just that. Unfortunately, the leak persists.

Can you show the relevant snippet of how you cleared the cache?

[...]

--
kangax

[mr_justin]

On Jan 14, 3:21 pm, kangax <kan...@gmail.com> wrote:
> Can you show the relevant snippet of how you cleared the cache?

Sure, it came from this thread:
http://prototype.lighthouseapp.com/projects/8886/tickets/322-prototype-16-new-element-constructor

In the Event#destroyCache method, added this:

for (var element in Element.cache) Element.cache[element] = null;

-justin

[mr_justin]

Wanted to update this thread, I just took some screen captures running
9 page refreshes on the Prototip2 demo page, showing an increase in
memory usage with every page refresh.

See image:
http://farm4.static.flickr.com/3370/3198951197_55e5869fbc_o.png

[kangax]

Thanks. That helps.

I just ran this "explorer" on prototype's DOM test page (/test/unit/
tmp/dom_test.html) and it does indeed leak ~1MB every page refresh -
even without prototip.

I don't have much time at the moment. Can you check if `null`ing cache
(by walking `Event.cache` with for..in) or `Event.stopObserving` all
elements in the document helps?

--
kangax

[mr_justin]

Hey kangax,
I just cloned prototype's git source, ran the tests to generate the
HTML test files, then edited the _destroyCache() method to include the
following:

for (var element in Element.cache) Element.cache[element] = null;

(it already includes code to stopObserving all events)

I then loaded up dom_test.html in IE7, with process explorer running,
and refreshed the page 10 times. Initially the memory went up after a
refresh (the first 2 refreshes, memory increased) but then every
refresh after that created no change whatsoever in memory. See below
image (the spikes in the CPU Usage are page loads). You can see that
over time, the memory doesn't even dip down when the page is being
redrawn. Compare that to the previously linked memory usage charts on
the Prototip demo page.

http://sandbox.enjoybeing.net/prototype-dom-test-memory-ie7.jpg

Thanks for your help

-justin

[kangax]

Very good :)
Does prototip still leak after applying this "patch"?

P.S. I could have sworn `stopObserving` loop was removed from the
source at some point (I think right before 1.6.0.3 release). Oh well.

--
kangax

[mr_justin]

Well unfortunately there is a 1MB leak in IE7 when you run the
event_test on the latest version of prototype. Since there is
significant changes in this version compared to the publicly available
1.6.0.3 version, I am guessing you guys are getting close to releasing
1.6.0.4?

FWIW, I took the checked out version (still labeled 1.6.0.3) and made
the following changes to _destroyCache:

function _destroyCache() {
- for (var i = 0, length = CACHE.length; i < length; i++)
+ for (var i = 0, length = CACHE.length; i < length; i++){
Event.stopObserving(CACHE[i]);
+ CACHE[i] = null;
+ }
+
+ for (var element in Element.cache) Element.cache[element] = null;
}

And a re-ran my tests and guess what? No memory leak. Wonderful except
that it looks like a lot of the Event stuff has been re-written so
applying these changes to the publicly available 1.6.0.3 version is
not completely straight-forward. I am still working on patching mine
but should have an update tomorrow by midday.

-justin

[mr_justin]

OK, so it looks like we found the leak. It was in Prototype's
destroyCache method that is registered to the window unload event in
IE only.

As I mentioned before, the Event-related code has changed quite a bit
from what's in the publicly available 1.6.0.3 vs. what's at the head
of the git repository (still called 1.6.0.3). I've actually found that
both code bases need to be patched. In the head of the repository,
Event.stopObserving is called on each element in the cache BUT those
elements are not null'd out after stop observing. Conversely, in the
public downloadable version, Event.stopObserving is NOT called on each
element BUT those elements are null'd out.

Based on our testing, both Event.stopObserving(CACHE[i]) and CACHE[i]
= null is required for adequate cleanup and to avoid memory leaks. I
also added the bit to clear out the Element cache as well for good
measure. Below is a diff that can be applied to the publicly
downloadable 1.6.0.3. This diff can easily be modified to work for the
Event code that is in the git repository.

Begin
patch---------------------------------------------------------------------------------------------------------

Index: /Users/justin/src/***/javascripts/prototype.js
===================================================================
--- /Users/justin/src/***/javascripts/prototype.js (revision xxxxxx)
+++ /Users/justin/src/***/javascripts/prototype.js (working copy)
@@ -3967,6 +3967,7 @@
};

wrapper.handler = handler;
+ wrapper.element = element;
c.push(wrapper);
return wrapper;
}
@@ -3983,9 +3984,16 @@
}

function destroyCache() {
- for (var id in cache)
- for (var eventName in cache[id])
+ for (var id in cache){
+ for (var eventName in cache[id]){
+ cache[id][eventName].each(function(wrapper) {
+ Event.stopObserving(wrapper.element, eventName,
wrapper.handler);
+ });
cache[id][eventName] = null;
+ }
+ }
+
+ for(var id in Element.cache) Element.cache[id] = null;
}

End
patch---------------------------------------------------------------------------

Hope this helps.

-justin

[mocambo]

Thanks for solution.
There is no memory leaks anymore on IE7 and some leaks disappeared
also on IE6. I think you are on the half way and you should test your
program also on IE6, which is still widely useful browser.

GPDE's "Javascript memory leaks detector" finds two type of leaks
probably caused by Prototype:
1. element._prototypeEventID marked as memory leak (circular
reference)
2. All element properties extended by Prototype marked as memory leaks

By doing ...

for (var id in cache){
for (var eventName in cache[id]){
cache[id][eventName].each(function(wrapper) {
Event.stopObserving(wrapper.element, eventName, wrapper.handler);
+ wrapper.element._prototypeEventID = null;

});
cache[id][eventName] = null;
}
}

... removes all leaks of type 1 but causes lots of other leaks of type
2.