Skip to content

memory leak ui.widget #513

Closed
wants to merge 9 commits into from

3 participants

@Y-Kamata
Y-Kamata commented Nov 2, 2011

Widget: modified _createWidget method and added __destroy method.
Fixed #7808 - ui.widget Memory (private bytes of IE process) increases when create and destroy of the simple widget are repeated.

master code memory leak test
http://jsfiddle.net/NzJGH/5/

my code memory leak test (fixed)
http://jsfiddle.net/yoshi6jp/NzJGH/11/

Y-Kamata added some commits Nov 2, 2011
@Y-Kamata Y-Kamata ui.widget: modified _createWidget method and added __destroy method. …
…Fixed #7808 - ui.widget Memory (private bytes of IE process) increases when create and destroy of the simple widget are repeated.
d2e603d
@Y-Kamata Y-Kamata bug fix d02ac44
@Y-Kamata Y-Kamata bugfix __destroy a2df8e8
@Y-Kamata Y-Kamata modified _createWidget 40ae50b
@scottgonzalez
jQuery Foundation member

Can you explain what's causing the memory leak? I really don't want to land this fix without knowing, because if the solution is to not use ._bind() then I feel like that leaves open a huge possibility for future leaks.

@Y-Kamata
Y-Kamata commented Nov 2, 2011

The reference of the variables ( element, instance) of the _bind method remains.

fix code
Y-Kamata@c6fc850#ui/jquery.ui.widget.js

test code
http://jsfiddle.net/yoshi6jp/NzJGH/12/

@scottgonzalez scottgonzalez and 1 other commented on an outdated diff Nov 2, 2011
ui/jquery.ui.widget.js
@@ -322,8 +322,13 @@ $.Widget.prototype = {
$( this ).hasClass( "ui-state-disabled" ) ) {
return;
}
- return ( typeof handler === "string" ? instance[ handler ] : handler )
- .apply( instance, arguments );
+ //return ( typeof handler === "string" ? instance[ handler ] : handler )
+ // .apply( instance, arguments );
+ // bugfix memory leak http://bugs.jqueryui.com/ticket/7808
+ var ret = ( typeof handler === "string" ? instance[ handler ] : handler )
+ .apply( instance, arguments );
+ instance = null;
@scottgonzalez
jQuery Foundation member
scottgonzalez added a note Nov 2, 2011

Doesn't this break the event handler for future events?

@Y-Kamata
Y-Kamata added a note Nov 2, 2011

There is no problem because it makes it to null after the event in the future is executed.
function handlerProxy() {
:
instance = null;
retrun ret;
}

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
@scottgonzalez
jQuery Foundation member

I'm not actually seeing any difference using sIEve (and it's not reporting any leaks before or after your change).

@Y-Kamata
Y-Kamata commented Nov 2, 2011

Performance monitor result. (Windows XP SP3 + IE8)

iexprloer#1 (fix code)
http://jsfiddle.net/yoshi6jp/NzJGH/12/
"55463936" --> "54935552" (not increase)

iexplorer#2 (master code)
http://jsfiddle.net/NzJGH/5/
"54763520" --> "64102400" (increase)

"(PDH-CSV 4.0) ()(-540)","\KAMATAPC2\Process(iexplore#1)\Private Bytes","\KAMATAPC2\Process(iexplore#2)\Private Bytes"
"11/03/2011 08:38:19.406","23760896","23486464"
"11/03/2011 08:38:34.406","23760896","23486464"
"11/03/2011 08:38:49.406","55463936","23560192"
"11/03/2011 08:39:04.406","55463936","54763520"
"11/03/2011 08:39:19.406","55463936","54960128"
"11/03/2011 08:39:34.406","55263232","54960128"
"11/03/2011 08:39:49.406","55263232","54960128"
"11/03/2011 08:40:04.406","55263232","55091200"
"11/03/2011 08:40:19.406","55263232","55611392"
"11/03/2011 08:40:34.406","55263232","55762944"
"11/03/2011 08:40:49.406","55263232","56188928"
"11/03/2011 08:41:04.406","55263232","56631296"
"11/03/2011 08:41:19.406","55263232","57049088"
"11/03/2011 08:41:34.406","55197696","58359808"
"11/03/2011 08:41:49.406","55197696","58294272"
"11/03/2011 08:42:04.406","55197696","58433536"
"11/03/2011 08:42:19.406","55197696","58748928"
"11/03/2011 08:42:34.421","55197696","59228160"
"11/03/2011 08:42:49.421","55197696","59400192"
"11/03/2011 08:43:04.421","55197696","60264448"
"11/03/2011 08:43:19.421","55132160","60547072"
"11/03/2011 08:43:34.421","55132160","60751872"
"11/03/2011 08:43:49.421","55066624","61255680"
"11/03/2011 08:44:04.421","55066624","61505536"
"11/03/2011 08:44:19.421","55066624","62062592"
"11/03/2011 08:44:34.421","55066624","62377984"
"11/03/2011 08:44:49.421","55066624","62869504"
"11/03/2011 08:45:04.421","55066624","63242240"
"11/03/2011 08:45:19.421","54935552","63799296"
"11/03/2011 08:45:34.421","54935552","64102400"

@jzaefferer
jQuery Foundation member

We've never been able to recreate the issue and we'll drop support for IE6 in 1.10.

@jzaefferer jzaefferer closed this Oct 22, 2012
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Something went wrong with that request. Please try again.