Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Memory leak in Framework7 Vue navigation #2848

Closed
curlycabbage opened this issue Nov 23, 2018 · 14 comments

Comments

Projects
None yet
3 participants
@curlycabbage
Copy link

commented Nov 23, 2018

  • Framework7 version: 3.5.2
  • Vue.js version: 2.5.17
  • Platform and Target: Windows 10 Chrome 70, Android 6 Chrome 70

Describe the bug

VueComponents aren't being garbage collected after you navigate off a page.

To Reproduce

  1. Download, update, install and run the official F7 Vue Webpack sample app: https://github.com/framework7io/framework7-template-vue-webpack.
  2. Visit page in Chrome. Note memory usage in DevTools | Memory. Take a heap snapshot.
  3. In the sample app, click on Form, then Back.
  4. In DevTools | Memory, force garbage collection then take another snapshot. Note that memory usage has increased by several MB.
  5. Click Form > Back > Form > Back a few more time, force garbage collection and then take another heap snapshot. Note that memory usage has increased yet more. Note the increasing number of VueComponent instances in each snapshot.

Expected behavior

While navigating back and forth between two pages in an F7 Vue app, I expect memory usage to stay within constant bounds. I do not expect the upper bound to continually group.

Actual Behavior

Memory usage grows continuously while navigating between pages in an F7 Vue app. It appears that VueComponents are not being GC'd after destroyed().

Screenshots

image
image
image

Additional context

I've reproduced this using:

It happens with both versions 3.4 (sample config) and 3.5.2 (current). It also happens with the real F7 Vue app I am working on. Navigation eats up memory and makes the UI increasingly sluggish. Eventually, the browser throws an out of memory exception.

@nolimits4web

This comment has been minimized.

Copy link
Member

commented Nov 26, 2018

Can you track where does it come from? Because last time we faced it, issue was on Vue/Webpack side, all components are being destroyed correctly in F7-Vue from F7 side

@curlycabbage

This comment has been minimized.

Copy link
Author

commented Nov 27, 2018

I have reproduced the memory leak in sample apps "F7 Vue Simple", F7 Vue Webpack", and "F7 React". This suggests to me that the problem is between the router and components, whether Vue or React, and that the router holds a reference to component objects even after the component has been destroyed.

Even a destroyed component cannot be garbage collected until all references to it have been removed from other reachable objects in the JS memory graph. The router is always reachable, so if it has a reference to a destroyed component, the component will never be GC'd. I'm guessing there might be an event emitter or an array somewhere in the router that still has a reference to the destroyed component object after you navigate off the page.

I don't know the router code well enough to know where to look, though. Any suggestions?

@nolimits4web

This comment has been minimized.

Copy link
Member

commented Nov 28, 2018

Maybe you can help? As i can't understand where it comes from, double checked all the stuff.

Phenome (Vue/React) Router is here https://github.com/framework7io/framework7/blob/master/src/phenome/utils/components-router.js. You may check page pageComponentLoader and removePage methods.

Pages itself handled in View component https://github.com/framework7io/framework7/blob/master/src/phenome/components/view.jsx

Router and View communicate with each other with events emitter https://github.com/framework7io/framework7/blob/master/src/phenome/utils/events.js

These are all files that can contain reference to components

@curlycabbage

This comment has been minimized.

Copy link
Author

commented Nov 30, 2018

I have made very limited progress so far, despite substantial effort.

I can consistently fix the sample app About page component memory leak by not rendering the on.click handler of the f7-link component. Why this handler keeps the components from getting garbage collected I don't know. Other components on the About page also render click handlers (like navleft), but for whatever reason these--on their own--don't cause the page and sub-components to stay in memory.

In the heap dump I see a lot of native_binds and feedback_cells in the retainers. This and the fact that the memory leaks appear to be all or nothing suggest to me that it might be one of the more exotic closure scope leaks, as described here:

https://blog.swmansion.com/hunting-js-memory-leaks-in-react-native-apps-bd73807d0fde.
https://blog.meteor.com/an-interesting-kind-of-javascript-memory-leak-8b47d2e7f156

But if that is the case, I haven't figured out how to identify the source from the heap dump or from trial & error.

Next I will try to reproduce the bug in plain VueJs, and use the results of that to further bisect the problem. This is not my wheelhouse, so it's be quite a learning curve to even get this far.

Unfortunately, if we can't solve this problem, we'll have to find something other than F7 for our app. We've already replaced most of our nested components with plain F7 markup. This has reduced the magnitude of the memory leak and improved overall performance (less CPU usage rendering pages, fewer objects on the JS heap). But there's still a steady leak, and we need our app to be stable during continuous heavy use, so any level of memory leak is going to be a problem for us. It's too bad, because otherwise the framework has been great.

Interesting note: Chrome, Firefox and Safari all exhibit this memory leak. Edge does not. This is another reason I think it might be an exotic closure bug, since how closures capture over and share variables in the same lexical scope is an implementation detail and could vary by JS engine.

nolimits4web added a commit that referenced this issue Nov 30, 2018

@nolimits4web

This comment has been minimized.

Copy link
Member

commented Nov 30, 2018

Thanks! Spent a while with this today. Don’t understand why is this happening but looks like for some reason Vue just doesn’t remove its “native” event listeners, so I reworked all click handlers to manual el.add/removeEventListeners. Situation became a much better, at least on many Ks pages amount of Vue components is not increasing. But looks like issue is still with inputs’ events. Will keep investigating

@nolimits4web

This comment has been minimized.

Copy link
Member

commented Nov 30, 2018

And weird thing is that similar events added in router pages are removed correctly. So problem is not in router

@curlycabbage

This comment has been minimized.

Copy link
Author

commented Dec 3, 2018

looks like for some reason Vue just doesn’t remove its “native” event listeners

This reminds me of a VueJS issue regarding click bindings, which appears consistent with your work-around to explicitly remove native listeners: vuejs/vue#7086

Evan You closed the issue with this comment:

This is intended behavior. When a vm is destroyed, its associated piece of DOM is considered discarded. When managed by Vue itself, the DOM will be detached and garbage collected, and all modern browsers correctly discards event listeners in the process as well.

It is usually not recommended to call $destroy() imperatively, and once you call that, the DOM that's left behind should not be reused. Calling $destroy(true) removes the DOM at the same time.

I don't really understand his explanation, except that he expects to native listeners to be cleaned up automatically as long as other things elsewhere are cleaned in line with VueJS expectations.

@mmis1000

This comment has been minimized.

Copy link

commented Dec 3, 2018

Will fixed slot ...etc break vue's assumption?
The element is no longer existed at what vue expected it to be, because it is moved away.
Maybe the LinusBorg/portal-vue project can give us a bit idea where did we done wrong

@curlycabbage

This comment has been minimized.

Copy link
Author

commented Dec 10, 2018

Hello Vladimir, do you have any updates on this issue?

I tested release 3.6.1 in the F7 Vue sample app. Navigating to and from the About page no longer causes VueComponent leaks. The About page instantiates 11 new VueComponents, and these are now cleaned up each time you navigate off. Great!

However, navigating to and from the Form page does continue to causes memory leaks. The Form page instantiates 96 new VueComponents, and these are not cleaned up after you navigate off. In the memory dump, I see f7Page.methods.onPageAfterOut, F7ListItemContent.render and F7Range.render as some of the most frequent retainers.

@nolimits4web

This comment has been minimized.

Copy link
Member

commented Dec 11, 2018

I keep researching. But issues on that pages is because we still have same Vue’s native input events (onChange/Focus/Blur, etc.) I tried to remove and situation becomes better, but I don’t feel it is right move. If we move all listeners to custom addEventListener then it will break React, because it requires those events to be specified. Need to find the origin of the issue

@mmis1000 will check the slots thing

@curlycabbage

This comment has been minimized.

Copy link
Author

commented Dec 17, 2018

Vladimir, just to clarify where things stand now:

a) you are confident that you will fix the memory leak, you just need to find the best approach.

-- or --

b) you may or may not be able to fix the memory leak, depending on whether you find the origin of the issue.

I spent a few days trying to find the origin of the issue, but with no luck. Since then I have put the issue aside, hoping that you might be able to solve it. Now we have some decisions to make regarding our own software. We can't release it with a memory leak, so this issue here will become a blocker sooner or later. Maybe we could crowd-source the answer? I have spare karma on SO to place a bounty, if there's a suitable question.

Cheers!

@nolimits4web

This comment has been minimized.

Copy link
Member

commented Dec 21, 2018

I would say (b) - i keep looking but can't 100% guarantee that will be able to find its origin

@stale

This comment has been minimized.

Copy link

commented Mar 21, 2019

This issue has been automatically marked as stale because it has not had recent activity. It will be closed if no further activity occurs. Thank you for your contributions.

@stale stale bot added the wontfix label Mar 21, 2019

@stale

This comment has been minimized.

Copy link

commented Apr 4, 2019

This issue has been automatically closed due to inactivity. If this issue is still actual, please, create the new one.

@stale stale bot closed this Apr 4, 2019

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
You can’t perform that action at this time.