Opened 4 months ago

Last modified 7 weeks ago

#15309 new bug

Memory leak on draggable.disable and enabled

Reported by: Rouven Bauer Owned by:
Priority: minor Milestone: none
Component: ui.draggable Version: 1.12.1
Keywords: Cc:
Blocked by: Blocking:

Description (last modified by Rouven Bauer)

I've noticed that repeatedly calling $(...).draggable("disable") and then $(...).draggable("enable") causes a memory leak. Please check out this jsFiddlehttps://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 );
		}
	} );
},

Change History (3)

comment:1 Changed 4 months ago by Rouven Bauer

Description: modified (diff)

comment:2 Changed 4 months ago by Rouven Bauer

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 ] = [];
}
Last edited 4 months ago by Rouven Bauer (previous) (diff)

comment:3 Changed 7 weeks ago by Scott González

Component: ui.coreui.draggable
Note: See TracTickets for help on using tickets.