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

fix(ssr): Memory leak in poll method (fix #2606). #2867

Closed
wants to merge 1 commit into from

Conversation

ronald-d-rogers
Copy link
Contributor

@ronald-d-rogers ronald-d-rogers commented Aug 2, 2019

Fix #2606.

This fix depends on the following being merged first to work: vuejs/vue#9479

Fixes memory leak in poll method when a Vue app is created per request, which is typical in SSR.

In particular, the leak happens when:

  1. The router-view is conditioned to appear upon a variable that never becomes true.
<div id="app">
  <div v-if="condition">
    <router-view />
  </div>
</div>
  1. A beforeRouteEnter guard is defined in the view in the following way:
export default {
  beforeRouteEnter(to, from, next) {
    // Poll happens only if you pass in a function to next(...)
    next(vm => {})
  }
}

@ronald-d-rogers
Copy link
Contributor Author

ronald-d-rogers commented Aug 2, 2019

Note I haven't had time to test this again after all of the changes that have been made to both projects. Will do this tomorrow if possible.

@ronald-d-rogers ronald-d-rogers changed the title fix(ssr): Memory leak in poll method (fix ##2606). fix(ssr): Memory leak in poll method (fix #2606). Aug 2, 2019
@ronald-d-rogers
Copy link
Contributor Author

There still appears to be a leak. I've gone ahead and updated https://github.com/ronald-d-rogers/vue-router-ssr-memory-leak with the latest versions of vue and vue-router, and included instructions on testing the fix and an explanation how it works. Hope this helps.

@posva
Copy link
Member

posva commented Aug 3, 2019

Thanks for the detailed report @ronald-d-rogers. I will try to give it a look next week

@ronald-d-rogers
Copy link
Contributor Author

@posva Note that I don't think this a great fix. I think the solution is probably to find a way to get rid of the poll, which was just added to fix a bug where the router-view hasn't yet appeared because the previous route is transitioning out, because we don't have a vm to pass to the beforeRouteEnter hook's next callback if the hook was defined with one (next(vm => { ... })), at the time the callbacks are called.

One way to get around it may be by putting the callback on the matching RouteRecord, adding some method like registerComponentInstance(key, vm) and registerBeforeRouterEnterCb(key, cb) to it, and only firing when both have been registered if (this.hasCb(key) && this.hasInstance(key)) { /* fire */ }.

But I can't tell if that would work here or not. Maybe I will give it a go today or tomorrow.

@ryouaki
Copy link

ryouaki commented Aug 5, 2019

This issues will block on poll function. So you should let poll exit at right time。

@posva
Copy link
Member

posva commented Sep 23, 2019

Let's close this in favor of #2875

@posva posva closed this Sep 23, 2019
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

SSR memory leak in poll method
3 participants