8000 fix(common): cleanup `updateLatestValue` if view is destroyed before … · angular/angular@255c79e · GitHub
[go: up one dir, main page]
Skip to content

Commit 255c79e

Browse files
arturovtAndrewKushnir
arturovt
authored and
AndrewKushnir
committed
fix(common): cleanup updateLatestValue if view is destroyed before promise resolves (#58041)
According to the promise specification, promises are not cancellable by default. Once a promise is created, it will either resolve or reject, and it doesn't provide a built-in mechanism to cancel it. There may be situations where a promise is provided, and it either resolves after the pipe has been destroyed or never resolves at all. If the promise never resolves — potentially due to factors beyond our control, such as third-party libraries — this can lead to a memory leak. When we use `async.then(updateLatestValue)`, the engine captures a reference to the `updateLatestValue` function. This allows the promise to invoke that function when it resolves. In this case, the promise directly captures a reference to the `updateLatestValue` function. If the promise resolves later, it retains a reference to the original `updateLatestValue`, meaning that even if the context where `updateLatestValue` was defined has been destroyed, the function reference remains in memory. This can lead to memory leaks if `updateLatestValue` is no longer needed or if it holds onto resources that should be released. When we do `async.then(v => ...)` the promise captures a reference to the lambda function (the arrow function). When we assign `updateLatestValue = null` within the context of an `unsubscribe` function, we're changing the reference of `updateLatestValue` in the current scope to `null`. The lambda will no longer have access to it after the assignment, effectively preventing any further calls to the original function and allowing it to be garbage collected. If Chrome is built with additional flags and run with `--allow-natives-syntax --track-retaining-path`, we could use `%DebugTrackRetainingPath` to see the distance from the root for `updateLatestValue` if it's passed directly to async.then, e.g.: ```js %DebugTrackRetainingPath(updateLatestValue); // Distance from root 4: 0x123456789abc <JSPromise (sfi = 0x1fbb02e2d7f1)> ``` PR Close #58041
1 parent d4dc7cb commit 255c79e

File tree

1 file changed

+40
-6
lines changed

1 file changed

+40
-6
lines changed

‎packages/common/src/pipes/async_pipe.ts

+40-6
Original file line numberDiff line numberDiff line change
@@ -18,7 +18,7 @@ import {
1818
ɵINTERNAL_APPLICATION_ERROR_HANDLER as INTERNAL_APPLICATION_ERROR_HANDLER,
1919
inject,
2020
} from '@angular/core';
21-
import {Observable, Subscribable, Unsubscribable} from 'rxjs';
21+
import type {Observable, Subscribable, Unsubscribable} from 'rxjs';
2222

2323
import {invalidPipeArgumentError} from './invalid_pipe_argument_error';
2424

@@ -61,13 +61,47 @@ class SubscribableStrategy implements SubscriptionStrategy {
6161
class PromiseStrategy implements SubscriptionStrategy {
6262
createSubscription(
6363
async: PromiseLike<any>,
64-
updateLatestValue: (v: any) => any,
65-
onError: (e: unknown) => void,
66-
): PromiseLike<any> {
67-
return async.then(updateLatestValue, onError);
64+
updateLatestValue: ((v: any) => any) | null,
65+
onError: ((e: unknown) => void) | null,
66+
): Unsubscribable {
67+
// According to the promise specification, promises are not cancellable by default.
68+
// Once a promise is created, it will either resolve or reject, and it doesn't
69+
// provide a built-in mechanism to cancel it.
70+
// There may be situations where a promise is provided, and it either resolves after
71+
// the pipe has been destroyed or never resolves at all. If the promise never
72+
// resolves — potentially due to factors beyond our control, such as third-party
73+
// libraries — this can lead to a memory leak.
74+
// When we use `async.then(updateLatestValue)`, the engine captures a reference to the
75+
// `updateLatestValue` function. This allows the promise to invoke that function when it
76+
// resolves. In this case, the promise directly captures a reference to the
77+
// `updateLatestValue` function. If the promise resolves later, it retains a reference
78+
// to the original `updateLatestValue`, meaning that even if the context where
79+
// `updateLatestValue` was defined has been destroyed, the function reference remains in memory.
80+
// This can lead to memory leaks if `updateLatestValue` is no longer needed or if it holds
81+
// onto resources that should be released.
82+
// When we do `async.then(v => ...)` the promise captures a reference to the lambda
83+
// function (the arrow function).
84+
// When we assign `updateLatestValue = null` within the context of an `unsubscribe` function,
85+
// we're changing the reference of `updateLatestValue` in the current scope to `null`.
86+
// The lambda will no longer have access to it after the assignment, effectively
87+
// preventing any further calls to the original function and allowing it to be garbage collected.
88+
async.then(
89+
// Using optional chaining because we may have set it to `null`; since the promise
90+
// is async, the view might be destroyed by the time the promise resolves.
91+
(v) => updateLatestValue?.(v),
92+
(e) => onError?.(e),
93+
);
94+
return {
95+
unsubscribe: () => {
96+
updateLatestValue = null;
97+
onError = null;
98+
},
99+
};
68100
}
69101

70-
dispose(subscription: PromiseLike<any>): void {}
102+
dispose(subscription: Unsubscribable): void {
103+
subscription.unsubscribe();
104+
}
71105
}
72106

73107
const _promiseStrategy = new PromiseStrategy();

0 commit comments

Comments
 (0)
0