Memory leak

[SergiyCh]

Please answer the following questions for yourself before submitting an issue. YOU MAY DELETE THE PREREQUISITES SECTION.

• [ x] I am running the latest version
• [ x] I checked the Readme and found no answer
• [ x] I checked to make sure that this issue has not already been filed
• [ x] This is related to angular-google-charts and not to Google Charts directly (if it's a Google Charts issue, their forum may help)

Description

When a component with google-chart is destroyed the memory related to Google chart remains.

To reproduce

Steps to reproduce the behaviour:

1. Create a component with google-chart and provide some data so a chart is drawn.
2. Create/destroy the component several times, e.g. using *ngFor with updated source.
3. Open chrome devtools, create two memory snapshots before and after component destroys, and check objects.
4. See a lot of google objects. They are only increasing.

Expected behaviour

Angular google chart component must clear the chart on destroy using clearChart() method of google charts.

Workaround

Call chart.clearChart() on ngDestroy() for parent component. chart is obtained using "ready" callback.

See the same workaround for Vue: https://github.com/devstark-com/vue-google-charts/issues/25

[kussmaul]

As noted in #230, I don't see clearChart() in the angular-google-chart code - it seems to have been added in #70, but maybe refactored away since then. Here is my (kludgy) code to call the underlying clearCharts:

/** Chart component(s). */  @ViewChildren (GoogleChartComponent)  charts    ! : QueryList<GoogleChartComponent>;

  ngOnDestroy() {
    this.charts.forEach((c) => (c.chart as unknown as { clearChart : () => void })?.clearChart());
  }

[FERNman]

@kussmaul A while ago, I refactored the library to use a ChartWrapper (always) to instantiate the charts. It has a getChart method that returns the interface google.visualization.ChartBase, which doesn't contain the clearChart method. One needs to check for the existence of clearChart on the chart property since some charts might not support it, and only then call it.

However, I'm quite convinced that calling clearChart to resolve the memory leak is not a fix but rather a workaround. In the Google Charts community, they also said that browsers should be able to clean up the chart if no references are around anymore. I'd guess that maybe looking into the event handlers we're registering in our components could yield some results, even though one would probably have to dive a bit deeper into this library and Google Charts itself.

[kussmaul]

+1 to fixes rather than workarounds. :-)

I see components (chart-editor, chart-wrapper, control-wrapper, dashboard) where ngOnit() calls subscribe() on loadChartPackages() without a matching unsubscribe(). Would this help?

[FERNman]

It's probably worth looking into if you find the time :) Also, a reproduction of this issue (Stackblitz or Codespaces) would be great!

I'm not entirely sure about subjects within components anymore. If that can cause issues, it might be the wrapperReadySubject in the GoogleChartComponent that's causing the issue.

[kussmaul]

Here is some more data, which might be useful, although it is definitely not a minimal reproduction. :-)

In Chrome DevTools, I used the Performance tab to record heap, nodes, listeners, etc. as I use my app to create ~30 angular-google-chart tables, wait ~5 seconds, delete them, wait ~5 seconds, create them again, etc.

Here is the timeline without clearChart() in ngOnDestroy():

Here is the timeline with clearChart() in ngOnDestroy():

It looks like clearChart() does reduce the heap, but nodes and listeners continue to increase. This might be my code, or angular-google-charts, or GoogleCharts, I suppose. :-)

[kussmaul]

I also looked at listeners: https://github.com/search?q=repo%3AFERNman%2Fangular-google-charts%20Listener&type=code

I see components (chart-wrapper, control-wrapper, dashboard, google-chart) where ngOnInit() (via private helper methods) calls google.visualization.events.addListener() 2-3 times without a matching removeListener() or removeAllListeners(). However, removeAllListeners() is called from ngOnInit() before addListener().

In contrast, chart-editor-ref calls addOneTimeListener() (twice) with a callback that includes 'removeAllListeners()`.

@FERNman, do you recall why removeAllListeners() is called from ngOnInit()?

Should these 4 classes call removeAllListeners() in ngOnDestroy()?