#4188 closed bug (patcheswelcome)
Widget: Huge memory leaks for all widgets in IE
Reported by: | paul | Owned by: | |
---|---|---|---|
Priority: | major | Milestone: | 2.0.0 |
Component: | ui.widget | Version: | 1.6rc6 |
Keywords: | Cc: | ||
Blocked by: | Blocking: |
Description (last modified by )
We have huge memory leaks in IE 6, even with the simpliest plugins, so I believe it's related to the widget factory. Tested with Drip 0.5 and /tests/visual/draggable/default.html
Attachments (1)
Change History (53)
comment:1 Changed 14 years ago by
Owner: | set to paul |
---|---|
Status: | new → accepted |
comment:2 Changed 14 years ago by
comment:3 Changed 14 years ago by
Description: | modified (diff) |
---|---|
Summary: | Huge memory leaks for all widgets in all version of IE → Huge memory leaks for all widgets in IE6 |
comment:4 Changed 14 years ago by
Ok, the issue seems to be around the calls to bind() in the widget constructor from line 270-283 in ui.core.js.
comment:5 Changed 14 years ago by
After further debugging, it seems that the memory leaks are directly related to any of the following events: setData, getData, remove. They just need to be bound - they don't even have to be triggered, and can just include an anonymous function.
comment:6 Changed 14 years ago by
Owner: | paul deleted |
---|---|
Status: | accepted → assigned |
comment:7 Changed 14 years ago by
Priority: | blocker → critical |
---|
comment:8 Changed 14 years ago by
Do you mean the binding in this block here? Is it happening
this.element = $(element) .bind('setData.' + name, function(event, key, value) { if (event.target == element) { return self._setData(key, value); } }) .bind('getData.' + name, function(event, key) { if (event.target == element) { return self._getData(key); } }) .bind('remove', function() { return self.destroy(); });
After a quick scan of the code, the only difference with these binds and the others being done is that these are custom event handlers. Would this be a bug with jQuery's custom event handlers?
comment:10 Changed 14 years ago by
All namespaced events leak memory in jQuery 1.3.x. I've created a ticket in core's Trac.
comment:11 Changed 14 years ago by
Milestone: | 1.7 → 1.8 |
---|
comment:12 follow-up: 13 Changed 14 years ago by
Looks like Brandon found a fix, therefore this ticket depends on jQuery's 1.3.3 release.
comment:13 Changed 14 years ago by
Replying to joern.zaefferer:
Looks like Brandon found a fix, therefore this ticket depends on jQuery's 1.3.3 release.
I am using the most recent build of jQuery (r6348) and the problem is still here. Widgets such as dialogs, tabs, and datepicker are leaking. They only seem to leak when *other* *unrelated* html elements are removed from the HTML DOM.
Changed 14 years ago by
Attachment: | ie6-memleaks.patch added |
---|
comment:14 Changed 14 years ago by
the problem is with the event bindings as mentioned above. If the anonymous functions are replaced by normal functions the leaks seem to disappear. Here is a simple test to create 5 memory leaks (tested in sieve):
<div id="leak"></div> <script type="text/javascript">//<![CDATA[ $.widget('ui.test1', { _init : function() { $(document.createElement('div')) .appendTo(this.element) .text('memleak') .test2(); } }); $.widget('ui.test2', { }); $('#leak').test1().remove(); //]]> </script>
Apply the patch attached and test again. the memory leaks should be fixed.
comment:15 Changed 14 years ago by
All three of those bindings will be removed in the new widget factory (http://jquery-ui.googlecode.com/svn/branches/dev/widget-factory/), so updating to that should fix this ticket, too.
We'll probably add some migration-code to make it even easier to use the new widget factory; currently a few minor changes to existing widgets are necessary.
comment:17 Changed 13 years ago by
I confirm that this bug exist for IE8 (jQuery UI 1.7.1).
These tests may be helpful (Tested on IE8).
<script> function doJQueryLeak(){ for(var i=0;i<100;i++){ _createJQueryLeak();} } function doNoJQueryLeak(){ for(var i=0;i<100;i++){ _createNoJQueryLeak();} } function doNoCoreIeEventLeak(){ for(var i=0;i<10000;i++){ _createNoLeakEvent(); }; } function doCoreIeEventLeak(){ for(var i=0;i<10000;i++){ _createLeakEvent(); }; } //private function _createJQueryLeak(){ var item = $("<span>Damn</span>"); //can be any jQuery UI widget item.draggable({ helper: 'clone', revert: 'invalid', zIndex: 2000, addClasses: false}); item.draggable('destroy'); item.unbind(); } function _createNoJQueryLeak(){ var item = $("<span>Damn</span>"); } function _createNoLeakEvent(){ var elm = document.createElement("DIV"); elm.innerHTML = "Some Div"; var myhandler = function(){ elm.yy = 5; } elm.attachEvent("onclick", myhandler); elm.detachEvent("onclick", myhandler); } function _createLeakEvent(){ var elm = document.createElement("DIV"); elm.innerHTML = "Some Div"; var myhandler = function(){ elm.yy = 5; } elm.attachEvent("onSetData", myhandler); elm.detachEvent("onSetData", myhandler); } </script>
comment:21 Changed 13 years ago by
Milestone: | 1.8 → 1.next |
---|
comment:22 Changed 13 years ago by
Its been a while... did jQuery 1.4 or jQuery UI 1.8 make any difference?
The related core ticket got closed a year ago: http://dev.jquery.com/ticket/4241
comment:23 Changed 13 years ago by
Priority: | critical → major |
---|
comment:24 Changed 13 years ago by
Component: | ui.core → ui.widget |
---|
comment:25 Changed 12 years ago by
In 1.8.9, there is a memory leak in ui.widget.js. I created a test case here http://jsfiddle.net/hEnFt/.
The gist of it is here:
<div id="container"> <div id="inner">inner div</div> </div> <hr/> <a href="#" id="clear">clear</a> <script type="text/javascript"> $("#inner").test(); $("#clear").click(function(event) { event.preventDefault(); $("#inner").test("destroy"); $("#container").html(""); }); </script>
Basically, on click of the link, I empty the container div. But before that I call destroy on the widget. I can see that there is a single leak in sIEve. I can trace the cause of the leak to this call in _createWidget:
var self = this; this.element.bind( "remove." + this.widgetName, function() { self.destroy(); });
If I comment out the bind, the leak goes away. I tested separately for binding/unbinding of custom events, with and without namespace, jquery does not seem to have an issue. It must be a circular reference not being cleaned up in the widget code but can't figure it out for myself.
comment:26 Changed 12 years ago by
Status: | assigned → open |
---|
jihohan: thanks for looking into this. I've recently changed that bit of code, can you retest against master and see if the leak still exists?
comment:28 Changed 12 years ago by
This is an additional observation regarding my code sample at http://jsfiddle.net/hEnFt/.
If I create another div outside of #container, and use the widget on it, then that starts leaking too.
<div id="container"><div id="inner"></div></div> <hr/> <div id="unrelated">
If I create the widget on both #inner and #unrelated and then I take out the #inner from the document, both #inner and #unrelated leak. So it seems if I remove any widget from the document, all widgets leak.
This happens on both 1.8.9 and the latest.
comment:29 Changed 12 years ago by
One more observation is that if you remove the bind call for "remove" event from _createWidget() and do bind outside the widget, then there is no leak. In other words, you do this:
$("#inner").test() .bind("remove.test", function() {});
Another interesting thing I noticed, as I dug into jquery 1.4.4, is that I found the following block of code in jQuery.event.add:
if ( elem.addEventListener ) { elem.addEventListener( type, eventHandle, false ); } else if ( elem.attachEvent ) { elem.attachEvent( "on" + type, eventHandle ); //elem.detachEvent( "on" + type, eventHandle ); // added by me }
So, if I comment out attachEvent call, there is no leak, and no surprise there. But if I uncomment the following line and detach the handler right away, it leaks. So the leak isn't coming from not unbinding or not cleaning up after ourselves. I believe it's some type of closure related leak that gets created at the point of attachEvent. I mean I've created jquery plugins (not jquery.ui) with event bindings but I've managed to create them leak free but none of my plugins have complex closures that jquery.ui does.
Finally, there is also no leak to speak of, until I remove a widget element from the document. If I never remove any elements, so that there is no ajax-y DOM manipulations stuff and if there are only whole page loads, there is no leak.
For example, I tested the leak patterns from Understanding and Solving Internet Explorer Leak Patterns, and I could not see any memory leaks because none of them involve removing an element from the document. Perhaps those particular leaks were fixed by service packs, seeing as that document is pretty old, circa 2005.
comment:30 Changed 12 years ago by
Testing with release 1.5 of jQuery, the number of leaks shown in sIEve for the test sample increased from 1 to 4. I'm not exactly sure what that number is supposed to mean but it can't be good.
comment:31 Changed 12 years ago by
sIEve is telling me that there are no leaks, even for older versions.
comment:32 follow-up: 33 Changed 12 years ago by
I still see leaks for IE7. Put http://jsbin.com/ekefov in sIEve and every refresh leaks 15 dom elements.
For the same page, sIEve report no leaks on IE8.
comment:33 follow-up: 34 Changed 12 years ago by
Replying to garyz:
I still see leaks for IE7. Put http://jsbin.com/ekefov in sIEve and every refresh leaks 15 dom elements.
For the same page, sIEve report no leaks on IE8.
This ticket is for memory leaks in all widgets because all widgets use $.ui.widget core, so tests for this ticket must use little more than $.ui.widget. For examples see earlier comments, such as http://bugs.jqueryui.com/ticket/4188#comment:25 .
Your test shows a leak in Dialog, so it should be added to #4565
comment:34 Changed 12 years ago by
Replying to rdworth:
Replying to garyz:
I still see leaks for IE7. Put http://jsbin.com/ekefov in sIEve and every refresh leaks 15 dom elements.
For the same page, sIEve report no leaks on IE8.
This ticket is for memory leaks in all widgets because all widgets use $.ui.widget core, so tests for this ticket must use little more than $.ui.widget. For examples see earlier comments, such as http://bugs.jqueryui.com/ticket/4188#comment:25 .
Your test shows a leak in Dialog, so it should be added to #4565
Sorry, should have read all the comments. I took out the dialog widget and replaced it with an empty widget. still shows 1 leak for IE7, okay for IE8. http://jsbin.com/ekefov/2/
comment:37 Changed 11 years ago by
First post guys, but we had massive memory leaks with jQuery UI in IE6 recently. We were using release 1.8.9 and found that dialog boxes would leak 6-10meg of memory. We also found that roughly ten custom event handlers (which used the method "bind") leaked 3-4meg per page. None of this memory was released on page submit.
We upgraded to a custom jQuery UI build of 1.8.21 (which included accordians, dialogs, and the core only) and it now works like a charm: 6-10meg of memory is still being used for dialogs, but it is released on page submit, and no memory is leaking relating to custom event handling.
Phew! Cheers guys.
comment:38 Changed 11 years ago by
Is anyone else still seeing a leak? If not, I'll close this as fixed in a few days.
comment:39 Changed 11 years ago by
I'm going to close this on Friday unless someone else can confirm or deny jmstreet's claim.
comment:40 Changed 11 years ago by
Issue from ticket #7808: http://jsfiddle.net/Bq7tJ/3/ is still reproducible. Why have you closed the original issue and don't have accepted the pull request: https://github.com/jquery/jquery-ui/pull/513?
comment:41 follow-up: 42 Changed 11 years ago by
@NikGovorov That ticket was closed as a duplicate. I didn't accept the pull request because it completely breaks widgets.
comment:42 follow-up: 43 Changed 11 years ago by
Replying to scott.gonzalez:
@NikGovorov That ticket was closed as a duplicate. I didn't accept the pull request because it completely breaks widgets.
Clear. Anyway all widgets leak in ie 7 and ie 8(didn't try in ie 6) with 1.8.21, even if create own simple(empty) widget.
comment:43 follow-up: 45 Changed 11 years ago by
Replying to NikGovorov:
Replying to scott.gonzalez:
@NikGovorov That ticket was closed as a duplicate. I didn't accept the pull request because it completely breaks widgets.
Clear. Anyway all widgets leak in ie 7 and ie 8(didn't try in ie 6) with 1.8.21, even if create own simple(empty) widget.
The same for the current version(master branch). http://jsfiddle.net/8nbN2/
comment:44 follow-up: 48 Changed 11 years ago by
I'm seeing @jmstreet's issue as well.
On IE7 using sIEve. With a single jQuery bind call I get a leak on the bound element. Just the presence of jQuery.ui and the bind call triggers the problem. There are no widget instances on the page.
A test case is at: http://jsfiddle.net/johnhunter/2LLD9/ (running this locally to avoid effects from jsfiddle).
When the page is reloaded the bound element is leaked. Loading the test page in sIEve and activating 'auto-refresh' shows a leak for the bound element on each page reload.
Can replicate with jQuery UI 1.8.21, 19, 18 and jQuery 1.7.2 - 1.6.2.
comment:45 Changed 11 years ago by
Replying to NikGovorov:
Replying to NikGovorov:
Replying to scott.gonzalez:
@NikGovorov That ticket was closed as a duplicate. I didn't accept the pull request because it completely breaks widgets.
Clear. Anyway all widgets leak in ie 7 and ie 8(didn't try in ie 6) with 1.8.21, even if create own simple(empty) widget.
The same for the current version(master branch). http://jsfiddle.net/8nbN2/
The issue disappears if use jquery 1.8.
comment:46 follow-up: 47 Changed 11 years ago by
I can confirm that, based on the original test: http://jsfiddle.net/Ds7Jv/8/
However there is a bigger leak that occurs if the page is reloaded (using Jquery 1.8b2): http://jsfiddle.net/Ds7Jv/14/
comment:47 Changed 11 years ago by
Replying to stumblor:
I can confirm that, based on the original test: http://jsfiddle.net/Ds7Jv/8/
However there is a bigger leak that occurs if the page is reloaded (using Jquery 1.8b2): http://jsfiddle.net/Ds7Jv/14/
Further testing - No leak in JQuery 1.4.4 JQuery UI 1.8.7 http://jsfiddle.net/Ds7Jv/17/
comment:48 Changed 11 years ago by
Replying to jh.huddle:
On IE7 using sIEve. With a single jQuery bind call I get a leak on the bound element. Just the presence of jQuery.ui and the bind call triggers the problem. There are no widget instances on the page.
This is due to a jQuery bug http://bugs.jquery.com/ticket/12171. Creating a disconnected element and then binding an event handler to it causes all subsequent bind calls to leak their elements. This is triggered in the Datepicker where the bindHover function binds event handlers to dpDiv which is a detached element.
comment:49 Changed 11 years ago by
Milestone: | 1.next → 2.0.0 |
---|
comment:51 Changed 11 years ago by
Summary: | Huge memory leaks for all widgets in IE6 → Widget: Huge memory leaks for all widgets in IE |
---|
Dropping the "6" from the title since support is being dropped in 1.10. There is talk in this ticket of leaks in IE7 and IE8 that should be verified with 1.9.
comment:52 Changed 10 years ago by
Can anyone reproduce this with jQuery 1.9.0 and jQuery UI 1.10.0?
comment:54 Changed 9 years ago by
I can reproduce this with jQuery 1.9.1, jQueryUI 1.10.3 on IE7 using the jquery UI demo site. Memory usage keeps climbing when I switch between the different widget pages.
comment:55 Changed 9 years ago by
Resolution: | → patcheswelcome |
---|---|
Status: | open → closed |
We haven't been able to track this down and fix it, so this has just been sitting. At this point, we're just going to close this ticket, but we'd gladly accept a patch if someone can figure out what's causing the leak.
First find: Does not happen for progressbar widget