Search and Top Navigation
#15309 new bug ()
Opened August 14, 2018 01:16PM UTC
Last modified October 25, 2018 01:53PM UTC
Memory leak on draggable.disable and enabled
Reported by: | robsdedude | Owned by: | |
---|---|---|---|
Priority: | minor | Milestone: | none |
Component: | ui.draggable | Version: | 1.12.1 |
Keywords: | Cc: | ||
Blocked by: | Blocking: |
Description
I've noticed that repeatedly calling $(...).draggable("disable")
and then $(...).draggable("enable")
causes a memory leak. Please check out this jsFiddle[https://jsfiddle.net/1hz9b4yk/23/] to see it in action or have a look at these screenshots.
- https://chat.opendfki.de/files/qkmjkpfys7nndktwf8c89uojuy/public?h=4IjgAYUsPQ5PloraoXuV6GD9n3wDNa0nZ2t53ATFbTg
- https://chat.opendfki.de/files/qtfarjppzbguzy4kpe3uj3hc4a/public?h=gXc9i6wTB0rXjXlJs9kv5lWDmS5ZxtXFHpInMOIvf1U
- https://chat.opendfki.de/files/c3uhwgwfobyiinoi7uaua5mnyo/public?h=v6JjFTH0ltc1Oa1V7WGCQ98zKoxLtCze9mKRNTVsLgY
- https://chat.opendfki.de/files/kce7x1j8z7ro5jommfzc6t7cfc/public?h=KkKPfQOdZ4olM_0CCfucZC4PJmT1w-nq2u-0ceI1i1U
- Note: screenshot taken on a colleague's MacBook running Chrome.
This happens to me on Linux Mint 18.4 (64 bit) Firefox and Chromium (even though I doubt that any of this is relevant). jQuery-UI is up to date (1.12.1) and jQuery is on version 3.3.1.
I've tracked down the problem and found this
... _setOptions (jquery-ui.js:434) (anonymous) (jquery-ui.js:144) _setOption (jquery-ui.js:2092) _super (jquery-ui.js:129) _setOption (jquery-ui.js:448) _setOptionDisabled (jquery-ui.js:486) _toggleClass (jquery-ui.js:569) _classes (jquery-ui.js:529) _on (jquery-ui.js:575)
So basically the _classes helper method calls ._on
over and over again (jquery-ui.js:529):
this._on( options.element, { "remove": "_untrackClassesElement" } );
while ._on
does two things that consume memory at each call:
- jquery-ui.js:591
this.bindings = this.bindings.add( element );
- jquery-ui.js:622
element.on( eventName, handlerProxy );
while handlerProxy is new function each time.
I came up with a quick-fix but since I'm not too familiar with the whole code base I'm unsure if this breaks anything else. I've replaced the ._on
method with this:
_on: function( suppressDisabledCheck, element, handlers ) { var delegateElement; var instance = this; // No suppressDisabledCheck flag, shuffle arguments if ( typeof suppressDisabledCheck !== "boolean" ) { handlers = element; element = suppressDisabledCheck; suppressDisabledCheck = false; } // No element argument, shuffle and use this.element if ( !handlers ) { handlers = element; element = this.element; delegateElement = this.widget(); } else { element = delegateElement = $( element ); if ( this.bindings.length !== this.bindings.not( element ).length ) { this.bindings = this.bindings.add( element ); } } if (instance._registeredUIHanlders == null) { instance._registeredUIHanlders = {}; } $.each( handlers, function( event, handler ) { if ( !instance._registeredUIHanlders.hasOwnProperty(event) ) { instance._registeredUIHanlders[ event ] = []; } if ( $.inArray( handler, instance._registeredUIHanlders[ event ] ) !== -1 ) { return; } instance._registeredUIHanlders[ event ].push( handler ); function handlerProxy() { // Allow widgets to customize the disabled handling // - disabled as an array instead of boolean // - disabled class as method for disabling individual parts if ( !suppressDisabledCheck && ( instance.options.disabled === true || $( this ).hasClass( "ui-state-disabled" ) ) ) { return; } return ( typeof handler === "string" ? instance[ handler ] : handler ) .apply( instance, arguments ); } // Copy the guid so direct unbinding works if ( typeof handler !== "string" ) { handlerProxy.guid = handler.guid = handler.guid || handlerProxy.guid || $.guid++; } var match = event.match( /^([\\w:-]*)\\s*(.*)$/ ); var eventName = match[ 1 ] + instance.eventNamespace; var selector = match[ 2 ]; if ( selector ) { delegateElement.on( eventName, selector, handlerProxy ); } else { element.on( eventName, handlerProxy ); } } ); },
Attachments (0)
Change History (3)
Changed August 14, 2018 01:18PM UTC by comment:1
description: | I've noticed that repeatedly calling `$(...).draggable("disable")` and then `$(...).draggable("enable")` causes a memory leak. Please check out this jsFiddle[https://jsfiddle.net/1hz9b4yk/23/] to see it in action or have a look at these screenshots. \ * https://chat.opendfki.de/files/qkmjkpfys7nndktwf8c89uojuy/public?h=4IjgAYUsPQ5PloraoXuV6GD9n3wDNa0nZ2t53ATFbTg \ * https://chat.opendfki.de/files/qtfarjppzbguzy4kpe3uj3hc4a/public?h=gXc9i6wTB0rXjXlJs9kv5lWDmS5ZxtXFHpInMOIvf1U \ * https://chat.opendfki.de/files/c3uhwgwfobyiinoi7uaua5mnyo/public?h=v6JjFTH0ltc1Oa1V7WGCQ98zKoxLtCze9mKRNTVsLgY \ * https://chat.opendfki.de/files/kce7x1j8z7ro5jommfzc6t7cfc/public?h=KkKPfQOdZ4olM_0CCfucZC4PJmT1w-nq2u-0ceI1i1U \ * Note: screenshot taken on a colleague's MacBook running Chrome. \ \ This happens to me on Linux Mint 18.4 (64 bit) Firefox and Chromium (even though I doubt that any of this is relevant). \ \ I've tracked down the problem and found this \ \ \ {{{ \ ... \ _setOptions (jquery-ui.js:434) \ (anonymous) (jquery-ui.js:144) \ _setOption (jquery-ui.js:2092) \ _super (jquery-ui.js:129) \ _setOption (jquery-ui.js:448) \ _setOptionDisabled (jquery-ui.js:486) \ _toggleClass (jquery-ui.js:569) \ _classes (jquery-ui.js:529) \ _on (jquery-ui.js:575) \ \ }}} \ \ So basically the _classes helper method calls `._on` over and over again (jquery-ui.js:529): \ \ {{{ \ this._on( options.element, { \ "remove": "_untrackClassesElement" \ } ); \ }}} \ \ while `._on` does two things that consume memory at each call: \ - jquery-ui.js:591 `this.bindings = this.bindings.add( element );` \ - jquery-ui.js:622 `element.on( eventName, handlerProxy );` while handlerProxy is new function each time. \ \ I came up with a quick-fix but since I'm not too familiar with the whole code base I'm unsure if this breaks anything else. I've replaced the `._on` method with this: \ \ {{{ \ _on: function( suppressDisabledCheck, element, handlers ) { \ var delegateElement; \ var instance = this; \ \ // No suppressDisabledCheck flag, shuffle arguments \ if ( typeof suppressDisabledCheck !== "boolean" ) { \ handlers = element; \ element = suppressDisabledCheck; \ suppressDisabledCheck = false; \ } \ \ // No element argument, shuffle and use this.element \ if ( !handlers ) { \ handlers = element; \ element = this.element; \ delegateElement = this.widget(); \ } else { \ element = delegateElement = $( element ); \ if ( this.bindings.length !== this.bindings.not( element ).length ) { \ this.bindings = this.bindings.add( element ); \ } \ } \ \ if (instance._registeredUIHanlders == null) { \ instance._registeredUIHanlders = {}; \ } \ \ $.each( handlers, function( event, handler ) { \ if ( !instance._registeredUIHanlders.hasOwnProperty(event) ) { \ instance._registeredUIHanlders[ event ] = []; \ } \ if ( $.inArray( handler, instance._registeredUIHanlders[ event ] ) !== -1 ) { \ return; \ } \ instance._registeredUIHanlders[ event ].push( handler ); \ function handlerProxy() { \ \ // Allow widgets to customize the disabled handling \ // - disabled as an array instead of boolean \ // - disabled class as method for disabling individual parts \ if ( !suppressDisabledCheck && \ ( instance.options.disabled === true || \ $( this ).hasClass( "ui-state-disabled" ) ) ) { \ return; \ } \ return ( typeof handler === "string" ? instance[ handler ] : handler ) \ .apply( instance, arguments ); \ } \ \ // Copy the guid so direct unbinding works \ if ( typeof handler !== "string" ) { \ handlerProxy.guid = handler.guid = \ handler.guid || handlerProxy.guid || $.guid++; \ } \ \ var match = event.match( /^([\\w:-]*)\\s*(.*)$/ ); \ var eventName = match[ 1 ] + instance.eventNamespace; \ var selector = match[ 2 ]; \ \ if ( selector ) { \ delegateElement.on( eventName, selector, handlerProxy ); \ } else { \ element.on( eventName, handlerProxy ); \ } \ } ); \ }, \ }}} \ \ → I've noticed that repeatedly calling `$(...).draggable("disable")` and then `$(...).draggable("enable")` causes a memory leak. Please check out this jsFiddle[https://jsfiddle.net/1hz9b4yk/23/] to see it in action or have a look at these screenshots. \ * https://chat.opendfki.de/files/qkmjkpfys7nndktwf8c89uojuy/public?h=4IjgAYUsPQ5PloraoXuV6GD9n3wDNa0nZ2t53ATFbTg \ * https://chat.opendfki.de/files/qtfarjppzbguzy4kpe3uj3hc4a/public?h=gXc9i6wTB0rXjXlJs9kv5lWDmS5ZxtXFHpInMOIvf1U \ * https://chat.opendfki.de/files/c3uhwgwfobyiinoi7uaua5mnyo/public?h=v6JjFTH0ltc1Oa1V7WGCQ98zKoxLtCze9mKRNTVsLgY \ * https://chat.opendfki.de/files/kce7x1j8z7ro5jommfzc6t7cfc/public?h=KkKPfQOdZ4olM_0CCfucZC4PJmT1w-nq2u-0ceI1i1U \ * Note: screenshot taken on a colleague's MacBook running Chrome. \ \ This happens to me on Linux Mint 18.4 (64 bit) Firefox and Chromium (even though I doubt that any of this is relevant). jQuery-UI is up to date (1.12.1) and jQuery is on version 3.3.1. \ \ I've tracked down the problem and found this \ \ \ {{{ \ ... \ _setOptions (jquery-ui.js:434) \ (anonymous) (jquery-ui.js:144) \ _setOption (jquery-ui.js:2092) \ _super (jquery-ui.js:129) \ _setOption (jquery-ui.js:448) \ _setOptionDisabled (jquery-ui.js:486) \ _toggleClass (jquery-ui.js:569) \ _classes (jquery-ui.js:529) \ _on (jquery-ui.js:575) \ \ }}} \ \ So basically the _classes helper method calls `._on` over and over again (jquery-ui.js:529): \ \ {{{ \ this._on( options.element, { \ "remove": "_untrackClassesElement" \ } ); \ }}} \ \ while `._on` does two things that consume memory at each call: \ - jquery-ui.js:591 `this.bindings = this.bindings.add( element );` \ - jquery-ui.js:622 `element.on( eventName, handlerProxy );` while handlerProxy is new function each time. \ \ I came up with a quick-fix but since I'm not too familiar with the whole code base I'm unsure if this breaks anything else. I've replaced the `._on` method with this: \ \ {{{ \ _on: function( suppressDisabledCheck, element, handlers ) { \ var delegateElement; \ var instance = this; \ \ // No suppressDisabledCheck flag, shuffle arguments \ if ( typeof suppressDisabledCheck !== "boolean" ) { \ handlers = element; \ element = suppressDisabledCheck; \ suppressDisabledCheck = false; \ } \ \ // No element argument, shuffle and use this.element \ if ( !handlers ) { \ handlers = element; \ element = this.element; \ delegateElement = this.widget(); \ } else { \ element = delegateElement = $( element ); \ if ( this.bindings.length !== this.bindings.not( element ).length ) { \ this.bindings = this.bindings.add( element ); \ } \ } \ \ if (instance._registeredUIHanlders == null) { \ instance._registeredUIHanlders = {}; \ } \ \ $.each( handlers, function( event, handler ) { \ if ( !instance._registeredUIHanlders.hasOwnProperty(event) ) { \ instance._registeredUIHanlders[ event ] = []; \ } \ if ( $.inArray( handler, instance._registeredUIHanlders[ event ] ) !== -1 ) { \ return; \ } \ instance._registeredUIHanlders[ event ].push( handler ); \ function handlerProxy() { \ \ // Allow widgets to customize the disabled handling \ // - disabled as an array instead of boolean \ // - disabled class as method for disabling individual parts \ if ( !suppressDisabledCheck && \ ( instance.options.disabled === true || \ $( this ).hasClass( "ui-state-disabled" ) ) ) { \ return; \ } \ return ( typeof handler === "string" ? instance[ handler ] : handler ) \ .apply( instance, arguments ); \ } \ \ // Copy the guid so direct unbinding works \ if ( typeof handler !== "string" ) { \ handlerProxy.guid = handler.guid = \ handler.guid || handlerProxy.guid || $.guid++; \ } \ \ var match = event.match( /^([\\w:-]*)\\s*(.*)$/ ); \ var eventName = match[ 1 ] + instance.eventNamespace; \ var selector = match[ 2 ]; \ \ if ( selector ) { \ delegateElement.on( eventName, selector, handlerProxy ); \ } else { \ element.on( eventName, handlerProxy ); \ } \ } ); \ }, \ }}} \ \ |
---|
Changed August 14, 2018 02:23PM UTC by comment:2
_comment0: | Oh my quick fix is missing something: One also needs to do some extra clean up in `._off`: \ \ {{{ \ if (this._registeredUIHanlders == null) { \ this._registeredUIHanlders[ eventName ] = []; \ } \ }}} → 1534322180697373 |
---|
Oh my quick fix is missing something: One also needs to do some extra clean up in ._off
:
if (this._registeredUIHanlders != null) { this._registeredUIHanlders[ eventName ] = []; }
Changed October 25, 2018 01:53PM UTC by comment:3
component: | ui.core → ui.draggable |
---|