From a736ba4791323b1e3b935304ba1bc789e854d817 Mon Sep 17 00:00:00 2001 From: arturovt Date: Wed, 30 Apr 2025 18:57:52 +0300 Subject: [PATCH] fix(common): cleanup `updateLatestValue` if view is destroyed before promise resolves Patch version of https://github.com/angular/angular/pull/58041. --- packages/common/src/pipes/async_pipe.ts | 48 +++++++++++++++++++++---- 1 file changed, 42 insertions(+), 6 deletions(-) diff --git a/packages/common/src/pipes/async_pipe.ts b/packages/common/src/pipes/async_pipe.ts index 034f17ac6998..c25d4de4c2c2 100644 --- a/packages/common/src/pipes/async_pipe.ts +++ b/packages/common/src/pipes/async_pipe.ts @@ -16,7 +16,7 @@ import { ɵisPromise, ɵisSubscribable, } from '@angular/core'; -import {Observable, Subscribable, Unsubscribable} from 'rxjs'; +import type {Observable, Subscribable, Unsubscribable} from 'rxjs'; import {invalidPipeArgumentError} from './invalid_pipe_argument_error'; @@ -54,13 +54,49 @@ class SubscribableStrategy implements SubscriptionStrategy { } class PromiseStrategy implements SubscriptionStrategy { - createSubscription(async: Promise, updateLatestValue: (v: any) => any): Promise { - return async.then(updateLatestValue, (e) => { - throw e; - }); + createSubscription( + async: Promise, + updateLatestValue: ((v: any) => any) | null, + ): Unsubscribable { + // 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. + async.then( + // Using optional chaining because we may have set it to `null`; since the promise + // is async, the view might be destroyed by the time the promise resolves. + (v) => updateLatestValue?.(v), + (e) => { + throw e; + }, + ); + return { + unsubscribe: () => { + updateLatestValue = null; + }, + }; } - dispose(subscription: Promise): void {} + dispose(subscription: Unsubscribable): void { + subscription.unsubscribe(); + } } const _promiseStrategy = new PromiseStrategy();