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

[bug] Memory leak with this.$router.apps array and secondary apps on page being destroyed #2639

Labels

Comments

@tmorehouse
Copy link

tmorehouse commented Mar 5, 2019

Version

3.0.2 (and earlier)

Reproduction link

https://bootstrap-vue.js.org/play/

See next steps to easily reproduce the issue.

Steps to reproduce

Just go to https://bootstrap-vue.js.org/play/

And enter the following code in the HTML Template area:

<div>
  <p>This App UID: {{ _uid }}</p>
  <p>$route.apps length: {{ $router.apps.length }}</p>
  <pre>{{
    $router.apps.map(
      a => {
      	return { uid: a._uid, destroyed: a._isDestroyed }
      }
    ).reverse()
  }}</pre>
</div>

and just enter { } into the JavaScript area (or leave it as is)

Hit space / delete the space every second or so in either the HTML entry area, or the javascript entry area (there is a 0.75 second debouncer on the app creation/destroy on keypress).

Each time, the rendered app will be destroyed and recreated.

The app shows a mapped version of $router.apps array (in the results section)

You will see the array grow and grow and grow, with all these destroyed instances sticking around.

It looks like vue-router isn't cleaning out $router.apps array during the destroy hook of the app. (`hook:destroyed').

What is expected?

That the apps that have been $destroyed would be removed from the $router.apps array

What is actually happening?

The destroyed app references remain in the $router.apps array, causing memory usage to grow/leak.


I've noticed a memory leak with this.$router.apps array. It might be related (but not sure) to other memory leaks reported.

On a docs site we have, we have a playground that allows you to create and test mini apps in real time.

As the user edits the test app, it destroys (app.$destroy()), and then re compiles and mounts the new version.

Each time the playground app is created, it gets appended to the this.$router.apps array, while the destroyed app still remains in the array (albeit it has a destroyed instance, but there is still quite a bit of stuff in the reference).

The playground is running under Nuxt.js, and each test in the playground is created as a new app (with a reference to the main $router instance).

We are adding in a temporary fix that will scan the $router.apps array, and slice out the destroyed app reference.

@tmorehouse
Copy link
Author

tmorehouse commented Mar 5, 2019

In this section:

vue-router/src/index.js

Lines 83 to 96 in bd41be0

init (app: any /* Vue component instance */) {
process.env.NODE_ENV !== 'production' && assert(
install.installed,
`not installed. Make sure to call \`Vue.use(VueRouter)\` ` +
`before creating root instance.`
)
this.apps.push(app)
// main app already initialized.
if (this.app) {
return
}

At line 94 (inside the if (this.app) check), the following code could be added:

     app.$once('hook:destroyed', () => {
       // Clean out destroyed app from this.apps array
       const index = this.apps.indexOf(app)
       if (index > -1) this.apps.splice(index, 1)
     })

Which would remove the app from the $router.apps array once the app is destroyed.

tmorehouse added a commit to tmorehouse/vue-router that referenced this issue Mar 5, 2019
…ixes vuejs#2639)


Prevent destroyed apps from causing memory leak in `$router.apps` array.

Fixes vuejs#2639
@tmorehouse tmorehouse changed the title Memory leak with this.$router.apps array and mini apps on page being destroyed [bug] Memory leak with this.$router.apps array and secondary apps on page being destroyed Mar 5, 2019
@posva posva added the has PR label Mar 26, 2019
posva pushed a commit that referenced this issue Apr 11, 2019
…ixes #2639)


Prevent destroyed apps from causing memory leak in `$router.apps` array.

Fixes #2639
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment