Opened 9 years ago

Last modified 5 years ago

#10050 open bug

Autocomplete: Destroy potential memory/resource leak

Reported by: scott.k.mitch1 Owned by:
Priority: minor Milestone: none
Component: ui.autocomplete Version: 1.10.4
Keywords: Cc:
Blocked by: Blocking:

Description

It seems like Autocomplete's destroy method may not be making all memory available for GC. I have a [test case](http://jsfiddle.net/WX8wd/) setup to try the potential issue out. I am using google chrome dev tool bar to debug and noticing that there are new elements in the Detatched DOM Tree section of heap snap shots before and after the destroy call is made. After waiting a while and forcing a GC collection the timeline indicates the number of nodes is not returning to the original state. The object's retaining tree indicates that the :not(.ui-menu-item) in function cache() object still may have a reference preventing resources from being re-claimed. Can someone confirm my analysis or explain why this is not an issue?

Change History (11)

comment:1 Changed 9 years ago by scott.k.mitch1

Here is some environment information that may be relevant:

Google Chrome Version 35.0.1916.114 Ubuntu 12.04 x86_64 3.2.0-61-generic

comment:2 Changed 9 years ago by tj.vantoll

Status: newopen

I'm admittedly no expert in using the heap tools in Chrome's DevTools, but what I'm seeing is two "Detached DOM tree" top level entries. One is datepicker related, which is a known problem that we're addressing in the rewrite.

The second one however contains two elements from autocomplete: the live region and the menu—which correspond to the liveRegion and menu variables on the autocomplete instance. The autocomplete widget does call remove() on each of these elements (see https://github.com/jquery/jquery-ui/blob/203b23a87c68312382aed45c0e67df4461c4ebfc/ui/autocomplete.js#L326-327), but perhaps a reference remains that keeps this from being cleaned up?

In local testing if I add this.liveRegion = null to autocomplete's _destroy(), the live region no longer appears as a Detached DOM tree, so I'll mark this issue as valid. The real question to me is: what still has a reference to the instance. scott.k.mitch1, do you have any ideas?

comment:3 Changed 9 years ago by tj.vantoll

Summary: Autocomplete Destroy potential memory/resource leakAutocomplete: Destroy potential memory/resource leak

comment:4 Changed 9 years ago by scott.k.mitch1

Thanks for looking into this issue. I have been evaluating and contributing to a few of other open source packages that have more severe memory leaks for my use-case. Unfortunately this means I have not had time to dive into Autocomplete.

A few general things to look out for are:

  • Clean up any jquery data cache you may be using. It is good practice to accompany any calls to .data('x',y) with a .removeData('x')
  • In general avoid using references to DOM elements in anonymous functions which are defined outside the scope of that anonymous function. Re-select unless you are sure the anonymous function will be made eligible for GC. For example if the anonymous function is used to register for an .on('click', ...) then you want to make sure you do an .off('click', ...) or equivalent call to un-register the event so there is no longer a reference to the anonymous function
  • Avoid storing references to DOM elements in the data cache, or if you must make sure that you clean up after yourself. (inspect line 539 return this._renderItem( ul, item ).data( "ui-autocomplete-item", item );)
Last edited 9 years ago by scott.k.mitch1 (previous) (diff)

comment:5 in reply to:  4 Changed 9 years ago by scott.k.mitch1

Replying to scott.k.mitch1: Hum...I meant to click 'Edit' but instead I clicked 'Reply'. I don't see a way to delete this so just Ignore this reply.

Why the custom issue interface? Doesn't Jira offer free issue hosting to open source projects?

Last edited 9 years ago by scott.k.mitch1 (previous) (diff)

comment:6 Changed 7 years ago by jGeek314

While researching a memory leak in my page that uses autocomplete I found similar issues with a growing detached DOM tree. The more I typed in the autocomplete input, the larger the Detached DOM tree would grow. After a lot of trial and error, I found my issue to be that in the menu.bindings was not being cleared and were referencing the detached DOM objects. By manually clearing them in the search function of the autocomplete:

search: function(e,ui){
     //fix bug in jQuery.ui somewhere where menu.bindings just grows and grows
     autoComplete.data("ui-autocomplete").menu.bindings = $();
},

The detached DOM tree disappeared (presumably because I removed the references to the detached objects).

The issue of the menu.bindings growing can be seen in even the basic autocomplete example:

https://jsfiddle.net/jGeek314/L6wspvnb/

Not sure if the issue I found is exactly the same as what is reported in the ticket, but it seemed close enough to be worth noting here.

Last edited 7 years ago by jGeek314 (previous) (diff)

comment:7 Changed 6 years ago by Scott González

#15095 is a duplicate of this ticket.

comment:8 Changed 6 years ago by eodb

We encountered the same issue in our application. After using the autocomplete widget on large lists (> 500 items) a couple of times, the widgets quickly becomes unusable. The menu bindings are always added (via menu.refresh) but never removed. The workaround mentioned above seems to work without noticeable side effects. Used: Autocomplete v1.12.1, jQuery v3.2.1

comment:9 Changed 6 years ago by sklarej

I can confirm that there is an issue with some kind of leak that has performance implications. You can easily duplicate this using the jQuery Combobox demo by doing the following:

  1. Goto the jQuery UI Autocomplete Combobox demo at https://jqueryui.com/autocomplete/#combobox
  2. Repeatedly press the dropdown menu arrow on the right of the combobox to show all items
  3. After about 20 times (this is dependent on your computer speed) you will start to see a slowdown in the time it takes to display the dropdown list.
  4. Every time you click the dropdown, it will take longer to display
  5. Autocomplete speed is also compromised

While this is not very noticeable with a smaller list of items, it can severely affect the user experience with a larger list.

We have some comboboxes with a large number of items (up to ~500 items). The autocomplete works quickly enough at first, but the more you type in the slower it goes. Even if you just open a jquery combobox dropdown list repeatedly, it gets slower and slower each time. We've had reports of typing in the contents of an autocomplete field taking over a minute.

Workaround: The workaround described in jGeek314's comment about clearing out the menu bindings had allowed us to continue to use the autocomplete control:

autoComplete.data("ui-autocomplete").menu.bindings = $();
Version 0, edited 6 years ago by sklarej (next)

comment:10 in reply to:  9 ; Changed 5 years ago by Colum Clissmann

Replying to sklarej:

I can confirm that there is an issue with some kind of leak that has performance implications as of jQuery UI 1.12.1 / jQuery 3.2.1. ...

Thanks indeed, the workaround works a treat. Would this not be suitable to be included in the jquery ui source?

To be clear, I did this each time the list changed. In a function that loaded the new filtered list I called this

$(inputId).data("ui-autocomplete").menu.bindings = $();

Last edited 5 years ago by Colum Clissmann (previous) (diff)

comment:11 in reply to:  10 Changed 5 years ago by Duane Hutchins

Replying to Colum Clissmann:

Replying to sklarej:

I can confirm that there is an issue with some kind of leak that has performance implications as of jQuery UI 1.12.1 / jQuery 3.2.1. ...

Thanks indeed, the workaround works a treat. Would this not be suitable to be included in the jquery ui source?

To be clear, I did this each time the list changed. In a function that loaded the new filtered list I called this

$(inputId).data("ui-autocomplete").menu.bindings = $();

If it helps anyone, adding this to the open event works great:

$( ".selector" ).autocomplete({
  open: function() { $( this ).data("ui-autocomplete").menu.bindings = $(); }
});
Note: See TracTickets for help on using tickets.