[Select pattern] prevent memory leak

[aitboudad]

This is a...

• feature request
• bug report
• usage question

What toolchain are you using for transpilation/bundling?

• Custom @ngTools/webpack

Actual Behaviour:

When using select pattern I noticed that all created instances doesn't released after unsubscribing which can raise memory leak

[SethDavenport]

Are you talking about the selection pooling used by @select (selection-map.ts)? That map is bounded by the number of distinct selections you make. So, while they are not explicitly released, the key space doesn't grow without bound unless you're somehow dynamically computing selectors with unique IDs, which would cause other problems...

[aitboudad]

here is an example that illustrate what I am talking about:

export class Counter {
  private sub;

  constructor(private ngRedux: NgRedux<IAppState>) {
    this.sub = this.ngRedux.select('counter').subscribe(v => console.log(v));
  }

  ngOnDestroy() {
    this.sub.unsubscribe();
  }
}

Every time I subscribe, two new instance of DistinctUntilChangedSubscriber is created and it will never destroyed when I unsubscribe

[ilkkaparssinen]

In our app we see same kind of memory leak (and because of this, all Angular components which use @select are not released - huge memory leak). If we downgrade to 6.2.1 the leak disappears. 6.4.3 is the first version where the leak seems to exist. Our usage pattern is simple - usually we make the subscribe and sometimes use async pipe - in my initial tests both approaches leak.

Our app is quite large, and the leak might well be some other side-effect of something totally different.

Versions: angular-cli (1.1.3) and angular 4.2.4.

Example of our usage pattern:
`@select(['products', 'records']) products$;
public products = [];
private sub:any;
constructor() {
this.sub = this.products$.subscribe(data => {
this.products = data;
});
}

ngOnDestroy() {
this.sub.unsubscribe();
}`

[kgarlikowski]

In out app we've also noticed quite a leak related to rxjs/async pipe and were able to track it down to this problem: ReactiveX/rxjs#2675. Maybe it's the same one as yours.

[SethDavenport]

@aitboudad: I've reproduced your issue here: SethDavenport/example-app@7b5696d

This is with 6.5.3

Looking into it.

[SethDavenport]

@ilkkaparssinen I suspect what you're seeing may be different - can you update to 6.5.3 and let me know it if still happens?

[SethDavenport]

I've traced @aitboudad's issue down to the use of switchMap here: https://github.com/angular-redux/store/blob/master/src/components/root-store.ts#L39

If I do a straight up store.subscribe(() => this._store$.next(store.getState())); on the base BehaviorSubject instead of swapping in a new Observable the leak goes away. Unfortunately it reintroduces issue #149, in which calling dispatch inside a subscribe callback results in ordering issues with the firing of selections.

Looking into what we can do to solve both issues simultaneously.

CC: @e-schultz

[SethDavenport]

OK ... wound back to before the fix for #372 and the leak isn't present. So switchMap by itself is not the issue. The way we're making an observable out of the redux store also changed at that point.

The investigation continues.

[SethDavenport]

@aitboudad I believe I have a fix for your leak. Can you try out 6.5.4-alpha.0 and let me know?

@ilkkaparssinen still looking into yours.

[SethDavenport]

@ilkkaparssinen I can reproduce components with @select not getting destroyed using 6.5.3 by running the example-app and switching between the 'lions' and 'elephants' routes repeatedly. If you then grab some memory snapshots in chrome you'll see a monotonically increasing number of instances of AnimalComponent.

The good news is that this is also fixed in 6.5.4-alpha.0. It must have been a symptom of the same issue.

I'll push out 6.5.4 shortly with the fix.

[SethDavenport]

6.5.4 released.

[ilkkaparssinen]

6.5.4 Fixes the issue for us. Thanks for everybody & sorry for the late response (I'm currently on vacation...).

[SethDavenport]

great, glad to hear it

[nasreddineskandrani]

@SethDavenport you can try to make use of drool to do automated leak tests on your example app. Which allow you to prevent leaks in release v.
If i found the time i ll give it a try my self in your example app and PR if concluent.

@kgarlikowski @ilkkaparssinen @aitboudad This is also apply for your product since you can avoid to upgrade a lib if it introduce leaks.

[SethDavenport]

Yeah it would be good to automate leak checking. Ideally as a prepublish step in angular-redux/store itself, e.g. after linting and unit tests.

[SethDavenport]

drool seems to use selenium though; angular-redux/store's test chain just runs in nodejs since there's no direct dependency on the DOM. I'd prefer to keep it that way since it's a lot simpler to maintain the tooling that way. I was toying with the idea of using nodejs heap dump tools to bake my own simple check but haven't gotten to it yet.

[SethDavenport]

e.g. https://github.com/lloyd/node-memwatch