Skip to content

Commit

Permalink
fix(cdk/overlay): avoid leaking memory through afterNextRender
Browse files Browse the repository at this point in the history
The `OverlayRef` was triggering an `afterEachRender` and passing in an `EnvironmentInjector`. Under the hood this uses a `DestroyRef` that is never destroyed, because the `EnvironmentInjector` is almost never destroyed.

These changes add an explicit `destroy` call to avoid the issue.

Fixes #29696.
  • Loading branch information
crisbeto committed Sep 9, 2024
1 parent f4cb9f1 commit 01a1c5d
Showing 1 changed file with 12 additions and 3 deletions.
15 changes: 12 additions & 3 deletions src/cdk/overlay/overlay-ref.ts
Original file line number Diff line number Diff line change
Expand Up @@ -9,14 +9,14 @@
import {Direction, Directionality} from '@angular/cdk/bidi';
import {ComponentPortal, Portal, PortalOutlet, TemplatePortal} from '@angular/cdk/portal';
import {
AfterRenderRef,
ComponentRef,
EmbeddedViewRef,
EnvironmentInjector,
NgZone,
afterNextRender,
afterRender,
untracked,
AfterRenderRef,
} from '@angular/core';
import {Location} from '@angular/common';
import {Observable, Subject, merge, SubscriptionLike, Subscription} from 'rxjs';
Expand All @@ -39,7 +39,7 @@ export type ImmutableObject<T> = {
*/
export class OverlayRef implements PortalOutlet {
private _backdropElement: HTMLElement | null = null;
private _backdropTimeout: number | undefined;
private _backdropTimeout: ReturnType<typeof setTimeout> | undefined;
private readonly _backdropClick = new Subject<MouseEvent>();
private readonly _attachments = new Subject<void>();
private readonly _detachments = new Subject<void>();
Expand Down Expand Up @@ -67,6 +67,9 @@ export class OverlayRef implements PortalOutlet {

private _afterRenderRef: AfterRenderRef;

/** Reference to the currently-running `afterNextRender` call. */
private _afterNextRenderRef: AfterRenderRef | undefined;

constructor(
private _portalOutlet: PortalOutlet,
private _host: HTMLElement,
Expand Down Expand Up @@ -151,9 +154,14 @@ export class OverlayRef implements PortalOutlet {
this._scrollStrategy.enable();
}

// We need to clean this up ourselves, because we're passing in an
// `EnvironmentInjector` below which won't ever be destroyed.
// Otherwise it causes some callbacks to be retained (see #29696).
this._afterNextRenderRef?.destroy();

// Update the position once the overlay is fully rendered before attempting to position it,
// as the position may depend on the size of the rendered content.
afterNextRender(
this._afterNextRenderRef = afterNextRender(
() => {
// The overlay could've been detached before the callback executed.
if (this.hasAttached()) {
Expand Down Expand Up @@ -267,6 +275,7 @@ export class OverlayRef implements PortalOutlet {
this._outsidePointerEvents.complete();
this._outsideClickDispatcher.remove(this);
this._host?.remove();
this._afterNextRenderRef?.destroy();

this._previousHostParent = this._pane = this._host = null!;

Expand Down

0 comments on commit 01a1c5d

Please sign in to comment.