Skip to main content

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.

http://jsbin.com/iqeyat/3

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 tj.vantoll comment:1

component: ui.coreui.resizable
status: newopen
summary: event handler of dialog are not detached when destroy?Resizable: Memory leak when destroying

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)

_alsoResize
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.

Changed June 05, 2013 12:02PM UTC by lsching17 comment:2

i am not a chrome expert, but placing the mouse on the related object will show the related information

[[Image(http://s3.postimg.org/fcwkfg7qr/line_number.png)]]

Changed June 05, 2013 12:24PM UTC by tj.vantoll comment:3

Ah yeah, I meant to say

_renderAxis
in 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 lsching17 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 lsching17 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.

http://jsbin.com/iqeyat/18/

http://jsbin.com/iqeyat/17/

::diff removed::

Changed June 06, 2013 12:20PM UTC by scottgonzalez comment:6

@lsching17 If you'd like to propose a change, please sign our CLA and submit a pull request.