akveo / akveo/nebular

Memory leak in `NbOverlayService`

Aperta
#3,132 3 commenti 0 reazioni 0 assegnatari Vedi su GitHub
Lingua principale
TypeScript
Stelle
8.1k
Fork
1.5k
Metriche di merge delle PR
Nessuna PR unita negli ultimi 30g

Descrizione

### Issue type

**I'm submitting a ...** (check one with "x")

* [x] bug report
* [ ] feature request

### Issue description

**Current behavior:**
I've noticed that I can run into a memory leak when using Nebular components that use overlays (tooltips, selects, etc.)
From what I can tell, [NbOverlayService](https://github.com/akveo/nebular/blob/master/src/framework/theme/components/cdk/overlay/overlay-service.ts#L44) subscribes to a singleton service every time an overlay is created so that overlay can be notified if RTL/LTR direction changes.

However, this global service is long lived and the overlay is not. This results in the `OverlayRef` being retained in memory even after it is otherwise disposed, as the subscription's closure remains alive and keeps a reference to the `OverlayRef`.

Aside: In versions of Angular before 14, the memory leak triggered by this could be quite large, as the OverlayRef contains a reference to the host element, which has a reference to an `__ngContext__` that would maintain a reference to the host component (which often has a lot of data on it). In Angular 14+ this was partially mitigated [by this Angular PR](https://github.com/angular/angular/pull/45051) which no longer kept a direct reference to the component in the `ngContext`.

**Expected behavior:**
That when an overlay is disposed it can be garbage collected.

**Steps to reproduce:**
- add a tooltip to a page
- take a heap snapshot in Chrome dev tools
- hover over the tooltip several times
- take another heap snapshot and compare and you'll find multiple NbAdjustableConnectedPositionStrategy

**Proposed Solution:**
From looking at the code, I suspect that NbOverlayService was setup as a shim over the Angular Material CDK Overlay creation so you could setup this subscription to the direction service.

Normally we could put this in the NbOverlayRef (since in its disposal code it removes subscriptions like this already), but since NbOverlayRef is just an alias for CDK's OverlayRef, this isn't an option.

One work around would be for Nebular's NbOverlayService code to subscribe to one of NbOverlayRef's public observables that it completes upon destruction. This could be used as an indicator to also cleanup the LayoutDirection subscription.

Something like:

```typescript
create(config?: NbOverlayConfig): NbOverlayRef {
const overlayRef = this.overlay.create(config);

const sub = this.layoutDirection.onDirectionChange().subscribe(dir => overlayRef.setDirection(dir));
overlayRef.attachments.subscribe({complete: () => sub.unsubscribe()});

return overlayRef;
}
```

Guida per i contributori

Apri la guida per i contributori

Valutazione

Questa issue non è ancora stata valutata.

Ricevi le nuove issue nella tua casella

Un breve riepilogo di issue GitHub adatte ai principianti.