Search and Top Navigation
#9358 open bug ()
Opened June 03, 2013 11:45PM UTC
Last modified June 06, 2013 12:20PM UTC
Resizable: Memory leak when destroying
| Reported by: | lsching17 | Owned by: | |
|---|---|---|---|
| Priority: | minor | Milestone: | none |
| Component: | ui.resizable | Version: | 1.10.3 |
| Keywords: | Cc: | ||
| Blocked by: | Blocking: |
Description
i create a testcase and access a "dummyObject" by closure during "open" event.
If event handlers are detached after "destroy", then the "dummyObject" should be inaccessible and be garbage-collected.
But chrome heapshot shows that the "dummyObject" is still accessible, and i suspect that the event handlers are not detached
[[Image(https://forum.jquery.com/viewImage.do?fileId=14737000004186517&forumGroupId=14737000000003003)]]
Attachments (0)
Change History (6)
Changed June 05, 2013 12:06AM UTC by comment:1
| component: | ui.core → ui.resizable |
|---|---|
| status: | new → open |
| summary: | event handler of dialog are not detached when destroy? → Resizable: Memory leak when destroying |
Changed June 05, 2013 12:02PM UTC by comment:2
i am not a chrome expert, but placing the mouse on the related object will show the related information
){kind=link}
Changed June 05, 2013 12:24PM UTC by comment:3
Ah yeah, I meant to say
_renderAxisin my previous comment. Still confusing why that would be holding a reference since the instance is gone. That'll help though, thanks.
Changed June 05, 2013 09:24PM UTC by comment:4
| _comment0: | I created a patch for jquery-ui.js (1.10.3) and problem seems fixed, but do not know whether it is good. \ \ http://jsbin.com/iqeyat/18/ \ http://jsbin.com/iqeyat/17/ \ \ {{{ \ --- new.js \ +++ old.js \ @@ -567,37 +567,29 @@ \ _init: $.noop, \ \ destroy: function() { \ - try{ \ - this._destroy(); \ - } finally { \ - // we can probably remove the unbind calls in 2.0 \ - // all event bindings should go through this._on() \ - this.element \ - .unbind( this.eventNamespace ) \ - // 1.9 BC for #7810 \ - // TODO remove dual storage \ - .removeData( this.widgetName ) \ - .removeData( this.widgetFullName ) \ - // support: jquery <1.6.3 \ - // http://bugs.jquery.com/ticket/9413 \ - .removeData( $.camelCase( this.widgetFullName ) ); \ - this.widget() \ - .unbind( this.eventNamespace ) \ - .removeAttr( "aria-disabled" ) \ - .removeClass( \ - this.widgetFullName + "-disabled " + \ - "ui-state-disabled" ); \ - \ - // clean up events and states \ - this.bindings.unbind( this.eventNamespace ); \ - this.hoverable.removeClass( "ui-state-hover" ); \ - this.focusable.removeClass( "ui-state-focus" ); \ - \ - //clean up properties \ - this.element=null; \ - this.options=null; \ - \ - } \ + this._destroy(); \ + // we can probably remove the unbind calls in 2.0 \ + // all event bindings should go through this._on() \ + this.element \ + .unbind( this.eventNamespace ) \ + // 1.9 BC for #7810 \ + // TODO remove dual storage \ + .removeData( this.widgetName ) \ + .removeData( this.widgetFullName ) \ + // support: jquery <1.6.3 \ + // http://bugs.jquery.com/ticket/9413 \ + .removeData( $.camelCase( this.widgetFullName ) ); \ + this.widget() \ + .unbind( this.eventNamespace ) \ + .removeAttr( "aria-disabled" ) \ + .removeClass( \ + this.widgetFullName + "-disabled " + \ + "ui-state-disabled" ); \ + \ + // clean up events and states \ + this.bindings.unbind( this.eventNamespace ); \ + this.hoverable.removeClass( "ui-state-hover" ); \ + this.focusable.removeClass( "ui-state-focus" ); \ }, \ _destroy: $.noop, \ \ @@ -2495,10 +2487,6 @@ \ \ //Initialize the mouse interaction \ this._mouseInit(); \ - \ - //clean up to avoid closure leakage \ - that = null, \ - o = null; \ \ }, \ \ \ }}} \ → 1370473354754472 |
|---|
please delete
Changed June 05, 2013 11:01PM UTC by comment:5
| _comment0: | I created a BAD patch for jquery-ui.js (1.10.3) and linkage seems fixed, but do not know whether the widgets are broken. \ \ http://jsbin.com/iqeyat/18/ \ http://jsbin.com/iqeyat/17/ \ \ {{{ \ --- new.js \ +++ old.js \ @@ -567,37 +567,29 @@ \ _init: $.noop, \ \ destroy: function() { \ - try{ \ - this._destroy(); \ - } finally { \ - // we can probably remove the unbind calls in 2.0 \ - // all event bindings should go through this._on() \ - this.element \ - .unbind( this.eventNamespace ) \ - // 1.9 BC for #7810 \ - // TODO remove dual storage \ - .removeData( this.widgetName ) \ - .removeData( this.widgetFullName ) \ - // support: jquery <1.6.3 \ - // http://bugs.jquery.com/ticket/9413 \ - .removeData( $.camelCase( this.widgetFullName ) ); \ - this.widget() \ - .unbind( this.eventNamespace ) \ - .removeAttr( "aria-disabled" ) \ - .removeClass( \ - this.widgetFullName + "-disabled " + \ - "ui-state-disabled" ); \ - \ - // clean up events and states \ - this.bindings.unbind( this.eventNamespace ); \ - this.hoverable.removeClass( "ui-state-hover" ); \ - this.focusable.removeClass( "ui-state-focus" ); \ - \ - //clean up properties \ - this.element=null; \ - this.options=null; \ - \ - } \ + this._destroy(); \ + // we can probably remove the unbind calls in 2.0 \ + // all event bindings should go through this._on() \ + this.element \ + .unbind( this.eventNamespace ) \ + // 1.9 BC for #7810 \ + // TODO remove dual storage \ + .removeData( this.widgetName ) \ + .removeData( this.widgetFullName ) \ + // support: jquery <1.6.3 \ + // http://bugs.jquery.com/ticket/9413 \ + .removeData( $.camelCase( this.widgetFullName ) ); \ + this.widget() \ + .unbind( this.eventNamespace ) \ + .removeAttr( "aria-disabled" ) \ + .removeClass( \ + this.widgetFullName + "-disabled " + \ + "ui-state-disabled" ); \ + \ + // clean up events and states \ + this.bindings.unbind( this.eventNamespace ); \ + this.hoverable.removeClass( "ui-state-hover" ); \ + this.focusable.removeClass( "ui-state-focus" ); \ }, \ _destroy: $.noop, \ \ @@ -2495,10 +2487,6 @@ \ \ //Initialize the mouse interaction \ this._mouseInit(); \ - \ - //clean up to avoid closure leakage \ - that = null, \ - o = null; \ \ }, \ \ \ }}} \ → 1370473497272920 |
|---|---|
| _comment1: | I created a BAD patch for jquery-ui.js (1.10.3) and linkage seems fixed, but do not know whether the widgets are broken. \ \ http://jsbin.com/iqeyat/18/ \ http://jsbin.com/iqeyat/17/ \ \ {{{ \ --- old.js \ +++ new.js \ @@ -567,29 +567,37 @@ \ _init: $.noop, \ \ destroy: function() { \ - this._destroy(); \ - // we can probably remove the unbind calls in 2.0 \ - // all event bindings should go through this._on() \ - this.element \ - .unbind( this.eventNamespace ) \ - // 1.9 BC for #7810 \ - // TODO remove dual storage \ - .removeData( this.widgetName ) \ - .removeData( this.widgetFullName ) \ - // support: jquery <1.6.3 \ - // http://bugs.jquery.com/ticket/9413 \ - .removeData( $.camelCase( this.widgetFullName ) ); \ - this.widget() \ - .unbind( this.eventNamespace ) \ - .removeAttr( "aria-disabled" ) \ - .removeClass( \ - this.widgetFullName + "-disabled " + \ - "ui-state-disabled" ); \ - \ - // clean up events and states \ - this.bindings.unbind( this.eventNamespace ); \ - this.hoverable.removeClass( "ui-state-hover" ); \ - this.focusable.removeClass( "ui-state-focus" ); \ + try{ \ + this._destroy(); \ + } finally { \ + // we can probably remove the unbind calls in 2.0 \ + // all event bindings should go through this._on() \ + this.element \ + .unbind( this.eventNamespace ) \ + // 1.9 BC for #7810 \ + // TODO remove dual storage \ + .removeData( this.widgetName ) \ + .removeData( this.widgetFullName ) \ + // support: jquery <1.6.3 \ + // http://bugs.jquery.com/ticket/9413 \ + .removeData( $.camelCase( this.widgetFullName ) ); \ + this.widget() \ + .unbind( this.eventNamespace ) \ + .removeAttr( "aria-disabled" ) \ + .removeClass( \ + this.widgetFullName + "-disabled " + \ + "ui-state-disabled" ); \ + \ + // clean up events and states \ + this.bindings.unbind( this.eventNamespace ); \ + this.hoverable.removeClass( "ui-state-hover" ); \ + this.focusable.removeClass( "ui-state-focus" ); \ + \ + //clean up properties \ + this.element=null; \ + this.options=null; \ + \ + } \ }, \ _destroy: $.noop, \ \ @@ -2487,6 +2495,10 @@ \ \ //Initialize the mouse interaction \ this._mouseInit(); \ + \ + //clean up to avoid closure leakage \ + that = null, \ + o = null; \ \ }, \ \ \ }}} \ → 1370521419297862 |
I created a BAD patch for jquery-ui.js (1.10.3) and linkage seems fixed, but do not know whether the widgets are broken.
::diff removed::
Changed June 06, 2013 12:20PM UTC by comment:6
@lsching17 If you'd like to propose a change, please sign our CLA and submit a pull request.
I'm not exactly sure what's going on here but this appears to be a problem with resizable and not dialog. Hit create and then destroy and the dummyObject instance will not be garbage collected: http://jsbin.com/iqeyat/14/edit.
Isching17 do you know if Chrome's dev tools give you any help tracking down line numbers / variable names? Or is it just function names? On this example (http://jsbin.com/iqeyat/14/edit)
is the only specific thing I see and I don't see anything suspicious in there: https://github.com/jquery/jquery-ui/blob/6df5c1a4ae738e591694e0fe2fa3bbb8b05f6b0a/ui/jquery.ui.resizable.js#L874.Thanks.