Skip to content
This repository was archived by the owner on Feb 22, 2018. It is now read-only.

Commit 3f67340

Browse files
rkirovrkirov
committed
feat(directive-injector): detect and throw on circular deps
The maximum depth for depth for dependency resolution is set to 50. No performance degradation observed on tree benchmark.
1 parent 4b6c45e commit 3f67340

File tree

2 files changed

+86
-44
lines changed

2 files changed

+86
-44
lines changed

‎lib/core_dom/directive_injector.dart

+75-44
Original file line numberDiff line numberDiff line change
@@ -52,6 +52,8 @@ const int CONTENT_PORT_KEY_ID = 16;
5252
const int EVENT_HANDLER_KEY_ID = 17;
5353
const int KEEP_ME_LAST = 18;
5454

55+
const int MAX_RESOLVING_DEPTH = 50;
56+
5557
class DirectiveInjector implements DirectiveBinder {
5658
static bool _isInit = false;
5759
static initUID() {
@@ -122,6 +124,8 @@ class DirectiveInjector implements DirectiveBinder {
122124
Key _key8 = null; dynamic _obj8; List<Key> _pKeys8; Function _factory8;
123125
Key _key9 = null; dynamic _obj9; List<Key> _pKeys9; Function _factory9;
124126

127+
int _resolving_depth;
128+
125129
@Deprecated("Deprecated. Use getFromParent instead.")
126130
Object get parent => this._parent;
127131

@@ -199,6 +203,7 @@ class DirectiveInjector implements DirectiveBinder {
199203
Object getFromParent(Type type) => _parent.get(type);
200204

201205
Object getByKey(Key key) {
206+
_resolving_depth = MAX_RESOLVING_DEPTH;
202207
var oldTag = _TAG_GET.makeCurrent();
203208
try {
204209
return _getByKey(key);
@@ -220,16 +225,16 @@ class DirectiveInjector implements DirectiveBinder {
220225

221226
Object _getDirectiveByKey(Key k, int visType, Injector i) {
222227
do {
223-
if (_key0 == null) break; if (identical(_key0, k)) return _obj0 == null ? _obj0 = _new(_pKeys0, _factory0) : _obj0;
224-
if (_key1 == null) break; if (identical(_key1, k)) return _obj1 == null ? _obj1 = _new(_pKeys1, _factory1) : _obj1;
225-
if (_key2 == null) break; if (identical(_key2, k)) return _obj2 == null ? _obj2 = _new(_pKeys2, _factory2) : _obj2;
226-
if (_key3 == null) break; if (identical(_key3, k)) return _obj3 == null ? _obj3 = _new(_pKeys3, _factory3) : _obj3;
227-
if (_key4 == null) break; if (identical(_key4, k)) return _obj4 == null ? _obj4 = _new(_pKeys4, _factory4) : _obj4;
228-
if (_key5 == null) break; if (identical(_key5, k)) return _obj5 == null ? _obj5 = _new(_pKeys5, _factory5) : _obj5;
229-
if (_key6 == null) break; if (identical(_key6, k)) return _obj6 == null ? _obj6 = _new(_pKeys6, _factory6) : _obj6;
230-
if (_key7 == null) break; if (identical(_key7, k)) return _obj7 == null ? _obj7 = _new(_pKeys7, _factory7) : _obj7;
231-
if (_key8 == null) break; if (identical(_key8, k)) return _obj8 == null ? _obj8 = _new(_pKeys8, _factory8) : _obj8;
232-
if (_key9 == null) break; if (identical(_key9, k)) return _obj9 == null ? _obj9 = _new(_pKeys9, _factory9) : _obj9;
228+
if (_key0 == null) break; if (identical(_key0, k)) return _obj0 == null ? _obj0 = _new(k, _pKeys0, _factory0) : _obj0;
229+
if (_key1 == null) break; if (identical(_key1, k)) return _obj1 == null ? _obj1 = _new(k, _pKeys1, _factory1) : _obj1;
230+
if (_key2 == null) break; if (identical(_key2, k)) return _obj2 == null ? _obj2 = _new(k, _pKeys2, _factory2) : _obj2;
231+
if (_key3 == null) break; if (identical(_key3, k)) return _obj3 == null ? _obj3 = _new(k, _pKeys3, _factory3) : _obj3;
232+
if (_key4 == null) break; if (identical(_key4, k)) return _obj4 == null ? _obj4 = _new(k, _pKeys4, _factory4) : _obj4;
233+
if (_key5 == null) break; if (identical(_key5, k)) return _obj5 == null ? _obj5 = _new(k, _pKeys5, _factory5) : _obj5;
234+
if (_key6 == null) break; if (identical(_key6, k)) return _obj6 == null ? _obj6 = _new(k, _pKeys6, _factory6) : _obj6;
235+
if (_key7 == null) break; if (identical(_key7, k)) return _obj7 == null ? _obj7 = _new(k, _pKeys7, _factory7) : _obj7;
236+
if (_key8 == null) break; if (identical(_key8, k)) return _obj8 == null ? _obj8 = _new(k, _pKeys8, _factory8) : _obj8;
237+
if (_key9 == null) break; if (identical(_key9, k)) return _obj9 == null ? _obj9 = _new(k, _pKeys9, _factory9) : _obj9;
233238
} while (false);
234239
switch (visType) {
235240
case VISIBILITY_LOCAL: return appInjector.getByKey(k);
@@ -275,7 +280,10 @@ class DirectiveInjector implements DirectiveBinder {
275280
}
276281
}
277282

278-
dynamic _new(List<Key> paramKeys, Function fn) {
283+
dynamic _new(Key k, List<Key> paramKeys, Function fn) {
284+
if (_resolving_depth-- == 0) {
285+
throw new DiCircularDependencyError(k);
286+
}
279287
var oldTag = _TAG_GET.makeCurrent();
280288
int size = paramKeys.length;
281289
var obj;
@@ -287,39 +295,44 @@ class DirectiveInjector implements DirectiveBinder {
287295
_TAG_INSTANTIATE.makeCurrent();
288296
obj = Function.apply(fn, params);
289297
} else {
290-
var a01 = size >= 01 ? _getByKey(paramKeys[00]) : null;
291-
var a02 = size >= 02 ? _getByKey(paramKeys[01]) : null;
292-
var a03 = size >= 03 ? _getByKey(paramKeys[02]) : null;
293-
var a04 = size >= 04 ? _getByKey(paramKeys[03]) : null;
294-
var a05 = size >= 05 ? _getByKey(paramKeys[04]) : null;
295-
var a06 = size >= 06 ? _getByKey(paramKeys[05]) : null;
296-
var a07 = size >= 07 ? _getByKey(paramKeys[06]) : null;
297-
var a08 = size >= 08 ? _getByKey(paramKeys[07]) : null;
298-
var a09 = size >= 09 ? _getByKey(paramKeys[08]) : null;
299-
var a10 = size >= 10 ? _getByKey(paramKeys[09]) : null;
300-
var a11 = size >= 11 ? _getByKey(paramKeys[10]) : null;
301-
var a12 = size >= 12 ? _getByKey(paramKeys[11]) : null;
302-
var a13 = size >= 13 ? _getByKey(paramKeys[12]) : null;
303-
var a14 = size >= 14 ? _getByKey(paramKeys[13]) : null;
304-
var a15 = size >= 15 ? _getByKey(paramKeys[14]) : null;
305-
_TAG_INSTANTIATE.makeCurrent();
306-
switch(size) {
307-
case 00: obj = fn(); break;
308-
case 01: obj = fn(a01); break;
309-
case 02: obj = fn(a01, a02); break;
310-
case 03: obj = fn(a01, a02, a03); break;
311-
case 04: obj = fn(a01, a02, a03, a04); break;
312-
case 05: obj = fn(a01, a02, a03, a04, a05); break;
313-
case 06: obj = fn(a01, a02, a03, a04, a05, a06); break;
314-
case 07: obj = fn(a01, a02, a03, a04, a05, a06, a07); break;
315-
case 08: obj = fn(a01, a02, a03, a04, a05, a06, a07, a08); break;
316-
case 09: obj = fn(a01, a02, a03, a04, a05, a06, a07, a08, a09); break;
317-
case 10: obj = fn(a01, a02, a03, a04, a05, a06, a07, a08, a09, a10); break;
318-
case 11: obj = fn(a01, a02, a03, a04, a05, a06, a07, a08, a09, a10, a11); break;
319-
case 12: obj = fn(a01, a02, a03, a04, a05, a06, a07, a08, a09, a10, a11, a12); break;
320-
case 13: obj = fn(a01, a02, a03, a04, a05, a06, a07, a08, a09, a10, a11, a12, a13); break;
321-
case 14: obj = fn(a01, a02, a03, a04, a05, a06, a07, a08, a09, a10, a11, a12, a13, a14); break;
322-
case 15: obj = fn(a01, a02, a03, a04, a05, a06, a07, a08, a09, a10, a11, a12, a13, a14, a15);
298+
try {
299+
var a01 = size >= 01 ? _getByKey(paramKeys[00]) : null;
300+
var a02 = size >= 02 ? _getByKey(paramKeys[01]) : null;
301+
var a03 = size >= 03 ? _getByKey(paramKeys[02]) : null;
302+
var a04 = size >= 04 ? _getByKey(paramKeys[03]) : null;
303+
var a05 = size >= 05 ? _getByKey(paramKeys[04]) : null;
304+
var a06 = size >= 06 ? _getByKey(paramKeys[05]) : null;
305+
var a07 = size >= 07 ? _getByKey(paramKeys[06]) : null;
306+
var a08 = size >= 08 ? _getByKey(paramKeys[07]) : null;
307+
var a09 = size >= 09 ? _getByKey(paramKeys[08]) : null;
308+
var a10 = size >= 10 ? _getByKey(paramKeys[09]) : null;
309+
var a11 = size >= 11 ? _getByKey(paramKeys[10]) : null;
310+
var a12 = size >= 12 ? _getByKey(paramKeys[11]) : null;
311+
var a13 = size >= 13 ? _getByKey(paramKeys[12]) : null;
312+
var a14 = size >= 14 ? _getByKey(paramKeys[13]) : null;
313+
var a15 = size >= 15 ? _getByKey(paramKeys[14]) : null;
314+
_TAG_INSTANTIATE.makeCurrent();
315+
switch(size) {
316+
case 00: obj = fn(); break;
317+
case 01: obj = fn(a01); break;
318+
case 02: obj = fn(a01, a02); break;
319+
case 03: obj = fn(a01, a02, a03); break;
320+
case 04: obj = fn(a01, a02, a03, a04); break;
321+
case 05: obj = fn(a01, a02, a03, a04, a05); break;
322+
case 06: obj = fn(a01, a02, a03, a04, a05, a06); break;
323+
case 07: obj = fn(a01, a02, a03, a04, a05, a06, a07); break;
324+
case 08: obj = fn(a01, a02, a03, a04, a05, a06, a07, a08); break;
325+
case 09: obj = fn(a01, a02, a03, a04, a05, a06, a07, a08, a09); break;
326+
case 10: obj = fn(a01, a02, a03, a04, a05, a06, a07, a08, a09, a10); break;
327+
case 11: obj = fn(a01, a02, a03, a04, a05, a06, a07, a08, a09, a10, a11); break;
328+
case 12: obj = fn(a01, a02, a03, a04, a05, a06, a07, a08, a09, a10, a11, a12); break;
329+
case 13: obj = fn(a01, a02, a03, a04, a05, a06, a07, a08, a09, a10, a11, a12, a13); break;
330+
case 14: obj = fn(a01, a02, a03, a04, a05, a06, a07, a08, a09, a10, a11, a12, a13, a14); break;
331+
case 15: obj = fn(a01, a02, a03, a04, a05, a06, a07, a08, a09, a10, a11, a12, a13, a14, a15);
332+
}
333+
} on DiCircularDependencyError catch (e) {
334+
e.keys.add(k);
335+
rethrow;
323336
}
324337
}
325338
oldTag.makeCurrent();
@@ -439,3 +452,21 @@ class DefaultDirectiveInjector extends DirectiveInjector {
439452
}
440453
}
441454
}
455+
456+
class DiCircularDependencyError extends Error {
457+
List<Key> keys;
458+
DiCircularDependencyError(key) : keys = [key];
459+
460+
// strips the cyclical part of the chain.
461+
List<Key> get resolveChain {
462+
List<Key> rkeys = keys.reversed.toList();
463+
for (num i = 0; i < rkeys.length; i++) {
464+
for (num j = i + 1; j < rkeys.length; j++) {
465+
if (rkeys[i] == rkeys[j]) return rkeys.sublist(0, j + 1);
466+
}
467+
}
468+
return rkeys;
469+
}
470+
String toString() => "(circular dependency ${resolveChain.join(' -> ')})";
471+
}
472+

‎test/core_dom/directive_injector_spec.dart

+11
Original file line numberDiff line numberDiff line change
@@ -76,6 +76,14 @@ void main() {
7676
expect(() => injector.get(_TypeA)).toThrow('No provider found for _TypeA');
7777
});
7878

79+
it('should throw circular dependency error', () {
80+
addDirective(_TypeC0);
81+
addDirective(_TypeC1);
82+
addDirective(_TypeC2);
83+
expect(() => injector.get(_TypeC0)).toThrow(
84+
'(circular dependency _TypeC0 -> _TypeC1 -> _TypeC2 -> _TypeC1)');
85+
});
86+
7987
describe('Visibility', () {
8088
DirectiveInjector childInjector;
8189
DirectiveInjector leafInjector;
@@ -130,3 +138,6 @@ class _Type8{ final _Type7 type7; _Type8(this.type7); }
130138
class _Type9{ final _Type8 type8; _Type9(this.type8); }
131139
class _TypeA{ final _Type9 type9; _TypeA(this.type9); }
132140

141+
class _TypeC0 {final _TypeC1 t; _TypeC0(this.t);}
142+
class _TypeC1 {final _TypeC2 t; _TypeC1(this.t);}
143+
class _TypeC2 {final _TypeC1 t; _TypeC2(this.t);}

0 commit comments

Comments
 (0)