akveo / akveo/nebular

Memory leak in `NbOverlayService`

Đang mở
#3,132 3 bình luận 0 reaction 0 người được giao Xem trên GitHub
Ngôn ngữ chính
TypeScript
Star
8.1k
Fork
1.5k
Chỉ số merge pull request
Không có pull request nào được merge trong 30 ngày

Mô tả

### 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;
}
```

Hướng dẫn đóng góp

Mở hướng dẫn đóng góp

Đánh giá

Issue này chưa được đánh giá.

Nhận issue mới trong hộp thư của bạn

Bản tóm tắt ngắn những issue GitHub phù hợp với người mới.