3

I am investigating an issue which appears to be coming from the fact that the "unsubscribe" method of the "Subscription" class does not appear to be disposing of the resources quickly enough which creates a memory leak.

Here is my scenario:

enter image description here

I have 2 components - LandingPageComponent (LPC) and WebPlayerComponent (WPC). The LPC is the first page that the user sees when they access the site. On that page, they have a link which takes them to the second page or WPC (passing an argument, e.g. "Operator Benchmarking") The user goes back to the previous page by clicking the browser's back button. That triggers the WPC's ngOnDestroy method which disposes of subscribtions (see code below).

WebPlayerComponent

export class WebPlayerComponent implements OnInit, OnDestroy {
    @Input() workbookId: string;
    @Input() workbookPage: string | number;

    private _workbooks: Workbook[];
    private _filters: Filter[];

    private _subscriptions: { [key: string]: Subscription } = {};
    
    ngOnInit() {
        this._subscriptions["combined"] = combineLatest(
            this.workbookService.workbooks$,
            this.filterService.filters$,
            this.libraryService.userFolderInitialised$
        ).subscribe(([workbooks, filters, userFolderInitialised]) => {
            this._workbooks = workbooks;
            this._filters = filters;

            this._subscriptions["webPlayerServiceSubscription"] = this.webPlayerService.openWorkbook(workbook.libraryPath, parameterString).subscribe(
                    (webPlayer) => {
                        console.log("_subscriptions before 'openWorkbook call'");
                        Object.keys(this._subscriptions).map((key) => {
                        console.log(key);
                        });

                        console.log("Openning document page: " + this.workbookPage);
                        webPlayer.openDocument("spotfire-container", this.workbookPage);
                    });
        });

    }
    
    ngOnDestroy() {
        Object.keys(this._subscriptions).map((key) => {
            this._subscriptions[key].unsubscribe();
            console.log("---Unsubscribed " + key + "---");
        });
    }
}

The problem starts occuring if the user clicks the browser's Back button and clicks on the "Link" quickly enough (in my example that's anything less that 5seconds). The subscribtions that are supposed to be disposed are not and are still active. I am able to confirm that by looking at the console's output:

enter image description here

In the above image you can see that after clickon on the "Link" again from the LandingPageComponent we see 2 sets of calls (the green numbers (1) & (2)), we also see the "Opening document page:" 3 times

This is how the output should look like if I wait 5 seconds before I click the link again

enter image description here

I am not sure why my subscription resources are not being disposed of fast enough (at least that is what I think the problem here is).

Note: I am using RxJS 6.4.0 and Angular 8.1.3

halfer
  • 19,824
  • 17
  • 99
  • 186
Georgi Koemdzhiev
  • 11,421
  • 18
  • 62
  • 126
  • 1
    Can you for once try to add your subscriptions (just for testing purposes) to a subscription class. `subs = new Subscription()` and then add your subscribe to the `subs` by writing: `subs.add(observable$.subscribe(...))`. In the unsubscribe simply call `subs.unsubscribe()`. Please inform us if this fixed your issue. Then we know its because of the subscription object. – Jonathan Stellwag Oct 06 '20 at 12:09
  • 5
    I would suggest to try: 1. unsubscribing with `takeUntil` operator, 2. remove nested subscriptions - so use `switchMap` operator for this.webPlayerService.openWorkbook call – vitaliy kotov Oct 06 '20 at 12:10
  • 4
    Well now I see the issue. Thanks to @vitaliy kotovs comment. I actually didn't see that you have a subscription inside a subscription. The reason for your memory leak is that you overwrite the reference to your old subscription whenever your outer subscription emits. That means you only unsubscribe the last added subscription. The previous one will always stay in memory and work. – Jonathan Stellwag Oct 06 '20 at 12:16
  • Thank you both for your suggestions. @vitaliykotov I did try `takeUntil` in the past without luck. As for using the `switchMap` operator, I am not sure how to apply it in my use case. Specifically, not sure what should go in the `switchMap(` "this._subscriptions["webPlayerServiceSubscription"] = this.webPlayerService.openWorkbook(workbook.libraryPath, parameterString).pipe(switchMap()).subscribe(..." @JonathanStellwag does it mean that your suggestion of using a single `subs` Subscription won't work in my case? – Georgi Koemdzhiev Oct 06 '20 at 12:59

1 Answers1

3

You can use switchMap like:

ngOnInit() {
    this._subscriptions["combined"] = combineLatest(
            this.workbookService.workbooks$,
            this.filterService.filters$,
            this.libraryService.userFolderInitialised$
    )
    .pipe(
        switchMap(([workbooks, filters, userFolderInitialised]) => {
            this._workbooks = workbooks;
            this._filters = filters;

            return this.webPlayerService.openWorkbook(workbook.libraryPath, parameterString)
        })
    )
    .subscribe((webPlayer) => {
        webPlayer.openDocument("spotfire-container", this.workbookPage);
     });
    }

So you change your observable from combineLatest to openWorkbook

vitaliy kotov
  • 570
  • 3
  • 5
  • Thank you, Vitaliy. Much appreciated, that worked like a charm. I am still new to RxJS and your input (together with Jonathan Stellwag's input) learned a lesson here - Don't nest subscriptions. Thank you! – Georgi Koemdzhiev Oct 06 '20 at 14:15