Google, you should know better. I fixed your memory leak for you.
This morning, I got a report of our CEO complaining of a memory leak when he left our website up over the weekend. This is a fairly common occurrence as he is often gone for several days at a time, uses IE7, and has decided that our website must refresh itself every 10 minutes. (We don't have ads on our pages, he just wants it to always be up-to-date on people's computers)
When our page was refreshed, we seemed to lose 100k-900k. Left over a weekend it got up to 200mb+ very easily. Sort of a scary issue.
We investigated, and after some digging figured out that our recent addition of Google Analytics was to blame. Surely Google Analytics, used by thousands upon thousands of websites, couldn't be to blame? Plus it's GOOGLE! Google doesn't make mistakes like that. However, after some quick searches, it became pretty obvious we weren't the only ones to discover this.
After we verified that Google Analytics was really at fault, I tried to dig into the source. They obfuscate/compress the code pretty good, so I ran it through an online javascript beautifier to try and figure out waht is going on.
It didn't help much, so we started looking for some likely culprits. Anything ActiveX related, etc. Most of those things you could turn off, and seemed to have to have no effect our memory leak.
And then I found it. Something that Google should just know. The problem with IE has been very well documented before and should be well understood by any javascript developer working today.
Basically, IE has two sets of memory that get garbage collected: the DOM objects and the javascript objects. Serious problems arise when javascript objects reference DOM objects, or those DOM objects somehow reference a javascript object. This latter scenario is often done via callbacks and anonymous functions. The garbage collector doesn't know what to do, and ends up not collecting something it otherwise should.
What does this have to do with Google Analytics? I found two bits of code that followed this pattern:
if (0 == z || 2 == z) {
var A = new Image(1, 1);
A.src = h.Da + l;
var p = 2 == z ?
function() {}: x ||
function() {};
A.onload = p
}
The key bit here is "A.onload = p". They are using an image as a low-budget AJAX call to report stats up to their server, and are apparently doing something upon completion of that call. The problem is that if not properly cleaned up, that image will keep a reference to the Google Analytics object, and neither will get cleaned up properly.
I changed that bit of code to:
if (0 == z || 2 == z) {
var A = new Image(1, 1);
A.src = h.Da + l;
A.onload = function()
{
A.onload = null;
if ((z != 2) && (x != null))
x();
};
}
... and our memory leaks went away.
Seems odd to blame Google for a memory leak in IE....
impressive!
chris, google's code created the memory leak by not using these objects correctly, creating exactly the problems microsoft warns against in the MSDN article that is linked.
please read the article again.
Seems odd for you to have such a love for Google that they can do no wrong...
I thought that IE7 doesn't suffer from this bug. Am I wrong?
They do it on purpose, they want you to use Google Chrome so that they can finally put a face to your IP address.
Well done ;)
But the problem is obviously the IE. Sure Google should have worked around the problem. But who can blame them. Probably everybody over there uses Firefox or Chrome :).
IE is just for Anti-Tech guys.
Shame on you google, shame.
you can only work around a broken design for so long, until you say fuck it and just let it burn! i'd have done the same!
Nice work.
blame IE, not Google! :)
I agree with the IE-bashers... if the bug is documented by MSDN doesn't that pretty prove that it's an IE bug? The title of this post would have been better as "google failed to write a workaround ie's well-known crappy garbage collector"
I think you are right in so far as limited testing of said code would have revealed the memory leak, but the reality is that JS is supposed to be GC'd and I did not detect, always prepared to be proved wrong, any cycle that would thwart the GC.
So IE is at fault in this instance. Doesn't make it OK but I think blaming the Google code is a stretch. Thanks for the knowledge though.
It's not web developers' job to hunt down all the possible memory leaks all the possible browser versions have, and provide workarounds for everything.
The most the should do is report a bug to Microsoft and gently demand them to fix their shit.
Perhaps they did already. Why does your boss use IE7?
Josh, your change modifies the behavior of the block in a subtle but potentially damaging manner. In the original, z == 2 is evaluated before the onload is set to determine whether or not x() is eventually called once the onload is executed. In your modified version, z == 2 is evaluated within the context of the onload.
Depending on the semantics behind z, it's possible that the value of z changed from the point at which the onload was being set to the point where the onload is being executed. If that were to happen, your change could potentially break some other behavior.
Also, blaming IE for the leak may be technically correct, but Google is delivering a product that runs in that environment. If you think your job ends at meeting XHTML and CSS3 standards without concern for whether anything you've done is actually functional in the handful of commonly used browsers out there, you won't be at that job for very long.
Thanks Bart.
z will be the same value when the function evaluates as when it was created. We're creating a closure for the onload event which will retain a copy of all variables it needs to evaluate. Yes, I know this sometimes this isn't true when loops are involved in javascript, but this was in an small-ish function which took z as a parameter and pretty much only set up the Image object.