search

framework7io / framework7

Full featured HTML framework for building iOS & Android apps
http://framework7.io
MIT License
18.08k stars 3.23k forks source link

Memory leak in Framework7 Vue navigation #2848

Closed curlycabbage closed 5 years ago

curlycabbage commented 5 years ago

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 commented 5 years ago

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 commented 5 years ago

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 commented 5 years ago

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 commented 5 years ago

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 commented 5 years ago

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 commented 5 years ago

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

curlycabbage commented 5 years ago

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: https://github.com/vuejs/vue/issues/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 commented 5 years ago

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 commented 5 years ago

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 commented 5 years ago

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 commented 5 years ago

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 commented 5 years ago

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

stale[bot] commented 5 years ago

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[bot] commented 5 years ago

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

ringsatu commented 5 years ago

Hi @nolimits4web any update for this issue? I have similar issue now, here is the repro:

OS: Windows 10 (64) Browser: Version 75.0.3770.142 (Official Build) (64-bit) F7: Latest

ringsatu commented 5 years ago

I managed to solve my issue, in my case it has nothing to do with F7, the issue related to Vuex