Skip to content

Commit

Permalink
Fix OOM with somepath due to infinite loop.
Browse files Browse the repository at this point in the history
This only happens when a `from` target is in the cycle, and the infinite loop occurs when we traverse a parent-edge loop. If the `from` target is not in the cycle, a parent-edge loop is never created because the parent-edges are already set.

**Fix:** Prevent parent-edges from being created from `from` targets to another `from` target, the idea here is that parents of a `from` target are not relevant to `somepath`.

**Implications:** In the case where `from` targets are dependencies of each other, the path going through other `from` targets will not be considered since all `from` targets are treated as "root" level targets. For example:

`somepath(//foo:a + //foo:transitive_dep_of_a, //foo:transitive_dep_of_both)` where this code will now only output the path from `//foo:transitive_dep_of_a -> //foo:transitive_dep_of_both` and never `//foo:a -> //foo:transitive_dep_of_a -> //foo:transitive_dep_of_both`.

PiperOrigin-RevId: 447410138
  • Loading branch information
zhengwei143 authored and copybara-github committed May 9, 2022
1 parent 612e0cf commit 7eda21f
Show file tree
Hide file tree
Showing 2 changed files with 30 additions and 2 deletions.
Original file line number Diff line number Diff line change
Expand Up @@ -75,11 +75,10 @@ public Iterable<Target> somePath(
if (visitor.hasVisited(t)) {
ArrayDeque<Target> result = new ArrayDeque<>();
Target at = t;
// TODO(ulfjack): This can result in an infinite loop if there's a dependency cycle.
while (true) {
result.addFirst(at);
List<Target> pred = visitor.getParents(at);
if (pred == null) {
if (pred == null || pred.isEmpty()) {
break;
}
at = pred.get(0);
Expand Down Expand Up @@ -279,6 +278,11 @@ private void visit(Target from, Attribute attribute, Target target)
}

visitAspectsIfRequired(from, attribute, target);
} else if (mode == VisitorMode.SOMEPATH) {
// Here we make sure that if this is a top-level visitation node (where 'from' is null),
// a parent edge cannot be made for this node. This prevents parent-edge cycles from being
// formed and hence infinite loops impossible when traversing parent-edges.
parentMap.putIfAbsent(target, ImmutableList.of());
}

if (visited.add(target)) {
Expand Down
24 changes: 24 additions & 0 deletions src/test/shell/integration/bazel_query_test.sh
Original file line number Diff line number Diff line change
Expand Up @@ -747,6 +747,30 @@ EOF
"$(cat foo/expected_lexicographical_result)" "$(cat bazel-bin/foo/q)"
}

function test_graphless_query_resilient_to_cycles() {
rm -rf foo
mkdir -p foo
cat > foo/BUILD <<EOF
sh_library(name = "a", deps = [":b"])
sh_library(name = "b", deps = [":c"])
sh_library(name = "c", deps = [":a"])
sh_library(name = "d")
EOF

for command in \
"somepath(//foo:a, //foo:c)" \
"somepath(//foo:a, //foo:d)" \
"somepath(//foo:c, //foo:d)" \
"allpaths(//foo:a, //foo:d)" \
"deps(//foo:a)" \
"rdeps(//foo:a, //foo:d)" \
"same_pkg_direct_rdeps(//foo:b)"
do
bazel query --experimental_graphless_query=true \
"$command" || fail "Expected success"
done
}

function test_lexicographical_output_does_not_affect_order_output_no() {
rm -rf foo
mkdir -p foo
Expand Down

0 comments on commit 7eda21f

Please sign in to comment.