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