diff --git a/defects/liquibase/patch/liquibase-0001-dependencygraph-recursive-size-depth-diamond.md b/defects/liquibase/patch/liquibase-0001-dependencygraph-recursive-size-depth-diamond.md index 4519cf9de..56e3dd1eb 100644 --- a/defects/liquibase/patch/liquibase-0001-dependencygraph-recursive-size-depth-diamond.md +++ b/defects/liquibase/patch/liquibase-0001-dependencygraph-recursive-size-depth-diamond.md @@ -1,5 +1,4 @@ # UNDF: UNDF-2026-000000578 -# UNDF: (pending) # liquibase-0001: DependencyUtil.DependencyGraph.recursiveSizeDepth — O(2^D) diamond re-traversal + O(N²) evaluatedNodes list scan ## CWE-407 — Algorithmic Complexity: O(2^D) recursive diamond re-traversal; O(N²) list scan in evaluated-node guard @@ -40,7 +39,7 @@ private int recursiveSizeDepth(GraphNode node, int safetyCounter) { int sum = 0; safetyCounter++; for (GraphNode n : node.getGoingOutNodes()) { - int depth = recursiveSizeDepth(n, safetyCounter); // recurse — no current-path guard + int depth = recursiveSizeDepth(n, safetyCounter); // recurse — no visited guard if (depth < 0) return -1; sum += depth; } @@ -54,9 +53,10 @@ private boolean isAlreadyEvaluated(GraphNode node) { Two distinct defects: -1. **Diamond re-traversal:** `recursiveSizeDepth` has no guard for nodes currently being - traversed in the recursion stack. On a diamond graph it visits shared nodes 2^D times. - Also causes incorrect depth estimates (double-counts shared nodes). +1. **Diamond re-traversal:** `recursiveSizeDepth` has no visited set. On a diamond graph + it visits shared nodes 2^D times. The existing `safetyCounter > 1000` guard is an + emergency brake, not a fix — it truncates the estimate incorrectly and fires only + when recursion nesting exceeds 1000, not when nodes are revisited. 2. **O(N) evaluatedNodes scan:** `evaluatedNodes` is an `ArrayList`. Each call to `isAlreadyEvaluated` or `areAlreadyEvaluated` is an O(N) scan. @@ -65,32 +65,44 @@ Two distinct defects: ## Fix Replace `evaluatedNodes: List>` with a `Set>` for O(1) membership, -and add a `Set> currentPath` parameter to `recursiveSizeDepth` to guard against -diamond re-traversal: +and add a persistent `Set> seen` parameter to `recursiveSizeDepth`. The `seen` +set is **not** cleared on backtrack — it accumulates all visited nodes across the entire +call tree, preventing any node from being counted twice regardless of how many paths lead +to it (diamond, fan-out, or any DAG shape). ```java // AFTER — O(N+E) total private final Set> evaluatedNodes = new LinkedHashSet<>(); // O(1) contains() -private int recursiveSizeDepth(GraphNode node, int safetyCounter, - Set> currentPath) { - if (safetyCounter > 1000) { return -1; } - if (evaluatedNodes.contains(node)) { return 0; } // O(1) - if (!currentPath.add(node)) { return 0; } // diamond guard: O(1), prevents 2^D - try { - if (node.getGoingOutNodes() == null || node.getGoingOutNodes().isEmpty()) { - return 1; - } - int sum = 0; - for (GraphNode n : node.getGoingOutNodes()) { - int depth = recursiveSizeDepth(n, safetyCounter + 1, currentPath); - if (depth < 0) return -1; - sum += depth; - } - return node.getGoingOutNodes().size() + sum; - } finally { - currentPath.remove(node); +// Public entry point: allocate seen set once per depth-check call +private int recursiveSizeDepth(List> nodes) { + if (nodes == null) return 0; + Set> seen = new HashSet<>(); + int sum = 0; + for (GraphNode node : nodes) { + int depth = recursiveSizeDepth(node, 0, seen); + if (depth < 0) return -1; + sum += depth; } + return sum; +} + +private int recursiveSizeDepth(GraphNode node, int safetyCounter, + Set> seen) { + if (safetyCounter > 1000) { return -1; } + if (evaluatedNodes.contains(node)) { return 0; } // O(1) — already emitted + if (!seen.add(node)) { return 0; } // O(1) — already counted this call + // NOTE: do NOT remove from `seen` on return — persistent across all branches + if (node.getGoingOutNodes() == null || node.getGoingOutNodes().isEmpty()) { + return 1; + } + int sum = 0; + for (GraphNode n : node.getGoingOutNodes()) { + int depth = recursiveSizeDepth(n, safetyCounter + 1, seen); + if (depth < 0) return -1; + sum += depth; + } + return node.getGoingOutNodes().size() + sum; } ``` @@ -105,8 +117,10 @@ private boolean areAlreadyEvaluated(List> nodes) { | Diamond depth (D) | Before (visits) | After (visits) | Speedup | |------------------|----------------|----------------|---------| -| 10 | 1,023 | 10 | 102× | -| 15 | 32,767 | 15 | 2,184× | -| 20 | 1,048,575 | 20 | 52,428× | +| 10 | 4,093 | 41 | 100× | +| 12 | 16,381 | 49 | 334× | +| 15 | 131,069 | 61 | 2,148× | -Growth before: O(2^D). Growth after: O(D). +Measured by unit test: `defects/liquibase/unit/unit/test_liquibase_0001.py` + +Growth before: O(2^D). Growth after: O(D+nodes) linear. diff --git a/defects/liquibase/unit/unit/test_liquibase_0001.py b/defects/liquibase/unit/unit/test_liquibase_0001.py new file mode 100644 index 000000000..d489030d6 --- /dev/null +++ b/defects/liquibase/unit/unit/test_liquibase_0001.py @@ -0,0 +1,281 @@ +""" +Unit test for liquibase-0001: + DependencyUtil.DependencyGraph.recursiveSizeDepth — O(2^D) diamond re-traversal + + O(N²) evaluatedNodes ArrayList scan. + +Two defects modelled here: +1. recursiveSizeDepth recurses over goingOutNodes without any visited set, + so chained diamond graphs are traversed O(2^D) times (exponential blowup). +2. isAlreadyEvaluated calls evaluatedNodes.contains() on an ArrayList — O(N) scan. + +Python reimplements the BEFORE and AFTER logic faithfully. +The fix for the diamond blowup is a persistent `seen` set (not removed on backtrack) +passed through the entire recursiveSizeDepth call tree. +""" +import time + + +# ---- BEFORE: defective implementation (mirrors DependencyUtil.java) ---- + +class GraphNodeBefore: + def __init__(self, value): + self.value = value + self.coming_in = [] + self.going_out = [] + + def add_going_out(self, node): + self.going_out.append(node) + + def add_coming_in(self, node): + self.coming_in.append(node) + + +class DependencyGraphBefore: + """Mirrors DependencyUtil.DependencyGraph with the defects intact.""" + + def __init__(self): + self.nodes = {} + self.evaluated_nodes = [] # ArrayList — O(N) contains() + self.total_visits = 0 # instrumentation + + def add(self, eval_first, eval_after): + if eval_first not in self.nodes: + self.nodes[eval_first] = GraphNodeBefore(eval_first) + if eval_after not in self.nodes: + self.nodes[eval_after] = GraphNodeBefore(eval_after) + first = self.nodes[eval_first] + after = self.nodes[eval_after] + first.add_going_out(after) + after.add_coming_in(first) + + def _is_already_evaluated(self, node): + return node in self.evaluated_nodes # O(N) list scan + + def _are_already_evaluated(self, nodes): + return all(self._is_already_evaluated(n) for n in nodes) + + def _recursive_size_depth(self, node, safety_counter): + self.total_visits += 1 + if safety_counter > 1000: + return -1 + if self._is_already_evaluated(node): # O(N), but misses unevaluated diamonds + return 0 + if not node.going_out: + return 1 + total = 0 + safety_counter += 1 + for n in node.going_out: + depth = self._recursive_size_depth(n, safety_counter) # no diamond guard + if depth < 0: + return -1 + total += depth + return len(node.going_out) + total + + +# ---- AFTER: fixed implementation ---- + +class GraphNodeAfter: + def __init__(self, value): + self.value = value + self.coming_in = [] + self.going_out = [] + + def add_going_out(self, node): + self.going_out.append(node) + + def add_coming_in(self, node): + self.coming_in.append(node) + + +class DependencyGraphAfter: + """Fix: + 1. evaluatedNodes is a set — O(1) contains(). + 2. recursiveSizeDepth takes a persistent `seen` set shared across the entire + call tree — each node is visited at most once regardless of diamond structure. + """ + + def __init__(self): + self.nodes = {} + self.evaluated_nodes = set() # O(1) contains() + self.total_visits = 0 + + def add(self, eval_first, eval_after): + if eval_first not in self.nodes: + self.nodes[eval_first] = GraphNodeAfter(eval_first) + if eval_after not in self.nodes: + self.nodes[eval_after] = GraphNodeAfter(eval_after) + first = self.nodes[eval_first] + after = self.nodes[eval_after] + first.add_going_out(after) + after.add_coming_in(first) + + def _is_already_evaluated(self, node): + return node in self.evaluated_nodes # O(1) set lookup + + def _are_already_evaluated(self, nodes): + return all(self._is_already_evaluated(n) for n in nodes) + + def _recursive_size_depth(self, node, safety_counter, seen): + """ + `seen` is a persistent set for the entire call tree — nodes are never + removed from it, so each shared (diamond) node is visited at most once. + """ + self.total_visits += 1 + if safety_counter > 1000: + return -1 + if node in self.evaluated_nodes: # O(1) — already emitted + return 0 + if node in seen: # O(1) diamond guard — already counted + return 0 + seen.add(node) + if not node.going_out: + return 1 + total = 0 + for n in node.going_out: + depth = self._recursive_size_depth(n, safety_counter + 1, seen) + if depth < 0: + return -1 + total += depth + return len(node.going_out) + total + + +# ---- Graph builders ---- + +def make_chained_diamond_graph(GraphClass, depth): + """ + Build a chain of `depth` diamonds using the provided graph class. + Structure (depth=2): + root -> left_0, right_0 + left_0, right_0 -> shared_0 + shared_0 -> left_1, right_1 + left_1, right_1 -> shared_1 + + At depth D, BEFORE visits O(2^D) nodes; AFTER visits O(4*D) nodes. + """ + g = GraphClass() + g.add("root", "left_0") + g.add("root", "right_0") + g.add("left_0", "shared_0") + g.add("right_0", "shared_0") + for i in range(1, depth): + g.add(f"shared_{i-1}", f"left_{i}") + g.add(f"shared_{i-1}", f"right_{i}") + g.add(f"left_{i}", f"shared_{i}") + g.add(f"right_{i}", f"shared_{i}") + return g + + +# ---- Tests ---- + +def test_chained_diamond_before_blowup(): + """ + BEFORE: chained diamonds cause superlinear (exponential) node visits. + At D=10, BEFORE visits far more nodes than AFTER. + """ + D = 10 + g = make_chained_diamond_graph(DependencyGraphBefore, D) + root = g.nodes["root"] + g.total_visits = 0 + g._recursive_size_depth(root, 0) + visits = g.total_visits + # There are only 4*D+1 unique nodes; BEFORE visits O(2^D) due to diamond re-traversal + unique_nodes = 4 * D + 1 + print(f" BEFORE D={D}: {visits} visits for {unique_nodes} unique nodes") + assert visits > unique_nodes * 10, ( + f"Expected BEFORE to visit >10x unique node count ({unique_nodes}), got {visits}" + ) + + +def test_chained_diamond_after_linear(): + """ + AFTER: each node visited at most once — linear in graph size. + """ + D = 10 + g = make_chained_diamond_graph(DependencyGraphAfter, D) + root = g.nodes["root"] + g.total_visits = 0 + g._recursive_size_depth(root, 0, set()) + visits = g.total_visits + unique_nodes = 4 * D + 1 + print(f" AFTER D={D}: {visits} visits for {unique_nodes} unique nodes") + # Each unique node visited at most once + assert visits <= unique_nodes + 5, ( + f"Expected AFTER to visit ~{unique_nodes} unique nodes, got {visits}" + ) + + +def test_exponential_blowup_ratio(): + """ + BEFORE/AFTER visit count ratio grows exponentially with D. + At D=12 the ratio should be >= 100x. + """ + D = 12 + g_before = make_chained_diamond_graph(DependencyGraphBefore, D) + g_after = make_chained_diamond_graph(DependencyGraphAfter, D) + + g_before.total_visits = 0 + root_before = g_before.nodes["root"] + g_before._recursive_size_depth(root_before, 0) + visits_before = g_before.total_visits + + g_after.total_visits = 0 + root_after = g_after.nodes["root"] + g_after._recursive_size_depth(root_after, 0, set()) + visits_after = g_after.total_visits + + ratio = visits_before / max(visits_after, 1) + print(f" D={D}: BEFORE={visits_before} visits, AFTER={visits_after} visits, ratio={ratio:.0f}x") + + assert ratio >= 100, f"Expected >=100x ratio at D={D}, got {ratio:.1f}x" + assert visits_after <= 4 * D + 10, ( + f"Expected AFTER <=linear at D={D}, got {visits_after}" + ) + + +def test_performance_isAlreadyEvaluated_list_vs_set(): + """ + AFTER (set) is significantly faster for isAlreadyEvaluated than BEFORE (list). + Build N evaluated nodes and measure time for a contains() check on the last element. + """ + N = 500 + + g_before = DependencyGraphBefore() + g_after = DependencyGraphAfter() + + nodes_before = [GraphNodeBefore(i) for i in range(N)] + nodes_after = [GraphNodeAfter(i) for i in range(N)] + + g_before.evaluated_nodes = list(nodes_before) # ArrayList + g_after.evaluated_nodes = set(nodes_after) # HashSet + + RUNS = 5000 + target_before = nodes_before[-1] + target_after = nodes_after[-1] + + t0 = time.perf_counter() + for _ in range(RUNS): + g_before._is_already_evaluated(target_before) + t_before = (time.perf_counter() - t0) / RUNS * 1e6 # microseconds + + t1 = time.perf_counter() + for _ in range(RUNS): + g_after._is_already_evaluated(target_after) + t_after = (time.perf_counter() - t1) / RUNS * 1e6 + + ratio = t_before / max(t_after, 1e-9) + print(f" N={N}: BEFORE={t_before:.2f}us, AFTER={t_after:.2f}us, ratio={ratio:.1f}x") + assert ratio >= 5.0, f"Expected >=5x speedup at N={N}, got {ratio:.1f}x" + + +if __name__ == "__main__": + tests = [ + test_chained_diamond_before_blowup, + test_chained_diamond_after_linear, + test_exponential_blowup_ratio, + test_performance_isAlreadyEvaluated_list_vs_set, + ] + for t in tests: + print(f"=== {t.__name__} ===") + t() + print(" PASS") + print("\nAll tests passed.")