Skip to content

Commit 1d8bd23

Browse files
aksOpsclaude
andcommitted
fix(resolver): four robustness fixes from dual-agent (superpowers + codex) brainstorm
Both reviewers independently identified the same four corner-cases in the Phase 4 + 6 wiring; this lands the converged fix list. 1. JavaSymbolResolver — `volatile` on `solver` and `combined` bootstrap() publishes; resolve() and the public accessors read from arbitrary virtual-thread carriers. The JLS Thread Start Rule covers the executor.submit() path but does NOT cover callers that read the public accessors after bootstrap on a different thread. Cheap fence, closes the visibility hole. 2. JavaSymbolResolver.resolve(String) — strict parse-success check JavaParser is permissive and may return a partial CompilationUnit even when the source has parse problems. Resolving against a partial CU silently emits simple-name-only edges and looks like coverage even though resolution is broken. Treat any non-success as EmptyResolved so the graph never carries phantom RESOLVED-tier edges from broken parses. 3. Analyzer.resolveFor — catch StackOverflowError Pathological generic / type-cycle inputs can blow JavaSymbolSolver's recursion stack. Catching the Error keeps the virtual-thread worker alive and degrades that file's resolution to lexical. Other Errors (OOM, ThreadDeath) remain fatal and propagate. 4. JavaSourceRootDiscovery.containsJavaFile — try-with-resources on Files.walk Files.walk holds an open directory stream; without a close, the file descriptor leaks for every plain-layout fallback scan. Cheap fix. mvn test: 3592 tests / 0 failures / 31 skipped (full suite, no regressions). Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
1 parent 60864e3 commit 1d8bd23

3 files changed

Lines changed: 31 additions & 9 deletions

File tree

src/main/java/io/github/randomcodespace/iq/analyzer/Analyzer.java

Lines changed: 9 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -220,6 +220,15 @@ private Resolved resolveFor(DiscoveredFile file, Object parsedAst, String conten
220220
log.debug("resolver {} threw unexpectedly for {}: {}",
221221
resolver.getClass().getSimpleName(), file.path(), e.toString());
222222
return EmptyResolved.INSTANCE;
223+
} catch (StackOverflowError e) {
224+
// Pathological generic / type-cycle inputs can blow JavaSymbolSolver's
225+
// recursion stack. Catching the Error keeps the virtual-thread
226+
// worker alive and the file's resolution simply degrades to lexical.
227+
// Other Errors (OOM, ThreadDeath) are not caught — they're fatal and
228+
// should propagate.
229+
log.warn("resolver {} stack-overflowed for {} — falling back to lexical",
230+
resolver.getClass().getSimpleName(), file.path());
231+
return EmptyResolved.INSTANCE;
223232
}
224233
}
225234

src/main/java/io/github/randomcodespace/iq/intelligence/resolver/java/JavaSourceRootDiscovery.java

Lines changed: 5 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -123,8 +123,11 @@ private static String nameOrEmpty(Path p) {
123123

124124
/** Cheap probe: does the directory tree under {@code root} have any {@code *.java}? */
125125
private static boolean containsJavaFile(Path root) {
126-
try {
127-
return Files.walk(root)
126+
// try-with-resources: Files.walk holds an open directory stream; without
127+
// a close, the file descriptor leaks for every plain-layout fallback
128+
// scan. Cheap fix.
129+
try (java.util.stream.Stream<Path> stream = Files.walk(root)) {
130+
return stream
128131
.filter(p -> !Files.isDirectory(p))
129132
.anyMatch(p -> p.toString().endsWith(".java"));
130133
} catch (IOException e) {

src/main/java/io/github/randomcodespace/iq/intelligence/resolver/java/JavaSymbolResolver.java

Lines changed: 17 additions & 7 deletions
Original file line numberDiff line numberDiff line change
@@ -40,8 +40,15 @@
4040
public class JavaSymbolResolver implements SymbolResolver {
4141

4242
private final JavaSourceRootDiscovery discovery;
43-
private CombinedTypeSolver combined;
44-
private JavaSymbolSolver solver;
43+
// volatile: bootstrap() publishes the solver; resolve() and the public
44+
// accessors read it from arbitrary virtual-thread carriers. Without
45+
// volatile a reader could see a half-initialized JavaSymbolSolver — a
46+
// narrow race that the JLS Thread Start Rule covers for the
47+
// executor.submit() path but does NOT cover for callers that read the
48+
// public accessors after bootstrap on a different thread. The fence is
49+
// cheap; the alternative is a quiet correctness hole.
50+
private volatile CombinedTypeSolver combined;
51+
private volatile JavaSymbolSolver solver;
4552

4653
public JavaSymbolResolver(JavaSourceRootDiscovery discovery) {
4754
this.discovery = discovery;
@@ -96,11 +103,14 @@ public Resolved resolve(DiscoveredFile file, Object parsedAst) {
96103
// symbol solver so resolve()s on the resulting AST work.
97104
ParserConfiguration cfg = new ParserConfiguration().setSymbolResolver(solver);
98105
ParseResult<CompilationUnit> parseResult = new JavaParser(cfg).parse(source);
99-
if (parseResult.getResult().isEmpty()) {
100-
// Unparseable source — return EmptyResolved rather than
101-
// surface a parse exception. Detectors that need the raw
102-
// content already have ctx.content() — symbol resolution
103-
// simply isn't available for files JavaParser can't accept.
106+
// Strict success check: JavaParser is permissive and may hand
107+
// back a partial CompilationUnit even when the source has parse
108+
// problems. Resolving against a partial CU silently emits
109+
// simple-name-only edges and looks like coverage even though
110+
// symbol resolution is broken. Treat any non-success as
111+
// "EmptyResolved, fall back to lexical" so the downstream graph
112+
// never carries phantom RESOLVED-tier edges from broken parses.
113+
if (!parseResult.isSuccessful() || parseResult.getResult().isEmpty()) {
104114
return EmptyResolved.INSTANCE;
105115
}
106116
cu = parseResult.getResult().get();

0 commit comments

Comments
 (0)