Skip to main content

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.

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 robsdedude 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 robsdedude 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 scottgonzalez comment:3

component: ui.coreui.draggable