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 happening while using registerModule/unregisterModule. #1507

Closed
skkart opened this issue Feb 13, 2019 · 6 comments · Fixed by #1508
Closed

Memory leak happening while using registerModule/unregisterModule. #1507

skkart opened this issue Feb 13, 2019 · 6 comments · Fixed by #1508

Comments

@skkart
Copy link

skkart commented Feb 13, 2019

Version

3.1.0

Reproduction link

https://jsfiddle.net/mrj8spu0/

Steps to reproduce

  1. Open the JSFiddle link mentioned as part of this issue.
  2. Check and record the heap memory in the browser on initial stage.
  3. Click on the "Register" button to dynamically register Module B 100 times
  4. Now we will see an increase in heap memory on account of registering module 100 times.
  5. Click on "Un-Register" button to dynamically un-register Module B 100 times
  6. Check the heap memory again, at this point will see an jump in memory usage on account of un-registering 100 module.

What is expected?

Heap memory generated on step 3 & 5 must be cleared after step 6. (Final stage)

What is actually happening?

Heap memory of the browser is not getting cleared.


We are observing memory leak of Store._vm while using registerModule/unregisterModule extensively.
On heap profiling we found that the oldVm instance is not getting garbage collected.

Probable Solution:
De-reference the oldVm instance on the same function scope where its getting instantiated (i.e. resetStoreVM() )

The above can be achieved by adding following code inside resetStoreVM function.

oldVm.$destroy();
oldVm = null; // This is the missing code

Attached google chrome (V 72.0.3626.96) heap snapshot and file.
heapsnapshot

Heap-20190213T220212.heaptimeline.zip

@parth67
Copy link
Contributor

parth67 commented Feb 14, 2019

Closure function at https://github.com/vuejs/vuex/blob/dev/src/store.js#L259 is retaining environment which contains oldVm reference.

Check screenshot GC root is able to reach oldVm object
screenshot from 2019-02-14 10-50-29

@tehnorm
Copy link

tehnorm commented Mar 1, 2019

Possibly related. We are seeing a memory leak in a mutation:

[types.RECORD_MEASUREMENT](st, measurement) {
  Vue.set(st.lastMeasurementByDevice, measurement.device_id, measurement);
},

This seems to help the leak.

[types.RECORD_MEASUREMENT](st, measurement) {
  Vue.set(st.lastMeasurementByDevice, measurement.device_id, Object.freeze(measurement));
},

Still working on a succinct reproducible case.

@parth67
Copy link
Contributor

parth67 commented Mar 2, 2019

@tehnorm can you share sample state declaration and sample measurement object?

@wolfgangwalther
Copy link

wolfgangwalther commented Apr 17, 2019

The fix in pr#1508 doesn't change anything for me, but the original suggestion by @skkart (adding oldVm = null) does solve the memory leak for me.

#1508 only fixed one of the anonymous functions preserving the previous context in resetStoreVM. See PR #1552 for a fix.

@afwn90cj93201nixr2e1re
Copy link

@ktsn what about the last one comment?

@wolfgangwalther
Copy link

finally resolved by #1546

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 a pull request may close this issue.

5 participants