From f6c9172afef682ca3f9e294904e988868013beba Mon Sep 17 00:00:00 2001 From: Kristiyan Kostadinov Date: Tue, 12 Sep 2017 02:33:56 +0200 Subject: [PATCH] perf: memory leak when subscribing to zone events (#6918) Fixes a few memory leaks that were caused by subscribing to `NgZone.onMicrotaskEmpty` or `NgZone.onStable`. Because the two streams are `EventEmitters`, the subscription doesn't get GC-ed correctly. These changes switch to converting the emitters to observables before subscribing to them. Fixes #6905. --- src/cdk/a11y/focus-trap.ts | 2 +- src/lib/autocomplete/autocomplete-trigger.ts | 2 +- src/lib/datepicker/calendar.ts | 8 +++++--- src/lib/datepicker/datepicker.ts | 4 +++- src/lib/sidenav/drawer.ts | 7 +++++-- src/lib/snack-bar/snack-bar-container.ts | 10 +++------- src/lib/tooltip/tooltip.ts | 2 +- 7 files changed, 19 insertions(+), 16 deletions(-) diff --git a/src/cdk/a11y/focus-trap.ts b/src/cdk/a11y/focus-trap.ts index cb09557accb..d726b830956 100644 --- a/src/cdk/a11y/focus-trap.ts +++ b/src/cdk/a11y/focus-trap.ts @@ -265,7 +265,7 @@ export class FocusTrap { if (this._ngZone.isStable) { fn(); } else { - first.call(this._ngZone.onStable).subscribe(fn); + first.call(this._ngZone.onStable.asObservable()).subscribe(fn); } } } diff --git a/src/lib/autocomplete/autocomplete-trigger.ts b/src/lib/autocomplete/autocomplete-trigger.ts index bbc97e95833..7d3aca8621f 100644 --- a/src/lib/autocomplete/autocomplete-trigger.ts +++ b/src/lib/autocomplete/autocomplete-trigger.ts @@ -372,7 +372,7 @@ export class MdAutocompleteTrigger implements ControlValueAccessor, OnDestroy { * stream every time the option list changes. */ private _subscribeToClosingActions(): Subscription { - const firstStable = first.call(this._zone.onStable); + const firstStable = first.call(this._zone.onStable.asObservable()); const optionChanges = map.call(this.autocomplete.options.changes, () => this._positionStrategy.recalculateLastPosition()); diff --git a/src/lib/datepicker/calendar.ts b/src/lib/datepicker/calendar.ts index af1e203c783..a434e9c0b52 100644 --- a/src/lib/datepicker/calendar.ts +++ b/src/lib/datepicker/calendar.ts @@ -218,9 +218,11 @@ export class MdCalendar implements AfterContentInit, OnDestroy { /** Focuses the active cell after the microtask queue is empty. */ _focusActiveCell() { - this._ngZone.runOutsideAngular(() => first.call(this._ngZone.onStable).subscribe(() => { - this._elementRef.nativeElement.querySelector('.mat-calendar-body-active').focus(); - })); + this._ngZone.runOutsideAngular(() => { + first.call(this._ngZone.onStable.asObservable()).subscribe(() => { + this._elementRef.nativeElement.querySelector('.mat-calendar-body-active').focus(); + }); + }); } /** Whether the two dates represent the same view in the current view mode (month or year). */ diff --git a/src/lib/datepicker/datepicker.ts b/src/lib/datepicker/datepicker.ts index 6e87b996dee..594b5e7bd6b 100644 --- a/src/lib/datepicker/datepicker.ts +++ b/src/lib/datepicker/datepicker.ts @@ -323,7 +323,9 @@ export class MdDatepicker implements OnDestroy { componentRef.instance.datepicker = this; // Update the position once the calendar has rendered. - first.call(this._ngZone.onStable).subscribe(() => this._popupRef.updatePosition()); + first.call(this._ngZone.onStable.asObservable()).subscribe(() => { + this._popupRef.updatePosition(); + }); } this._popupRef.backdropClick().subscribe(() => this.close()); diff --git a/src/lib/sidenav/drawer.ts b/src/lib/sidenav/drawer.ts index 672727ddba6..909fe6a0a38 100644 --- a/src/lib/sidenav/drawer.ts +++ b/src/lib/sidenav/drawer.ts @@ -409,8 +409,11 @@ export class MdDrawerContainer implements AfterContentInit, OnDestroy { } // NOTE: We need to wait for the microtask queue to be empty before validating, // since both drawers may be swapping positions at the same time. - takeUntil.call(drawer.onPositionChanged, this._drawers.changes).subscribe(() => - first.call(this._ngZone.onMicrotaskEmpty).subscribe(() => this._validateDrawers())); + takeUntil.call(drawer.onPositionChanged, this._drawers.changes).subscribe(() => { + first.call(this._ngZone.onMicrotaskEmpty.asObservable()).subscribe(() => { + this._validateDrawers(); + }); + }); } /** Toggles the 'mat-drawer-opened' class on the main 'md-drawer-container' element. */ diff --git a/src/lib/snack-bar/snack-bar-container.ts b/src/lib/snack-bar/snack-bar-container.ts index 1c8c1f88400..0828b73cd12 100644 --- a/src/lib/snack-bar/snack-bar-container.ts +++ b/src/lib/snack-bar/snack-bar-container.ts @@ -186,13 +186,9 @@ export class MdSnackBarContainer extends BasePortalHost implements OnDestroy { * errors where we end up removing an element which is in the middle of an animation. */ private _completeExit() { - // Note: we shouldn't use `this` inside the zone callback, - // because it can cause a memory leak. - const onExit = this._onExit; - - first.call(this._ngZone.onMicrotaskEmpty).subscribe(() => { - onExit.next(); - onExit.complete(); + first.call(this._ngZone.onMicrotaskEmpty.asObservable()).subscribe(() => { + this._onExit.next(); + this._onExit.complete(); }); } } diff --git a/src/lib/tooltip/tooltip.ts b/src/lib/tooltip/tooltip.ts index c12ab895111..d5a7c14169a 100644 --- a/src/lib/tooltip/tooltip.ts +++ b/src/lib/tooltip/tooltip.ts @@ -392,7 +392,7 @@ export class MdTooltip implements OnDestroy { this._tooltipInstance.message = this.message; this._tooltipInstance._markForCheck(); - first.call(this._ngZone.onMicrotaskEmpty).subscribe(() => { + first.call(this._ngZone.onMicrotaskEmpty.asObservable()).subscribe(() => { if (this._tooltipInstance) { this._overlayRef!.updatePosition(); }