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

Commit e21e511

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 2b0d9de commit e21e511

File tree

2 files changed

+97
-25
lines changed

2 files changed

+97
-25
lines changed

‎lib/core_dom/directive_injector.dart

+69-23
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,11 @@ 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+
// This value needs to be reset on every entry method into a given instance
128+
// of directiveInjector. Currently those methods are only _getByKey
129+
// and _enterGetDirectiveByKey.
130+
int _resolvingDepth;
131+
125132
@Deprecated("Deprecated. Use getFromParent instead.")
126133
Object get parent => this._parent;
127134

@@ -199,46 +206,62 @@ class DirectiveInjector implements DirectiveBinder {
199206
Object getFromParent(Type type) => _parent.get(type);
200207

201208
Object getByKey(Key key) {
209+
_resolvingDepth = MAX_RESOLVING_DEPTH;
202210
var oldTag = _TAG_GET.makeCurrent();
203211
try {
204212
return _getByKey(key);
205-
} on ResolvingError catch (e, s) {
206-
e.appendKey(key);
207-
rethrow;
208213
} finally {
209214
oldTag.makeCurrent();
210215
}
211216
}
212217
Object getFromParentByKey(Key key) => _parent.getByKey(key);
213218

214219
Object _getByKey(Key key) {
215-
int uid = key.uid;
216-
if (uid == null || uid == UNDEFINED_ID) return appInjector.getByKey(key);
217-
bool isDirective = uid < 0;
218-
return isDirective ? _getDirectiveByKey(key, uid, appInjector) : _getById(uid);
220+
if (_resolvingDepth-- == 0) {
221+
throw new DiCircularDependencyError(key);
222+
}
223+
try {
224+
int uid = key.uid;
225+
if (uid == null || uid == UNDEFINED_ID) return appInjector.getByKey(key);
226+
bool isDirective = uid < 0;
227+
return isDirective ? _getDirectiveByKey(key, uid, appInjector) : _getById(uid);
228+
} on ResolvingError catch (e, s) {
229+
e.appendKey(key);
230+
rethrow;
231+
}
232+
}
233+
234+
Object _enterGetDirectiveByKey(Key k, int visType, Injector i) {
235+
try {
236+
_resolvingDepth = MAX_RESOLVING_DEPTH;
237+
return _getDirectiveByKey(k, visType, i);
238+
} on ResolvingError catch (e, s) {
239+
e.appendKey(k);
240+
rethrow;
241+
}
219242
}
220243

221244
Object _getDirectiveByKey(Key k, int visType, Injector i) {
222245
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;
246+
if (_key0 == null) break; if (identical(_key0, k)) return _obj0 == null ? _obj0 = _new(k, _pKeys0, _factory0) : _obj0;
247+
if (_key1 == null) break; if (identical(_key1, k)) return _obj1 == null ? _obj1 = _new(k, _pKeys1, _factory1) : _obj1;
248+
if (_key2 == null) break; if (identical(_key2, k)) return _obj2 == null ? _obj2 = _new(k, _pKeys2, _factory2) : _obj2;
249+
if (_key3 == null) break; if (identical(_key3, k)) return _obj3 == null ? _obj3 = _new(k, _pKeys3, _factory3) : _obj3;
250+
if (_key4 == null) break; if (identical(_key4, k)) return _obj4 == null ? _obj4 = _new(k, _pKeys4, _factory4) : _obj4;
251+
if (_key5 == null) break; if (identical(_key5, k)) return _obj5 == null ? _obj5 = _new(k, _pKeys5, _factory5) : _obj5;
252+
if (_key6 == null) break; if (identical(_key6, k)) return _obj6 == null ? _obj6 = _new(k, _pKeys6, _factory6) : _obj6;
253+
if (_key7 == null) break; if (identical(_key7, k)) return _obj7 == null ? _obj7 = _new(k, _pKeys7, _factory7) : _obj7;
254+
if (_key8 == null) break; if (identical(_key8, k)) return _obj8 == null ? _obj8 = _new(k, _pKeys8, _factory8) : _obj8;
255+
if (_key9 == null) break; if (identical(_key9, k)) return _obj9 == null ? _obj9 = _new(k, _pKeys9, _factory9) : _obj9;
233256
} while (false);
234257
switch (visType) {
235258
case VISIBILITY_LOCAL: return appInjector.getByKey(k);
236-
case VISIBILITY_DIRECT_CHILD: return _parent._getDirectiveByKey(k, VISIBILITY_LOCAL, i);
237-
case VISIBILITY_CHILDREN: return _parent._getDirectiveByKey(k, VISIBILITY_CHILDREN, i);
259+
case VISIBILITY_DIRECT_CHILD: return _parent._enterGetDirectiveByKey(k, VISIBILITY_LOCAL, i);
260+
case VISIBILITY_CHILDREN: return _parent._enterGetDirectiveByKey(k, VISIBILITY_CHILDREN, i);
238261
// SHADOW
239-
case VISIBILITY_COMPONENT_LOCAL: return _parent._getDirectiveByKey(k, VISIBILITY_LOCAL, i);
240-
case VISIBILITY_COMPONENT_DIRECT_CHILD: return _parent._getDirectiveByKey(k, VISIBILITY_DIRECT_CHILD, i);
241-
case VISIBILITY_COMPONENT_CHILDREN: return _parent._getDirectiveByKey(k, VISIBILITY_CHILDREN, i);
262+
case VISIBILITY_COMPONENT_LOCAL: return _parent._enterGetDirectiveByKey(k, VISIBILITY_LOCAL, i);
263+
case VISIBILITY_COMPONENT_DIRECT_CHILD: return _parent._enterGetDirectiveByKey(k, VISIBILITY_DIRECT_CHILD, i);
264+
case VISIBILITY_COMPONENT_CHILDREN: return _parent._enterGetDirectiveByKey(k, VISIBILITY_CHILDREN, i);
242265
default: throw null;
243266
}
244267
}
@@ -275,7 +298,7 @@ class DirectiveInjector implements DirectiveBinder {
275298
}
276299
}
277300

278-
dynamic _new(List<Key> paramKeys, Function fn) {
301+
dynamic _new(Key k, List<Key> paramKeys, Function fn) {
279302
var oldTag = _TAG_GET.makeCurrent();
280303
int size = paramKeys.length;
281304
var obj;
@@ -439,3 +462,26 @@ class DefaultDirectiveInjector extends DirectiveInjector {
439462
}
440463
}
441464
}
465+
466+
// For efficiency we run through the maximum resolving depth and unwind
467+
// instead of setting 'resolving' key per type.
468+
class DiCircularDependencyError extends ResolvingError {
469+
DiCircularDependencyError(key) : super(key);
470+
471+
// strips the cyclical part of the chain.
472+
List<Key> get stripCycle {
473+
List<Key> rkeys = keys.reversed.toList();
474+
for (num i = 0; i < rkeys.length; i++) {
475+
// Skipping 'cycles' of length 1, which represent resolution
476+
// switching injectors and not true cycles.
477+
for (num j = i + 2; j < rkeys.length; j++) {
478+
if (rkeys[i] == rkeys[j]) return rkeys.sublist(0, j + 1);
479+
}
480+
}
481+
return rkeys;
482+
}
483+
484+
String get resolveChain => stripCycle.join(' -> ');
485+
String toString() => "circular dependency (${resolveChain})";
486+
}
487+

‎test/core_dom/directive_injector_spec.dart

+28-2
Original file line numberDiff line numberDiff line change
@@ -17,10 +17,11 @@ void main() {
1717
Scope scope;
1818
Animate animate;
1919

20-
addDirective(Type type, [Visibility visibility]) {
20+
addDirective(Type type, [Visibility visibility, DirectiveInjector targetInjector]) {
21+
if (targetInjector == null) targetInjector = injector;
2122
if (visibility == null) visibility = Visibility.LOCAL;
2223
var reflector = Module.DEFAULT_REFLECTOR;
23-
injector.bindByKey(
24+
targetInjector.bindByKey(
2425
new Key(type),
2526
reflector.factoryFor(type),
2627
reflector.parameterKeysFor(type),
@@ -76,6 +77,28 @@ void main() {
7677
expect(() => injector.get(_TypeA)).toThrow('No provider found for _TypeA');
7778
});
7879

80+
81+
describe('error handling', () {
82+
it('should throw circular dependency error', () {
83+
addDirective(_TypeC0);
84+
addDirective(_TypeC1, Visibility.CHILDREN);
85+
addDirective(_TypeC2, Visibility.CHILDREN);
86+
expect(() => injector.get(_TypeC0)).toThrow(
87+
'circular dependency (_TypeC0 -> _TypeC1 -> _TypeC2 -> _TypeC1)');
88+
});
89+
90+
it('should throw circular dependency error accross injectors', () {
91+
var childInjector =
92+
new DirectiveInjector(injector, appInjector, null, null, null, null, null);
93+
94+
addDirective(_TypeC0, Visibility.LOCAL, childInjector);
95+
addDirective(_TypeC1, Visibility.CHILDREN);
96+
addDirective(_TypeC2, Visibility.CHILDREN);
97+
expect(() => childInjector.get(_TypeC0)).toThrow(
98+
'circular dependency (_TypeC0 -> _TypeC1 -> _TypeC1 -> _TypeC2 -> _TypeC1)');
99+
});
100+
});
101+
79102
describe('Visibility', () {
80103
DirectiveInjector childInjector;
81104
DirectiveInjector leafInjector;
@@ -130,3 +153,6 @@ class _Type8{ final _Type7 type7; _Type8(this.type7); }
130153
class _Type9{ final _Type8 type8; _Type9(this.type8); }
131154
class _TypeA{ final _Type9 type9; _TypeA(this.type9); }
132155

156+
class _TypeC0 {final _TypeC1 t; _TypeC0(this.t);}
157+
class _TypeC1 {final _TypeC2 t; _TypeC1(this.t);}
158+
class _TypeC2 {final _TypeC1 t; _TypeC2(this.t);}

0 commit comments

Comments
 (0)