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 due to missing $destroy #514

Closed
aiankile opened this issue Feb 1, 2019 · 1 comment · May be fixed by tt9133github/snyktest#3
Closed

Memory leak due to missing $destroy #514

aiankile opened this issue Feb 1, 2019 · 1 comment · May be fixed by tt9133github/snyktest#3
Labels
Type: Bug Bug or Bug fixes

Comments

@aiankile
Copy link

aiankile commented Feb 1, 2019

Note: This issue practically duplicates #498 (see my comment there), but provides an explanation + solution

vue & vue-i18n version

vue 2.5.21 & vue-i18n 8.7.0

plus as specimen of a global-plugin e. g.: v-media-query 1.0.4

Reproducing issue (see steps below)

https://jsfiddle.net/DeepOne/qc7xpzuj/10/

Steps to reproduce

Setup (or use fiddle instead):

  • use any global plugin/mixin keeping track of Vue instances
  • create a Vue SFC with a local i18n definition

Warmup (pre-load possibly lazy chunks):

  • show and hide above component once

Actual test (Chrome):

  • Open Chrome DevTools -> Memory -> select fiddle.jshell.net -> take heap snapshot ("Snapshot 1")
  • show component (click button)
  • take second snapshot, select it, switch to "Comparison" with "Snapshot 1", filter by "Vue"
    --> take note: +1 Vue, +1 VueComponent, +1 VueI18n
  • hide component again (click button)
  • take third snapshot, select it, switch to "Comparison" with "Snapshot 2", filter by "Vue"
    --> this time: -1 VueComponent, -1 VueI18n
    --> that is, one Vue instance wasn't destroyed cleanly
  • select Snapshot 2 (still comparison), open "Vue" tree, hover over the one instance there
    --> it is i18n's _vm instance, still in the state isDestroyed=false
    --> select instance, identify global plugin/mixin (in jsfiddle: mapVmIdToVm) as the only retainer

In order to verify that this behavior is caused by the global plugin/mixin, comment out the Vue.mixin part of the fiddle and repeat above steps: Everything is cleanly removed this time.

What is Expected?

"Normal" VueComponent instances are destroyed automatically by Vue.
For manually created instances (not bound to a parent), however, $destroy has to be called manually, else the beforeDestroy hook is never called.

Reason: A number of Vue plugins (including quite reknown ones), including v-media-query and vue-i18n itself, keep track of all existing Vue instances / components via globally injected beforeCreate/created hooks, basically saving references to "this" of each created instance. Obviously, this functionality relies on correct destruction of all instances and thereby removal of already dead references in beforeDestroy hook. That is what vue-i18n itself does in its mixin.beforeDestroy!

What is actually happening?

In vue-i18n component-based variant, a new Vue instance is created as this._i18n._vm in each component with a local i18n option. While watchers are being correctly unregistered in beforeDestroy and the reference this._i18n is being cleared, the instance itself doesn't get destroyed via $destroy. As a consequence those global plugins are keeping a reference to it (as the instance still is in a valid, isDestroyed=false state).

Possible solution

A simple solution would be a call to this._i18n._vm.$destroy(); just before this._i18n = null;. This is, however, only possible if _i18n was created locally, else we're destroying root or parent i18n vm.

As of now, existence of _i18nWatcher can be used as flag for "it's our local _i18n copy", leading us to the following crude fix (in beforeDestroy):

const doDestroy = !!this._i18nWatcher
// ... watcher deletion goes here...
if (doDestroy) {
  this._i18n._vm.$destroy();
}
this._i18n = null;

^ This has definitely solved all of the memory leak issues in my app I could bring in connection with vue-i18n. Though there is surely a more elegant solution for the local mode detection (simply a separate flag)?

@kazupon kazupon added Type: Bug Bug or Bug fixes and removed in confirm labels Feb 13, 2019
@kazupon
Copy link
Owner

kazupon commented Feb 13, 2019

Thank you for your reporting!
Great reproting!
I've reproduced this issue.
As said you, this._i18n._vm is not destroyed.
I'm greatly thankful that you have found this issue. 🙇

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Type: Bug Bug or Bug fixes
Projects
None yet
Development

Successfully merging a pull request may close this issue.

2 participants