Skip to content

Commit

Permalink
fix(core): use WeakRef to prevent object retention in WeakMap
Browse files Browse the repository at this point in the history
Although keys are not strongly referenced in a `WeakMap`, values are, potentially leading to data retention issues and improper garbage collection. By utilizing `WeakRef`, this problem can be mitigated effectively. Weak references allow the garbage collector to collect an object even if it is only weakly referenced. This can prevent memory leaks in the injector debugger profiler.

Closes angular#55396
  • Loading branch information
alan-agius4 committed Apr 23, 2024
1 parent f84d7dd commit 28d83d0
Show file tree
Hide file tree
Showing 5 changed files with 28 additions and 27 deletions.
40 changes: 20 additions & 20 deletions packages/core/src/render3/debug/framework_injector_profiler.ts
Original file line number Diff line number Diff line change
Expand Up @@ -6,11 +6,12 @@
* found in the LICENSE file at https://angular.io/license
*/

import {InjectionToken} from '../../di';
import {Injector} from '../../di/injector';
import {EnvironmentInjector} from '../../di/r3_injector';
import {Type} from '../../interface/type';
import {assertDefined, throwError} from '../../util/assert';
import {assertTNode, assertTNodeForLView} from '../assert';
import {assertTNodeForLView} from '../assert';
import {getComponentDef} from '../definition';
import {getNodeInjectorLView, getNodeInjectorTNode, NodeInjector} from '../di';
import {TNode} from '../interfaces/node';
Expand Down Expand Up @@ -54,16 +55,15 @@ import {InjectedService, InjectorCreatedInstance, InjectorProfilerContext, Injec
*
*/
class DIDebugData {
resolverToTokenToDependencies =
new WeakMap<Injector|LView, WeakMap<Type<unknown>, InjectedService[]>>();
resolverToProviders = new WeakMap<Injector|TNode, ProviderRecord[]>();
resolverToTokenToDependencies = new WeakMap<
Injector|LView, WeakMap<Type<unknown>|InjectionToken<unknown>, WeakRef<InjectedService[]>>>();
resolverToProviders = new WeakMap<Injector|TNode, WeakRef<ProviderRecord[]>>();
standaloneInjectorToComponent = new WeakMap<Injector, Type<unknown>>();

reset() {
this.resolverToTokenToDependencies =
new WeakMap<Injector|LView, WeakMap<Type<unknown>, InjectedService[]>>();
this.resolverToProviders = new WeakMap<Injector|TNode, ProviderRecord[]>();
this.standaloneInjectorToComponent = new WeakMap<Injector, Type<unknown>>();
this.resolverToTokenToDependencies = new WeakMap();
this.resolverToProviders = new WeakMap();
this.standaloneInjectorToComponent = new WeakMap();
}
}

Expand Down Expand Up @@ -121,7 +121,7 @@ function handleInjectEvent(context: InjectorProfilerContext, data: InjectedServi
const diResolverToInstantiatedToken = frameworkDIDebugData.resolverToTokenToDependencies;

if (!diResolverToInstantiatedToken.has(diResolver)) {
diResolverToInstantiatedToken.set(diResolver, new WeakMap<Type<unknown>, InjectedService[]>());
diResolverToInstantiatedToken.set(diResolver, new WeakMap());
}

// if token is a primitive type, ignore this event. We do this because we cannot keep track of
Expand All @@ -131,17 +131,15 @@ function handleInjectEvent(context: InjectorProfilerContext, data: InjectedServi
}

const instantiatedTokenToDependencies = diResolverToInstantiatedToken.get(diResolver)!;
if (!instantiatedTokenToDependencies.has(context.token!)) {
instantiatedTokenToDependencies.set(context.token!, []);
}

const {token, value, flags} = data;

assertDefined(context.token, 'Injector profiler context token is undefined.');

const dependencies = instantiatedTokenToDependencies.get(context.token);
assertDefined(dependencies, 'Could not resolve dependencies for token.');
let dependencies = instantiatedTokenToDependencies.get(context.token)?.deref();
if (!dependencies) {
dependencies = [];
instantiatedTokenToDependencies.set(context.token, new WeakRef(dependencies));
}

const {token, value, flags} = data;
if (context.injector instanceof NodeInjector) {
dependencies.push({token, value, flags, injectedIn: getNodeInjectorContext(context.injector)});
} else {
Expand Down Expand Up @@ -251,11 +249,13 @@ function handleProviderConfiguredEvent(
throwError('A ProviderConfigured event must be run within an injection context.');
}

if (!resolverToProviders.has(diResolver)) {
resolverToProviders.set(diResolver, []);
let resolverData = resolverToProviders.get(diResolver)?.deref();
if (!resolverData) {
resolverData = [];
resolverToProviders.set(diResolver, new WeakRef(resolverData));
}

resolverToProviders.get(diResolver)!.push(data);
resolverData.push(data);
}

function getDIResolver(injector: Injector|undefined): Injector|LView|null {
Expand Down
9 changes: 5 additions & 4 deletions packages/core/src/render3/util/injector_discovery_utils.ts
Original file line number Diff line number Diff line change
Expand Up @@ -132,12 +132,12 @@ function getDependenciesForTokenInInjector<T>(
const {resolverToTokenToDependencies} = getFrameworkDIDebugData();

if (!(injector instanceof NodeInjector)) {
return resolverToTokenToDependencies.get(injector)?.get?.(token as Type<T>) ?? [];
return resolverToTokenToDependencies.get(injector)?.get?.(token)?.deref() ?? [];
}

const lView = getNodeInjectorLView(injector);
const tokenDependencyMap = resolverToTokenToDependencies.get(lView);
const dependencies = tokenDependencyMap?.get(token as Type<T>) ?? [];
const dependencies = tokenDependencyMap?.get(token)?.deref() ?? [];

// In the NodeInjector case, all injections for every node are stored in the same lView.
// We use the injectedIn field of the dependency to filter out the dependencies that
Expand Down Expand Up @@ -207,7 +207,8 @@ function getProviderImportsContainer(injector: Injector): Type<unknown>|null {
function getNodeInjectorProviders(injector: NodeInjector): ProviderRecord[] {
const diResolver = getNodeInjectorTNode(injector);
const {resolverToProviders} = getFrameworkDIDebugData();
return resolverToProviders.get(diResolver as TNode) ?? [];

return resolverToProviders.get(diResolver as TNode)?.deref() ?? [];
}

/**
Expand Down Expand Up @@ -395,7 +396,7 @@ function walkProviderTreeToDiscoverImportPaths(
*/
function getEnvironmentInjectorProviders(injector: EnvironmentInjector): ProviderRecord[] {
const providerRecordsWithoutImportPaths =
getFrameworkDIDebugData().resolverToProviders.get(injector) ?? [];
getFrameworkDIDebugData().resolverToProviders.get(injector)?.deref() ?? [];

// platform injector has no provider imports container so can we skip trying to
// find import paths
Expand Down
2 changes: 1 addition & 1 deletion packages/tsconfig-build.json
Original file line number Diff line number Diff line change
Expand Up @@ -21,7 +21,7 @@
"target": "es2022",
// Keep the below in sync with ng_module.bzl
"useDefineForClassFields": false,
"lib": ["es2020", "dom", "dom.iterable"],
"lib": ["es2020", "dom", "dom.iterable", "ES2021.WeakRef"],
"skipLibCheck": true,
// don't auto-discover @types/node, it results in a ///<reference in the .d.ts output
"types": [],
Expand Down
2 changes: 1 addition & 1 deletion packages/tsconfig-tsec-base.json
Original file line number Diff line number Diff line change
Expand Up @@ -3,7 +3,7 @@
"extends": "./tsconfig-build.json",
"compilerOptions": {
"noEmit": true,
"lib": ["es2020", "dom", "dom.iterable"],
"lib": ["es2020", "dom", "dom.iterable", "ES2021.WeakRef"],
"plugins": [{"name": "tsec", "exemptionConfig": "./tsec-exemption.json"}]
}
}
2 changes: 1 addition & 1 deletion packages/tsconfig.json
Original file line number Diff line number Diff line change
Expand Up @@ -29,7 +29,7 @@
},
"rootDir": ".",
"inlineSourceMap": true,
"lib": ["es2020", "dom", "dom.iterable"],
"lib": ["es2020", "dom", "dom.iterable", "ES2021.WeakRef"],
"skipDefaultLibCheck": true,
"skipLibCheck": true,
"types": ["angular"]
Expand Down

0 comments on commit 28d83d0

Please sign in to comment.