Skip to main content

Search and Top Navigation

#10056 closed bug (fixed)

Opened May 21, 2014 07:30PM UTC

Closed August 21, 2014 01:07PM UTC

Last modified September 12, 2014 10:45AM UTC

Tabs: "manipulation" example leaks memory

Reported by: rosenfeld Owned by: rosenfeld
Priority: minor Milestone: 1.11.2
Component: ui.tabs Version: 1.10.4
Keywords: Cc:
Blocked by: Blocking:
Description

Try this example: http://jqueryui.com/tabs/#manipulation

Closing a tab works by removing the li and respective panel elements and calling 'refresh' on the tabs component.

But by inspecting $('#tabs').data('uiTabs'), you'll notice that there are still references to the removed elements in 'hoverable', 'focusable' and 'binding' properties at least.

Our application heavily depends on dynamic tabs support and fixing this leak would be very helpful. Unfortunately I have no idea on how to properly fix this.

Attachments (0)
Change History (13)

Changed May 21, 2014 07:37PM UTC by scottgonzalez comment:1

component: ui.coreui.tabs
owner: → rosenfeld
status: newpending

How big of an issue is this for you? Widgets don't manually track event bindings, the widget factory does it automatically and only does cleanup on destroy.

Changed May 21, 2014 07:54PM UTC by rosenfeld comment:2

status: pendingnew

Here's how my application works. Users search for deals and open the ones they are interested as new tabs. Once they are done with it, they will close the deals.

When we display a deal, it uses lots of memory, and we also bind several events to elements in the tab panel, which should be automatically unbound once we call remove() on the li and panel. But since the li and panel elements are leaking, all of the DOM we have created for each deal remains in the memory even after the user closes the deals.

Since this is an SPA, user's usually don't leave our application for a long period of time. If they open too many tabs, they will get a big increase in their memory usage due to this leak.

I can't destroy the tabs component itself since this is the first thing that is created in the application and holds all of the parts of our application as tab panes (most are fixed, but the deals are dynamic ones).

The leak goes really fast for our application since each deal tab will use a fair amount of memory.

Changed May 25, 2014 08:14PM UTC by petersendidit comment:3

Seems like the correct way to solve this is to handle removing elements from hoverable, focusable, and bindings in the _off method in widget. This would require _off to loop over each of the elements being passed in and see if those elements still had a namespaced event for the widget and then remove them from the correct lists. Would make _off far more costly to call.

Some like this would solve the problem: https://gist.github.com/petersendidit/4a4cb89654e88433eecd

Don't know if the cost of doing all that work is worth it for all widgets.

Changed May 27, 2014 01:26PM UTC by tj.vantoll comment:4

summary: jQuery UI tabs "manipulation" example leaks memoryTabs: "manipulation" example leaks memory

Well, even if

_off()
cleans up those bindings, you still need a place to call
_off()
, and it seems like the appropriate place to do that is
refresh()
.

We can build an extension as a POC to see how messy the cleanup would be.

Changed May 27, 2014 01:38PM UTC by tj.vantoll comment:5

Here's an example that uses petersendidit's

_off()
, and defines a
removeTab()
method that cleans up after itself: http://jsfiddle.net/tj_vantoll/Cwttg/.

Here's a thought: could we just unbind all previously attached events in

refresh()
, then re-bind them? Or would that have side effects that I'm not thinking of? It seems like we should come up with some solution to this, as it is a problem with any of our widgets that have a
refresh()
method.

Changed May 27, 2014 03:03PM UTC by rosenfeld comment:6

I'd just like to point out that even the example in jsfiddle won't fix the detached elements leak.

After clicking on the remove button for the first time, you can still get the LI element through:

$('#tabs').data('uiTabs').bindings.prevObject.prevObject.prevObject.prevObject.prevObject.prevObject.prevObject[1]

Changed July 17, 2014 11:03PM UTC by scottgonzalez comment:7

status: newopen

Changed August 20, 2014 08:18PM UTC by scottgonzalez comment:8

Changed August 21, 2014 01:07PM UTC by Scott González comment:9

Widget: Avoid memory leaks when unbinding events with ._off()

Ref #10056

Ref gh-1319

Changeset: b397294d42e783aacd4cc3a52bbe3aacc0f3f725

Changed August 21, 2014 01:07PM UTC by Scott González comment:10

resolution: → fixed
status: openclosed

Tabs: Avoid memory leak during refresh

Fixes #10056

Ref gh-1319

Changeset: 2e8e52dec1eaa06ed170a0ed9769c7b97129c955

Changed August 21, 2014 01:07PM UTC by Scott González comment:11

Accordion: Avoid memory leak during refresh

Ref #10056

Closes gh-1319

Changeset: 849c6fd5376e12c6093c557bd4836ef0b145f145

Changed August 21, 2014 01:08PM UTC by scottgonzalez comment:12

milestone: none1.11.2

Changed September 12, 2014 10:45AM UTC by rosenfeld comment:13

Thank you!