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-leaks #34

Closed
aminpaks opened this issue May 27, 2017 · 4 comments
Closed

Memory-leaks #34

aminpaks opened this issue May 27, 2017 · 4 comments

Comments

@aminpaks
Copy link

aminpaks commented May 27, 2017

I changed two lines in your example and run it. After about 30,000 actions app accumulates up to 300MB memory and crashes.

These are steps to reproduce this issue:

  1. Update addTicket in animal component to, this code simulates dispatching many actions to observe the issue easier:
  addTicket = () => {
    for (let i = 0; i < 10000; i++) {
      setTimeout(() => this.subStore.dispatch({ type: 'ADD_TICKET' }), 1);
    }
  }
  1. Remove the createLogger and devTools enhancer from store/module:
    // Tell Redux about our reducers and epics. If the Redux DevTools
    // chrome extension is available in the browser, tell Redux about
    // it too.
    store.configureStore(
      rootReducer,
      {},
      [ ...rootEpics.createEpics() ]);
  1. Run the app yarn start
  2. Click on + to start the process: this will dispatch 10000 actions and update the UI.
  3. Observe how app crashes after a few times clicking on the plus

You may check the memory in Chrome devTools too.

@aminpaks aminpaks changed the title Memory-leak Memory-leaks May 29, 2017
@aminpaks
Copy link
Author

aminpaks commented May 29, 2017

I already fixed the issue in angular-redux/store project, where can I ask for PR?
angular-redux/store

@SethDavenport
Copy link
Member

Hey - looking into it now. Please understand that I don't get to everything immediately.

@SethDavenport
Copy link
Member

I've reproduced the issue and confirmed that your fix works. Thanks, nice catch!

I also took the liberty of creating a PR with from your fork, here: angular-redux/store#409. There are a couple of minor cleanup items after which I'd be happy to merge it.

@SethDavenport
Copy link
Member

I cleaned up the PR and merged it. The fix has been released in @angular-redux/store 6.4.1.

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

No branches or pull requests

2 participants