Description
PathRecordingDependencyVisitor cannot handle dependency graphs that have 3 or more cycles such as below:
gid:a:1 (1) +- gid:b:0 | \- ^1 +- gid:b:1 | \- ^1 \- gid:b:2 \- ^1
It fails with StackOverflowError or OutOfMemoryError. Test case.
Solutions
I came up with three solutions. I pick solution #1 for simplicity.
1. Use "parents" to check the cycle, rather than visited set
This is the simplest. Checking array element member is usually discouraged especially for large data set. The implementation should confirm the overhead of this solution.
2. Use AbstractMapBag/Multiset for visited set
Creating a new class that extends AbstractMapBag and leverages IdentityHashMap. Although this solution would be theoretically more efficient than solution #1, I felt it's overkill to create a class just for this solution.
AbstractMapBag(new IdentityHashMap<DependencyNode, AbstractMapBag.MutableInteger>())
IdentityHashMap<DependencyNode, Integer>() would work as a multiset.
3. Call visitLeave only when visitEnter is true
The cause of this bug is DefaultDependencyNode calling visitLeave regardless of visitEnter result.
I'm not sure how many other visitors rely on visitLeave being called regardless of visitEnter result.
Illustration on why existing algorithm does not catch cycle
The following illustration is the node traversal for the test case above by current algorithm. This illustration tracks the dependency node graph and the "visited" set maintained by the visitor.
- visited set. An internal data structure in PathRecordingDependencyVisitor to avoid cycle (link).
- visitEnter(node): PathRecordingDependencyVisitor's function (link). When returning true, the node's children is traversed by the algorithm. This function adds the node to visited set.
- visitLeave(node): PathRecordingDependencyVisitor's function (link). This function removes the node from visited set.
The initial state starts with node "a" and visited set {a}.
First child of a is b0. Because visited does not contain, visitEnter(b0) returns true, meaning that the algorithm traverses this b0's children next. B0 is added to visited.
B0's children is "a". Because visited set contains "a", visitEnter(a) returns false. This means that the algorithm does not traverse this "a"'s children. A is added to visited set (already it has).
Now not traversing this "a"'s children, the algorithm calls visitLeave(a). This removes "a" from visited set.
B0's children are all traversed. the algorithm calls visitLeave(b0). This removes "b0" from visited set.
Now visited set is empty.
Next child of the root "a" is b1. B1 is not in visited set, thus visitEnter(b1) returns true. This means the algorithm traverses the children of this b1.
B1's only child is a. "a" is not in visited set. visitEnter(a) returns true. This means to traverse "a"'s children.
A's first children is b0. b0 is not in visited set. visitEnter(b0) returns true, meaning to traverse children of this b0.
(img 0242)
The only child of b0 is "a". Visited set contains "a", and thus not traversing its children.
visitLeave(a) removes "a" from visited set.
b0's children is all traversed. VisitLeave(b0) removes b0 from visited set.
Next child of this "a" is b1. B1 is in visited set, and thus visitEnter(b1) returns false. This node's children is not to be traversed.
(img 0255)
visitLeave(b1) removes b1 from visited set. Now visited is emtpy.
The last child of "a" is b2. VisitEnter(b2) returns true. It's children is to be traversed. B2 is in visited set.
B2's only child is "a". "a" is not in visited set, thus visitEnter(a) returns true. The algorithm traverses this "a"'s children.
(img 0258)
(...omit...)
IMG_0266 shows the step where I decided to give up. The algorithm does not seem to stop. Indeed the test shows that. The path from the root to the furthest a includes 5 "a" nodes. I concluded the visited set is not working as expected to avoid cycle.
Attachments
Attachments
Issue Links
- is depended upon by
-
MRESOLVER-38 SOE/OOME in DefaultDependencyNode.accept
-
- Closed
-
- is related to
-
-
- Closed
-
- links to
