Skip to content

Commit

Permalink
perf: memory leak when subscribing to zone events (#6918)
Browse files Browse the repository at this point in the history
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.
  • Loading branch information
crisbeto authored and mmalerba committed Sep 12, 2017
1 parent a1ec81a commit f6c9172
Show file tree
Hide file tree
Showing 7 changed files with 19 additions and 16 deletions.
2 changes: 1 addition & 1 deletion src/cdk/a11y/focus-trap.ts
Original file line number Diff line number Diff line change
Expand Up @@ -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);
}
}
}
Expand Down
2 changes: 1 addition & 1 deletion src/lib/autocomplete/autocomplete-trigger.ts
Original file line number Diff line number Diff line change
Expand Up @@ -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());

Expand Down
8 changes: 5 additions & 3 deletions src/lib/datepicker/calendar.ts
Original file line number Diff line number Diff line change
Expand Up @@ -218,9 +218,11 @@ export class MdCalendar<D> 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). */
Expand Down
4 changes: 3 additions & 1 deletion src/lib/datepicker/datepicker.ts
Original file line number Diff line number Diff line change
Expand Up @@ -323,7 +323,9 @@ export class MdDatepicker<D> 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());
Expand Down
7 changes: 5 additions & 2 deletions src/lib/sidenav/drawer.ts
Original file line number Diff line number Diff line change
Expand Up @@ -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. */
Expand Down
10 changes: 3 additions & 7 deletions src/lib/snack-bar/snack-bar-container.ts
Original file line number Diff line number Diff line change
Expand Up @@ -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();
});
}
}
2 changes: 1 addition & 1 deletion src/lib/tooltip/tooltip.ts
Original file line number Diff line number Diff line change
Expand Up @@ -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();
}
Expand Down

0 comments on commit f6c9172

Please sign in to comment.