Angular 8 memory leak, adds more and more subscriptions

Viewed 1406

The problem:

My app is working fine on the first component I load, and when I click to switch to another component, it creates another subscription on My DAL.

Code

servers.component.html <- the default component

<div class="row">
  <div class="col-md-12">
    <app-server-list></app-server-list>
  </div>
  <router-outlet></router-outlet>
</div>

server-backup.component.html

<div class="row">
  <div class="col-md-12">
    <app-server-list ></app-server-list>
  </div>
</div>

In server-list.component.ts I want to refresh the data from the DB every 5 seconds, so I do this:

this.dataStorageService.getServerPerformanceInfo()
this.interval = setInterval(() =>
{
  this.dataStorageService.getServerPerformanceInfo()
} , 5000)

When I switch between servers and servers-backup I unsubscribe:

ngOnDestroy()
{

  this.subscriptions.forEach(subscription => {
    subscription.unsubscribe();  
  });
 clearInterval(this.interval);

}

And finally on my data-storage-service.ts

getServerPerformanceInfo()
{
  this.serverPerfSub = this.http.get<ServerPerformance[]>(DEV + '/getServerPerformance').subscribe
  (
    (data) => {
      this.serversService.setPerformanceInfo(data);
    }
  )
}

As I switch between my components, the server-list component gets destroyed, created, but the dataStorageService calls get more and more frequent until my API method starts failing.

Surely this has to do with the subscribe on the dataStorageService, but how do I clean that?

1 Answers

Javascript

You need to clear your setInterval.

interval: number;

ngOnInit() {
  this.dataStorageService.getServerPerformanceInfo()
  this.interval = setInterval(() => {
    this.dataStorageService.getServerPerformanceInfo();
  } , 5000);
}

ngOnDestroy() {
  clearInterval(this.interval);
}

RxJS

This can be done with javascript, but I would recommend using the RxJS interval since you are already dealing with observables.

And since you want to immediately invoke it, you would be better off using the RxJS timer.

// import { timer, Subject } from 'rxjs';
// import { takeUntil, catchError } from 'rxjs/operators';

private destroyed$ = new Subject();

ngOnInit() {
  // start the timer and initiate the observable immediately.
  // then emit a value every 5 seconds
  timer(0, 5000).pipe(
    // unsubscribe on ngOnDestroy
    takeUntil(this.destroyed$)
  ).subscribe(() => {
    // TODO: catch errors in data storage service
    this.dataStorageService.getServerPerformanceInfo();
  });
}

ngOnDestroy() {
  this.destroyed$.next();
  this.destroyed$.complete();
}

Moving the subscription out of your service

I would recommend moving your subscription out of your service though. You can still perform the action in a tap operator. You would use a switchMap in your component to chain the service call to your timer.

data-storage-service.ts

getServerPerformanceInfo(): Observable<any> {
  const url = DEV + '/getServerPerformance';
  return this.http.get<ServerPerformance[]>(url).pipe(
    tap((data) => this.serversService.setPerformanceInfo(data))    
  );
}

component.ts

ngOnInit() {
  timer(0, 5000).pipe(
    takeUntil(this.destroyed$),
    switchMap(() => this.dataStorageService.getServerPerformanceInfo()),
    catchError(error => of('An error has occured'))
  ).subscribe(() => {
    // TODO: Something?
  });
}

If you want to catch errors, you can catch them in the pipe using catchError. I would recommend catching them as close to the source as possible - preferably soon after the http request.

Related