Skip to content

StackOverflowError in GraphMetricsCollector.computeSealedDepth on a large Java codebase #215

Description

@sergeykad

Summary

htmlReport fails with java.lang.StackOverflowError in GraphMetricsCollector.computeSealedDepth. The codebase is plain Java 21. It has no Kotlin and no sealed types. No report is written.

Environment

  • RefactorFirst 0.10.0 (refactor-first-maven-plugin)
  • Maven 3.9.9, Temurin JDK 21.0.12, Linux
  • Gradle monorepo, run with the dummy POM from the README next to .git

Command

MAVEN_OPTS="-Xmx16g" mvn -B org.hjug.refactorfirst.plugin:refactor-first-maven-plugin:0.10.0:htmlReport -DshowDetails=true

Output

[INFO] Analyzing Cycles
[INFO] JavaSourceFileGraphBuilder: walking N Java files under ...
...
Exception in thread "main" java.lang.StackOverflowError
	at org.hjug.graphbuilder.metrics.GraphMetricsCollector.computeSealedDepth(GraphMetricsCollector.java:306)
	at org.hjug.graphbuilder.metrics.GraphMetricsCollector.computeSealedDepth(GraphMetricsCollector.java:306)
	... (every printed frame is line 306)

-DanalyzeCycles=false does not avoid it. Both branches of SimpleHtmlReport.generateReport
call CycleRanker.generateClassReferencesGraph.

Cause

MetricsVisitorLogic (around line 166) records every implements type as a sealed hierarchy ancestor. The comment describes Kotlin sealed subtypes, but the code runs for Java classes too. So every Java interface implementation ends up in getSealedHierarchyAncestors().

computeSealedDepth then recurses into each ancestor found in classMetrics, and it does not track which classes it has already entered. If the ancestor chain contains a cycle, the recursion never ends. A real Java implements chain is far too short to fill the stack, so a cycle is present in the collected data. It probably comes from type attribution without the full classpath
(the log shows many JavaParsingException and null methodType warnings), or from the same FQN in more than one source root. I could not isolate the classes that form the cycle.

Suggested fix

Track the classes on the current path and stop when one repeats:

     private int computeSealedDepth(ClassMetrics metrics) {
+        return computeSealedDepth(metrics, new HashSet<>());
+    }
+
+    private int computeSealedDepth(ClassMetrics metrics, Set<String> visiting) {
         Set<String> ancestors = metrics.getSealedHierarchyAncestors();
         if (ancestors.isEmpty()) {
             // No sealed hierarchy ancestors: depth 1 if sealed, 0 otherwise
             return metrics.isSealed() ? 1 : 0;
         }
+        if (!visiting.add(metrics.getFullyQualifiedName())) {
+            return 0;
+        }
         // Has sealed hierarchy ancestors: traverse them first
         int maxAncestorDepth = 0;
         boolean hasObservableAncestor = false;
@@
             hasObservableAncestor = true;
-            maxAncestorDepth = Math.max(maxAncestorDepth, computeSealedDepth(ancestor));
+            maxAncestorDepth = Math.max(maxAncestorDepth, computeSealedDepth(ancestor, visiting));
         }
+        visiting.remove(metrics.getFullyQualifiedName());

With this change applied locally, the same run gets past graph metrics and continues into ranking.

Separately, MetricsVisitorLogic could record implements ancestors only for Kotlin sources, as its comment describes. That keeps Java interface implementations out of the sealed hierarchy data.

Activity

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Metadata

Metadata

Assignees

No one assigned

    Labels

    No labels
    No labels

    Type

    No type

    Projects

    No projects

      Milestone

      No milestone

      Relationships

      None yet

      Development

      No branches or pull requests

      Issue actions