Skip to content

Commit 5e61e8d

Browse files
atscottmmalerba
authored andcommitted
fix(router): Fix memory leak through Navigation.abort and canDeactivate guards (angular#64141)
This commit updates the internal transition to handle context retention through the abort function. This retention chain included the previousNavigation and setting this to a noop function resolves the issue. fixes angular#63983 PR Close angular#64141
1 parent 015d582 commit 5e61e8d

File tree

2 files changed

+14
-7
lines changed

2 files changed

+14
-7
lines changed

packages/core/test/bundling/router/bundle.golden_symbols.json

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -930,6 +930,7 @@
930930
"nodeChildrenAsMap",
931931
"noop",
932932
"noop2",
933+
"noop3",
933934
"normalizeQueryParams",
934935
"notFoundValueOrThrow",
935936
"observable",

packages/router/src/navigation_transition.ts

Lines changed: 13 additions & 7 deletions
Original file line numberDiff line numberDiff line change
@@ -17,6 +17,7 @@ import {
1717
signal,
1818
Type,
1919
untracked,
20+
ɵWritable as Writable,
2021
} from '@angular/core';
2122
import {BehaviorSubject, combineLatest, EMPTY, from, Observable, of, Subject} from 'rxjs';
2223
import {
@@ -306,6 +307,8 @@ export interface Navigation {
306307
readonly abort: () => void;
307308
}
308309

310+
const noop = () => {};
311+
309312
export interface NavigationTransition {
310313
id: number;
311314
currentUrlTree: UrlTree;
@@ -325,7 +328,6 @@ export interface NavigationTransition {
325328
targetRouterState: RouterState | null;
326329
guards: Checks;
327330
guardsResult: GuardResult | null;
328-
abortController: AbortController;
329331
}
330332

331333
/**
@@ -437,7 +439,6 @@ export class NavigationTransitions {
437439
targetRouterState: null,
438440
guards: {canActivateChecks: [], canDeactivateChecks: []},
439441
guardsResult: null,
440-
abortController: new AbortController(),
441442
id,
442443
});
443444
});
@@ -451,6 +452,7 @@ export class NavigationTransitions {
451452
// Using switchMap so we cancel executing navigations when a new one comes in
452453
switchMap((overallTransitionState) => {
453454
let completedOrAborted = false;
455+
const abortController = new AbortController();
454456
return of(overallTransitionState).pipe(
455457
switchMap((t) => {
456458
// It is possible that `switchMap` fails to cancel previous navigations if a new one happens synchronously while the operator
@@ -488,7 +490,7 @@ export class NavigationTransitions {
488490
...lastSuccessfulNavigation,
489491
previousNavigation: null,
490492
},
491-
abort: () => t.abortController.abort(),
493+
abort: () => abortController.abort(),
492494
});
493495
const urlTransition =
494496
!router.navigated || this.isUpdatingInternalState() || this.isUpdatedBrowserUrl();
@@ -540,7 +542,7 @@ export class NavigationTransitions {
540542
router.config,
541543
this.urlSerializer,
542544
this.paramsInheritanceStrategy,
543-
overallTransitionState.abortController.signal,
545+
abortController.signal,
544546
),
545547

546548
// Update URL if in `eager` update mode
@@ -780,13 +782,13 @@ export class NavigationTransitions {
780782
take(1),
781783

782784
takeUntil(
783-
abortSignalToObservable(overallTransitionState.abortController.signal).pipe(
785+
abortSignalToObservable(abortController.signal).pipe(
784786
// Ignore aborts if we are already completed, canceled, or are in the activation stage (we have targetRouterState)
785787
filter(() => !completedOrAborted && !overallTransitionState.targetRouterState),
786788
tap(() => {
787789
this.cancelNavigationTransition(
788790
overallTransitionState,
789-
overallTransitionState.abortController.signal.reason + '',
791+
abortController.signal.reason + '',
790792
NavigationCancellationCode.Aborted,
791793
);
792794
}),
@@ -796,6 +798,10 @@ export class NavigationTransitions {
796798
tap({
797799
next: (t: NavigationTransition) => {
798800
completedOrAborted = true;
801+
this.currentNavigation.update((nav) => {
802+
(nav as Writable<Navigation>).abort = noop;
803+
return nav;
804+
});
799805
this.lastSuccessfulNavigation.set(untracked(this.currentNavigation));
800806
this.events.next(
801807
new NavigationEnd(
@@ -828,7 +834,7 @@ export class NavigationTransitions {
828834
),
829835

830836
finalize(() => {
831-
overallTransitionState.abortController.abort();
837+
abortController.abort();
832838
/* When the navigation stream finishes either through error or success,
833839
* we set the `completed` or `errored` flag. However, there are some
834840
* situations where we could get here without either of those being set.

0 commit comments

Comments
 (0)