Skip to content

Commit ab93e4b

Browse files
hajekjirihajekjiri
committed
fix(core): fix race condition in class dependency resolution
Fix race condition in class dependency resolution, which could otherwise lead to undefined or null injection. This is a followup to #15405 that fixes another case of this bug that was not taken care of in the previous PR. In this case specifically, the staticity of the dependency tree could be checked before all dependencies are loaded and the param / property Barrier is passed, resulting in a potentially wrong evaluation of the `isTreeStatic` property of the `InstanceWrapper`. Closes #4873
1 parent d559b81 commit ab93e4b

File tree

5 files changed

+113
-146
lines changed

5 files changed

+113
-146
lines changed

‎integration/injector/e2e/many-global-modules.spec.ts‎

Lines changed: 28 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -49,21 +49,42 @@ class TransientProvider {}
4949
@Injectable()
5050
class RequestProvider {}
5151

52+
@Injectable()
53+
class ForeignTransientProvider {}
54+
5255
@Injectable()
5356
export class Dependant {
5457
constructor(
5558
private readonly transientProvider: TransientProvider,
5659

60+
private readonly foreignTransientProvider: ForeignTransientProvider,
61+
5762
@Inject(RequestProvider)
5863
private readonly requestProvider: RequestProvider,
5964
) {}
6065

6166
public checkDependencies() {
6267
expect(this.transientProvider).to.be.instanceOf(TransientProvider);
68+
expect(this.foreignTransientProvider).to.be.instanceOf(
69+
ForeignTransientProvider,
70+
);
6371
expect(this.requestProvider).to.be.instanceOf(RequestProvider);
6472
}
6573
}
6674

75+
@Global()
76+
@Module({
77+
providers: [
78+
{
79+
provide: ForeignTransientProvider,
80+
scope: Scope.TRANSIENT,
81+
useClass: ForeignTransientProvider,
82+
},
83+
],
84+
exports: [ForeignTransientProvider],
85+
})
86+
export class ModuleWithForeignTransientProvider {}
87+
6788
@Global()
6889
@Module({
6990
providers: [
@@ -98,6 +119,12 @@ export class GlobalModuleWithRequestProvider {}
98119

99120
@Module({
100121
imports: [
122+
/*
123+
* ForeginTransientProvider will be resolved quickly because its host module is imported first.
124+
* IMPORTANT: Do not move this module, otherwise we may not catch future regressions.
125+
*/
126+
ModuleWithForeignTransientProvider,
127+
101128
GlobalModule1,
102129
GlobalModule2,
103130
GlobalModule3,
@@ -115,7 +142,7 @@ export class GlobalModuleWithRequestProvider {}
115142
export class AppModule {}
116143

117144
describe('Many global modules', () => {
118-
it('should inject request-scoped useFactory provider and transient-scoped useClass provider from different modules', async () => {
145+
it('should inject request-scoped and transient-scoped providers from different modules', async () => {
119146
const moduleBuilder = Test.createTestingModule({
120147
imports: [AppModule],
121148
});

‎integration/inspector/e2e/fixtures/post-init-graph.json‎

Lines changed: 32 additions & 32 deletions
Original file line numberDiff line numberDiff line change
@@ -2263,6 +2263,22 @@
22632263
},
22642264
"id": "-1303681274"
22652265
},
2266+
"-831049991": {
2267+
"source": "594986539",
2268+
"target": "-1721730431",
2269+
"metadata": {
2270+
"type": "class-to-class",
2271+
"sourceModuleName": "InputModule",
2272+
"sourceClassName": "InputService",
2273+
"targetClassName": "CircularService",
2274+
"sourceClassToken": "InputService",
2275+
"targetClassToken": "CircularService",
2276+
"targetModuleName": "CircularModule",
2277+
"keyOrIndex": 0,
2278+
"injectionType": "constructor"
2279+
},
2280+
"id": "-831049991"
2281+
},
22662282
"-886102564": {
22672283
"source": "208171089",
22682284
"target": "671882984",
@@ -2280,6 +2296,22 @@
22802296
},
22812297
"id": "-886102564"
22822298
},
2299+
"-2146943494": {
2300+
"source": "-234035039",
2301+
"target": "928565345",
2302+
"metadata": {
2303+
"type": "class-to-class",
2304+
"sourceModuleName": "RequestChainModule",
2305+
"sourceClassName": "RequestChainService",
2306+
"targetClassName": "HelperService",
2307+
"sourceClassToken": "RequestChainService",
2308+
"targetClassToken": "HelperService",
2309+
"targetModuleName": "HelperModule",
2310+
"keyOrIndex": 0,
2311+
"injectionType": "constructor"
2312+
},
2313+
"id": "-2146943494"
2314+
},
22832315
"-2003045613": {
22842316
"source": "-377928898",
22852317
"target": "-616397055",
@@ -2312,38 +2344,6 @@
23122344
},
23132345
"id": "-881420795"
23142346
},
2315-
"-831049991": {
2316-
"source": "594986539",
2317-
"target": "-1721730431",
2318-
"metadata": {
2319-
"type": "class-to-class",
2320-
"sourceModuleName": "InputModule",
2321-
"sourceClassName": "InputService",
2322-
"targetClassName": "CircularService",
2323-
"sourceClassToken": "InputService",
2324-
"targetClassToken": "CircularService",
2325-
"targetModuleName": "CircularModule",
2326-
"keyOrIndex": 0,
2327-
"injectionType": "constructor"
2328-
},
2329-
"id": "-831049991"
2330-
},
2331-
"-2146943494": {
2332-
"source": "-234035039",
2333-
"target": "928565345",
2334-
"metadata": {
2335-
"type": "class-to-class",
2336-
"sourceModuleName": "RequestChainModule",
2337-
"sourceClassName": "RequestChainService",
2338-
"targetClassName": "HelperService",
2339-
"sourceClassToken": "RequestChainService",
2340-
"targetClassToken": "HelperService",
2341-
"targetModuleName": "HelperModule",
2342-
"keyOrIndex": 0,
2343-
"injectionType": "constructor"
2344-
},
2345-
"id": "-2146943494"
2346-
},
23472347
"-1816180282": {
23482348
"source": "-848516688",
23492349
"target": "-1673986099",

‎integration/inspector/e2e/fixtures/pre-init-graph.json‎

Lines changed: 48 additions & 48 deletions
Original file line numberDiff line numberDiff line change
@@ -2181,22 +2181,6 @@
21812181
},
21822182
"id": "1976848738"
21832183
},
2184-
"-2105726668": {
2185-
"source": "-1803759743",
2186-
"target": "1010833816",
2187-
"metadata": {
2188-
"type": "class-to-class",
2189-
"sourceModuleName": "PropertiesModule",
2190-
"sourceClassName": "PropertiesService",
2191-
"targetClassName": "token",
2192-
"sourceClassToken": "PropertiesService",
2193-
"targetClassToken": "token",
2194-
"targetModuleName": "PropertiesModule",
2195-
"keyOrIndex": "token",
2196-
"injectionType": "property"
2197-
},
2198-
"id": "-2105726668"
2199-
},
22002184
"-21463590": {
22012185
"source": "-1378706112",
22022186
"target": "1004276345",
@@ -2213,6 +2197,22 @@
22132197
},
22142198
"id": "-21463590"
22152199
},
2200+
"-2105726668": {
2201+
"source": "-1803759743",
2202+
"target": "1010833816",
2203+
"metadata": {
2204+
"type": "class-to-class",
2205+
"sourceModuleName": "PropertiesModule",
2206+
"sourceClassName": "PropertiesService",
2207+
"targetClassName": "token",
2208+
"sourceClassToken": "PropertiesService",
2209+
"targetClassToken": "token",
2210+
"targetModuleName": "PropertiesModule",
2211+
"keyOrIndex": "token",
2212+
"injectionType": "property"
2213+
},
2214+
"id": "-2105726668"
2215+
},
22162216
"-1657371464": {
22172217
"source": "-1673986099",
22182218
"target": "1919157847",
@@ -2247,6 +2247,22 @@
22472247
},
22482248
"id": "-1303681274"
22492249
},
2250+
"-831049991": {
2251+
"source": "594986539",
2252+
"target": "-1721730431",
2253+
"metadata": {
2254+
"type": "class-to-class",
2255+
"sourceModuleName": "InputModule",
2256+
"sourceClassName": "InputService",
2257+
"targetClassName": "CircularService",
2258+
"sourceClassToken": "InputService",
2259+
"targetClassToken": "CircularService",
2260+
"targetModuleName": "CircularModule",
2261+
"keyOrIndex": 0,
2262+
"injectionType": "constructor"
2263+
},
2264+
"id": "-831049991"
2265+
},
22502266
"-886102564": {
22512267
"source": "208171089",
22522268
"target": "671882984",
@@ -2264,6 +2280,22 @@
22642280
},
22652281
"id": "-886102564"
22662282
},
2283+
"-2146943494": {
2284+
"source": "-234035039",
2285+
"target": "928565345",
2286+
"metadata": {
2287+
"type": "class-to-class",
2288+
"sourceModuleName": "RequestChainModule",
2289+
"sourceClassName": "RequestChainService",
2290+
"targetClassName": "HelperService",
2291+
"sourceClassToken": "RequestChainService",
2292+
"targetClassToken": "HelperService",
2293+
"targetModuleName": "HelperModule",
2294+
"keyOrIndex": 0,
2295+
"injectionType": "constructor"
2296+
},
2297+
"id": "-2146943494"
2298+
},
22672299
"-2003045613": {
22682300
"source": "-377928898",
22692301
"target": "-616397055",
@@ -2296,38 +2328,6 @@
22962328
},
22972329
"id": "-881420795"
22982330
},
2299-
"-831049991": {
2300-
"source": "594986539",
2301-
"target": "-1721730431",
2302-
"metadata": {
2303-
"type": "class-to-class",
2304-
"sourceModuleName": "InputModule",
2305-
"sourceClassName": "InputService",
2306-
"targetClassName": "CircularService",
2307-
"sourceClassToken": "InputService",
2308-
"targetClassToken": "CircularService",
2309-
"targetModuleName": "CircularModule",
2310-
"keyOrIndex": 0,
2311-
"injectionType": "constructor"
2312-
},
2313-
"id": "-831049991"
2314-
},
2315-
"-2146943494": {
2316-
"source": "-234035039",
2317-
"target": "928565345",
2318-
"metadata": {
2319-
"type": "class-to-class",
2320-
"sourceModuleName": "RequestChainModule",
2321-
"sourceClassName": "RequestChainService",
2322-
"targetClassName": "HelperService",
2323-
"sourceClassToken": "RequestChainService",
2324-
"targetClassToken": "HelperService",
2325-
"targetModuleName": "HelperModule",
2326-
"keyOrIndex": 0,
2327-
"injectionType": "constructor"
2328-
},
2329-
"id": "-2146943494"
2330-
},
23312331
"-1816180282": {
23322332
"source": "-848516688",
23332333
"target": "-1673986099",

‎packages/core/injector/injector.ts‎

Lines changed: 5 additions & 8 deletions
Original file line numberDiff line numberDiff line change
@@ -680,14 +680,11 @@ export class Injector {
680680
inquirerId,
681681
);
682682
if (!instanceHost.isResolved && !instanceWrapperRef.forwardRef) {
683-
wrapper.settlementSignal?.insertRef(instanceWrapperRef.id);
684-
685-
await this.loadProvider(
686-
instanceWrapperRef,
687-
relatedModule,
688-
contextId,
689-
wrapper,
690-
);
683+
/*
684+
* Provider will be loaded shortly in resolveComponentHost() once we pass the current
685+
* Barrier. We cannot load it here because doing so could incorrectly evaluate the
686+
* staticity of the dependency tree and lead to undefined / null injection.
687+
*/
691688
break;
692689
}
693690
}

‎packages/core/test/injector/injector.spec.ts‎

Lines changed: 0 additions & 57 deletions
Original file line numberDiff line numberDiff line change
@@ -447,63 +447,6 @@ describe('Injector', () => {
447447
),
448448
).to.eventually.be.eq(null);
449449
});
450-
451-
it('should call "loadProvider" when component is not resolved', async () => {
452-
const moduleFixture = {
453-
imports: new Map([
454-
[
455-
'key',
456-
{
457-
providers: {
458-
has: () => true,
459-
get: () =>
460-
new InstanceWrapper({
461-
isResolved: false,
462-
}),
463-
},
464-
exports: {
465-
has: () => true,
466-
},
467-
imports: new Map(),
468-
},
469-
],
470-
] as any),
471-
};
472-
await injector.lookupComponentInImports(
473-
moduleFixture as any,
474-
metatype as any,
475-
new InstanceWrapper(),
476-
);
477-
expect(loadProvider.called).to.be.true;
478-
});
479-
480-
it('should not call "loadProvider" when component is resolved', async () => {
481-
const moduleFixture = {
482-
relatedModules: new Map([
483-
[
484-
'key',
485-
{
486-
providers: {
487-
has: () => true,
488-
get: () => ({
489-
isResolved: true,
490-
}),
491-
},
492-
exports: {
493-
has: () => true,
494-
},
495-
relatedModules: new Map(),
496-
},
497-
],
498-
] as any),
499-
};
500-
await injector.lookupComponentInImports(
501-
moduleFixture as any,
502-
metatype as any,
503-
null!,
504-
);
505-
expect(loadProvider.called).to.be.false;
506-
});
507450
});
508451

509452
describe('resolveParamToken', () => {

0 commit comments

Comments
 (0)