Skip to content
This repository has been archived by the owner on Apr 12, 2024. It is now read-only.

ngSanitize 1.5.0 triggers Internet Explorer memory leak #13800

Open
PhilipWallin opened this issue Jan 19, 2016 · 26 comments
Open

ngSanitize 1.5.0 triggers Internet Explorer memory leak #13800

PhilipWallin opened this issue Jan 19, 2016 · 26 comments

Comments

@PhilipWallin
Copy link

The changes from v1.4.8 to v1.5.0 of angular-sanitize triggers a memory leak in our project. When using Internet Explorer 11 with other javascript libraries each pageload will increase the memory usage, until the browser stop working. I've put together an example project which just have the minimum amounts of script tags in the header to reproduce the problem. Every time I refresh the page Internet Explorer 11 will leak memory, and will eventually (at around 1.2GB memory usage for me) get random renderer and javascript errors. If I remove either of "es6-shim.js", "kendo.all.min.js", or "angular-sanitize.js" the memory leak wont happen. In the example project I have simplified the angular-sanitize code as much as I could to show what functionality is triggering the memory leak for me.

(As the file attachment seems to be broken, here is a link to the example project: https://dl.dropboxusercontent.com/u/775659/Leaker.zip)

Notes:

  • If the IIFE in my simplified script is removed the memory leak wont happen.
  • If "createHTMLDocument" isn't called in an inner function the leak wont happen.
  • If "documentElement" isn't accessed on the newly created HTML document the leak wont happen.
  • If an angular module isn't registred the leak wont happen.
  • When changing any of the conditions, for example using angular-sanitize v1.4.8, memory usage could of course still increase. The difference being that the memory is temporary, will be capped, and will clear if you go to another page. In my example project the leak wont cap, and will remain even if you leave the page.
@Narretz
Copy link
Contributor

Narretz commented Jan 19, 2016

Thank for the detailed write-up. I can confirm that there's a memory leak, and that the steps to prevent it work. I find removing the IIFE especially interesting. I've also experienced that the leak isn't as bad if you simply remove the createThingy function and create the doc straight like this var thingy = window.document.implementation.createHTMLDocument("doccy")
So it looks like IE11 doesn't like it when this is inside a function.
I wonder, how big of an increase do you see per page load? For me it was around 20MB in your initial example, and with the fn removed around 10MB.
Maybe we can remove the IIFE in the sanitize code, too. However, I think there's always one outer IIFE around the whole sanitize module, so we might not get it that way.

You should definitely open a bug report about this at MSDN Connect. They don't have a great track record with responses, but it's better than nothing.

@Narretz
Copy link
Contributor

Narretz commented Jan 19, 2016

The only reliable solution I've found is indeed to remove all IIFE from the angular sanitize code (there are 2). But we can't remove the outer one, because the keeps all our stuff private.

@Narretz
Copy link
Contributor

Narretz commented Jan 19, 2016

Another thing, are you using Kendo Professional or Open Source Edition?

@PhilipWallin
Copy link
Author

In my tests the memory growth is around 25-30 MB each refresh. I'm using Kendo Professional in the project.

In my simplified script example I can add the whole inner IIFE to trigger the leak, and add just the code of the inner IIFE to avoid the leak. This, however, does not seem to work as easily on the full angular-sanitize.

I will report this to Microsoft too, but I figured that was... more of a long term solution :)

@Narretz
Copy link
Contributor

Narretz commented Jan 20, 2016

The thing is that the trigger isn't that common (since you need both kendo ui and ES6 Shim), so I'd like to refrain from trying some brute force approach in ngSanitize. It would be interesting to find out what exactly triggers it in ES6 Shim / Kendo. Problem is that they their distributables are huge.

@PhilipWallin
Copy link
Author

I figured as much, which is why it's really annoying finding a bug which is triggered by three different libraries. I considered binary searching for triggers in the other projects too, but deemed it to not be worth the time, as even if I found that X + Y + Z triggers a problem it would in the end still be an IE bug. In best case I could prove it's a common scenario, which would make at least one of the libraries want to avoid it.

@petebacondarwin
Copy link
Member

I don't think that this issue should block release of 1.5.0 - I am moving it to the 1.5.x milestone.

@petebacondarwin petebacondarwin modified the milestones: 1.5.x - migration-facilitation, 1.5.0-rc.2 Jan 22, 2016
@PhilipWallin
Copy link
Author

There shouldn't be any problem running older version of sanitize (1.4.8) with newer Angular (1.5.0), right? I guess this could change in the future, but I need a short term solution for now.

@petebacondarwin
Copy link
Member

You are correct @PhilipWallin

@PhilipWallin
Copy link
Author

From what I understand this issue is caused by circular references in the Javascript, maybe circular reference through closures? https://msdn.microsoft.com/en-us/library/bb250448(v=vs.85).aspx

@petebacondarwin
Copy link
Member

That is often the cause of memory leaks but the trick is how to find those circular references, when you have three libraries involved!

@PhilipWallin
Copy link
Author

I cannot really say I completely understand the problem, but it might be useful for you to know that downgrading Sanitize version didn't really solve my problem, I only thought it did. It did solve the problem where pressing F5 would retain memory, but moving between pages still do increase memory consumption until crash. For you it might be good to know that in some cases the current code structure could cause this behavior in IE11. I've kind of given up on the problem due to its complexity.

@sjulescu
Copy link

I’m facing the same situation. Any proposed solution or workaround available?
Thanks

@petebacondarwin
Copy link
Member

@sjulescu - can you provide a running example?

@sjulescu
Copy link

sjulescu commented Oct 4, 2016

We have a large application, so providing an example is more complicated. I can provide the result of our analyses and please let me know if the example is still necessary.
We are using AngularJS v1.5.6 and the leak is reproducible only on IE11.

So, we have a large application with several pages, on each page we are loading more than 50 scripts. We've seen that loading a page is causing huge memory leaks. After analyzing we realized that the combination of: angular-sanitize.js and jquery.i18n.properties-1.0.9.js are causing the leaks.
Going further with the analyze for angular-sanitize we identified that the declaration of
var inertBodyElement as global variable is causing the leak. To be 100% sure we temporary added in the angular-sanitize a function to nullify the inertBodyElement that we call later from our code when navigating from the page. Something like:
window.cleanInertBodyElement = function() {
inertBodyElement = null;
}
The result was that the memory leak is gone.

A similar thing we identified in the jquery.i18n.properties-1.0.9 with a global variable: cbSplit.

@petebacondarwin
Copy link
Member

Are you saying that the leak only occurs if "both" inertBodyElement and cbSplit are in the global scope?

@sjulescu
Copy link

sjulescu commented Oct 4, 2016

At the beginning we were thinking that the combination is causing the leak, but the combination is just amplifying it.
We have the leak using only angular-sanitize or only jquery.i18n.properties-1.0.9.

@petebacondarwin
Copy link
Member

OK, let me look into that.

@petebacondarwin petebacondarwin self-assigned this Oct 4, 2016
@nathand8
Copy link

Also having this problem. It's beginning to affect users to where they have to close IE 11 every hour.

Any updates? @petebacondarwin

@gkalpak
Copy link
Member

gkalpak commented Jan 17, 2017

Does anyone have a reproduction using just angular-sanitize (which @sjulescu mentioned is enough to cause the leak)? Using @PhilipWallin's .zip (from #13800 (comment)), I can only see the 25MB-30MB increase in total memory even without angular or angular-sanitize. It seems to be mostly caused by kendo-ui (although es6-shim amplifies it a bit).

@theaspect
Copy link

theaspect commented Feb 8, 2017

Solution by @sjulescu adding this to angular-sanitize.js 1.5.8 seems fix an issue

window.cleanInertBodyElement = function() {
    inertBodyElement = null;
};

window.onunload = function() {
    cleanInertBodyElement();
}

Also in onUnload event iterate all objects in windows and document scope and manually delete it

@theaspect
Copy link

Closed PR because seizuring onUnload is a bad idea

@larryolsson
Copy link

Hi, we are also experiencing this problem. Again it is difficult to produce an example since the app is large and complex. Any updates?

@gkalpak
Copy link
Member

gkalpak commented Mar 7, 2017

We can't do much without an actual reproduction (without 3rd-party dependencies).

@Narretz Narretz modified the milestones: 1.5.x, 1.6.x Mar 31, 2017
@tizub
Copy link

tizub commented Apr 3, 2017

Hi.
The problem seems to be present only when there is a polyfill on the page (ES6 in my case), along with angular-sanitize. I tried with many different polyfill implementations I could find, all of them triggered the issue: core-js, es6-shim, etc.
As Angular 2 requires one (core-js suggested by the docs), we can't have an Angular 1 app in 1.5.x and an Angular 2 app in the same page without having this issue. In a way, this might not be considered as "using third party libraries"...
Attached is a minimal example app to illustrate this bug.
Hope it helps!
ie-memory-leak.zip

@gkalpak
Copy link
Member

gkalpak commented Apr 11, 2017

Not sure how using a 3rd party lib can be considered "not using a 3rd party lib". It is not an issue of terminology. The problem with issues appearing only in the presense of 3rd party libs is that the problem is likely to be in that lib ang not AngularJS, which makes is very difficult for us to debug and identify.

@Narretz Narretz modified the milestones: 1.6.x, 1.7.x Apr 12, 2018
@petebacondarwin petebacondarwin removed their assignment May 14, 2018
@petebacondarwin petebacondarwin modified the milestones: 1.7.x, 1.7.x - won't fix May 16, 2018
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Projects
None yet
Development

Successfully merging a pull request may close this issue.

9 participants