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

Possible memory leak? #1058

Closed
mmase opened this issue Dec 7, 2018 · 8 comments
Closed

Possible memory leak? #1058

mmase opened this issue Dec 7, 2018 · 8 comments
Labels

Comments

@mmase
Copy link

mmase commented Dec 7, 2018

Version

1.0.0-beta.26

Steps to reproduce

  • Run a large test suite
  • I cannot provide a minimal reproduction because any minimal reproduction would not be helpful here. I am looking for a discussion. to see if others see this as well as to possible solutions.

What is expected?

Vue components are garbage collected

What is actually happening?

80% of VueComponent instances are retained in memory; even after calling wrapper.destroy after each instance. Heap size continues to grow and grow over time. Garbage collection occurs (either automatically or by force), but only a minimal amount is removed from memory.

@meis
Copy link

meis commented Dec 8, 2018

Could this be related to #892? I'm experiencing both symptoms: as I re-run my tests with a mocha watcher they get slower (some event timeout) and the heap size keeps growing. Eventually, I get a JavaScript heap out of memory error and the process crashes.

I've also observed that manual GC don't help and a lot of VueComponent objects never get cleaned.

@mmase
Copy link
Author

mmase commented Dec 8, 2018

@meis FWIW, we have been having this issue since at least beta 10.

@eddyerburgh
Copy link
Member

Thanks for the report.

The main issue I found is cached constructors added to component options objects, which results in lots of VueComponent instances that can't be garbage collected. This is likely why VueComponent objects weren't removed even after calling $destroy. I've opened a PR to remove the component that's mounted cached constructor.

I'll also add a cleanUp function that can run in a beforeEach to call $destroy on all mounted instances.

Another issue I found is that some plugins keep references to instances. I'll investigate this further.

@meis
Copy link

meis commented Dec 10, 2018

Thanks a lot @eddyerburgh.

I've had the chance to quickly test the new version (1.0.0-beta.27) this afternoon. My numbers show an improvement of roughly 50% of the heap growth compared to the last version. This means that, on subsequent runs of the test suite, the heap only increases ~50KB instead of the ~100KB growth I was observing before.

At this point, it's not clear for me which module is responsible for the remaining leaks, but your change certainly helped!

@mmase
Copy link
Author

mmase commented Dec 11, 2018

Thanks for this progress! I have done some further testing and found some leaks within vue-router. There are event listeners that hold onto components forever. Trying to determine if there is a quick solution to this particular problem. Not sure if anyone else is also using vue-router.

@mmase
Copy link
Author

mmase commented Dec 11, 2018

Related: vuejs/vue-router#2341

@mmase
Copy link
Author

mmase commented Dec 11, 2018

I have seemed to find this vm.$emit closure as a possible culprit for holding onto vue components forever:

function logEvents (vm, emitted, emittedByOrder) {
  var emit = vm.$emit;
  vm.$emit = function (name) {
    var args = [], len = arguments.length - 1;
    while ( len-- > 0 ) args[ len ] = arguments[ len + 1 ];

    (emitted[name] || (emitted[name] = [])).push(args);
    emittedByOrder.push({ name: name, args: args });
    var x = emit.call.apply(emit, [ vm, name ].concat( args ));
    console.count(x);
    return x;
  };
}

@eddyerburgh
Copy link
Member

There are two memory leaks I can identify:

  1. Vue Router as identified by @mmase

  2. The cached constructors in the _Ctor added to component options when Vue.extend is called with extend options.

  3. Will need to be solved in Vue Router

  4. Is difficult. We already remove cached constructors in mount. I'll look at other ways we could stop cached constructors causing memory leaks.

(@mmase logEvents adds properties to the instance, so if the instance is GCd correctly it won't cause a memory leak)

I'm going to close this issue in favor of #892 which is tracking the same issue.

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

No branches or pull requests

3 participants