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

Commit 9de049f

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 e5cf378 commit 9de049f

File tree

2 files changed

+87
-44
lines changed

2 files changed

+87
-44
lines changed

‎lib/core_dom/directive_injector.dart

Lines changed: 76 additions & 44 deletions
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 _resolvingDepth;
128+
125129
@Deprecated("Deprecated. Use getFromParent instead.")
126130
Object get parent => this._parent;
127131

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

201205
Object getByKey(Key key) {
206+
// All 'get'-like methods should go through this code, so that the depth counter is correct.
207+
_resolvingDepth = MAX_RESOLVING_DEPTH;
202208
var oldTag = _TAG_GET.makeCurrent();
203209
try {
204210
return _getByKey(key);
@@ -220,16 +226,16 @@ class DirectiveInjector implements DirectiveBinder {
220226

221227
Object _getDirectiveByKey(Key k, int visType, Injector i) {
222228
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;
229+
if (_key0 == null) break; if (identical(_key0, k)) return _obj0 == null ? _obj0 = _new(k, _pKeys0, _factory0) : _obj0;
230+
if (_key1 == null) break; if (identical(_key1, k)) return _obj1 == null ? _obj1 = _new(k, _pKeys1, _factory1) : _obj1;
231+
if (_key2 == null) break; if (identical(_key2, k)) return _obj2 == null ? _obj2 = _new(k, _pKeys2, _factory2) : _obj2;
232+
if (_key3 == null) break; if (identical(_key3, k)) return _obj3 == null ? _obj3 = _new(k, _pKeys3, _factory3) : _obj3;
233+
if (_key4 == null) break; if (identical(_key4, k)) return _obj4 == null ? _obj4 = _new(k, _pKeys4, _factory4) : _obj4;
234+
if (_key5 == null) break; if (identical(_key5, k)) return _obj5 == null ? _obj5 = _new(k, _pKeys5, _factory5) : _obj5;
235+
if (_key6 == null) break; if (identical(_key6, k)) return _obj6 == null ? _obj6 = _new(k, _pKeys6, _factory6) : _obj6;
236+
if (_key7 == null) break; if (identical(_key7, k)) return _obj7 == null ? _obj7 = _new(k, _pKeys7, _factory7) : _obj7;
237+
if (_key8 == null) break; if (identical(_key8, k)) return _obj8 == null ? _obj8 = _new(k, _pKeys8, _factory8) : _obj8;
238+
if (_key9 == null) break; if (identical(_key9, k)) return _obj9 == null ? _obj9 = _new(k, _pKeys9, _factory9) : _obj9;
233239
} while (false);
234240
switch (visType) {
235241
case VISIBILITY_LOCAL: return appInjector.getByKey(k);
@@ -275,7 +281,10 @@ class DirectiveInjector implements DirectiveBinder {
275281
}
276282
}
277283

278-
dynamic _new(List<Key> paramKeys, Function fn) {
284+
dynamic _new(Key k, List<Key> paramKeys, Function fn) {
285+
if (_resolvingDepth-- == 0) {
286+
throw new DiCircularDependencyError(k);
287+
}
279288
var oldTag = _TAG_GET.makeCurrent();
280289
int size = paramKeys.length;
281290
var obj;
@@ -287,39 +296,44 @@ class DirectiveInjector implements DirectiveBinder {
287296
_TAG_INSTANTIATE.makeCurrent();
288297
obj = Function.apply(fn, params);
289298
} 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);
299+
try {
300+
var a01 = size >= 01 ? _getByKey(paramKeys[00]) : null;
301+
var a02 = size >= 02 ? _getByKey(paramKeys[01]) : null;
302+
var a03 = size >= 03 ? _getByKey(paramKeys[02]) : null;
303+
var a04 = size >= 04 ? _getByKey(paramKeys[03]) : null;
304+
var a05 = size >= 05 ? _getByKey(paramKeys[04]) : null;
305+
var a06 = size >= 06 ? _getByKey(paramKeys[05]) : null;
306+
var a07 = size >= 07 ? _getByKey(paramKeys[06]) : null;
307+
var a08 = size >= 08 ? _getByKey(paramKeys[07]) : null;
308+
var a09 = size >= 09 ? _getByKey(paramKeys[08]) : null;
309+
var a10 = size >= 10 ? _getByKey(paramKeys[09]) : null;
310+
var a11 = size >= 11 ? _getByKey(paramKeys[10]) : null;
311+
var a12 = size >= 12 ? _getByKey(paramKeys[11]) : null;
312+
var a13 = size >= 13 ? _getByKey(paramKeys[12]) : null;
313+
var a14 = size >= 14 ? _getByKey(paramKeys[13]) : null;
314+
var a15 = size >= 15 ? _getByKey(paramKeys[14]) : null;
315+
_TAG_INSTANTIATE.makeCurrent();
316+
switch(size) {
317+
case 00: obj = fn(); break;
318+
case 01: obj = fn(a01); break;
319+
case 02: obj = fn(a01, a02); break;
320+
case 03: obj = fn(a01, a02, a03); break;
321+
case 04: obj = fn(a01, a02, a03, a04); break;
322+
case 05: obj = fn(a01, a02, a03, a04, a05); break;
323+
case 06: obj = fn(a01, a02, a03, a04, a05, a06); break;
324+
case 07: obj = fn(a01, a02, a03, a04, a05, a06, a07); break;
325+
case 08: obj = fn(a01, a02, a03, a04, a05, a06, a07, a08); break;
326+
case 09: obj = fn(a01, a02, a03, a04, a05, a06, a07, a08, a09); break;
327+
case 10: obj = fn(a01, a02, a03, a04, a05, a06, a07, a08, a09, a10); break;
328+
case 11: obj = fn(a01, a02, a03, a04, a05, a06, a07, a08, a09, a10, a11); break;
329+
case 12: obj = fn(a01, a02, a03, a04, a05, a06, a07, a08, a09, a10, a11, a12); break;
330+
case 13: obj = fn(a01, a02, a03, a04, a05, a06, a07, a08, a09, a10, a11, a12, a13); break;
331+
case 14: obj = fn(a01, a02, a03, a04, a05, a06, a07, a08, a09, a10, a11, a12, a13, a14); break;
332+
case 15: obj = fn(a01, a02, a03, a04, a05, a06, a07, a08, a09, a10, a11, a12, a13, a14, a15);
333+
}
334+
} on DiCircularDependencyError catch (e) {
335+
e.keys.add(k);
336+
rethrow;
323337
}
324338
}
325339
oldTag.makeCurrent();
@@ -439,3 +453,21 @@ class DefaultDirectiveInjector extends DirectiveInjector {
439453
}
440454
}
441455
}
456+
457+
class DiCircularDependencyError extends Error {
458+
final List<Key> keys;
459+
DiCircularDependencyError(key) : keys = [key];
460+
461+
// strips the cyclical part of the chain.
462+
List<Key> get resolveChain {
463+
List<Key> rkeys = keys.reversed.toList();
464+
for (num i = 0; i < rkeys.length; i++) {
465+
for (num j = i + 1; j < rkeys.length; j++) {
466+
if (rkeys[i] == rkeys[j]) return rkeys.sublist(0, j + 1);
467+
}
468+
}
469+
return rkeys;
470+
}
471+
String toString() => "circular dependency (${resolveChain.join(' -> ')})";
472+
}
473+

‎test/core_dom/directive_injector_spec.dart

Lines changed: 11 additions & 0 deletions
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)