fix: complete methods for classes loaded by multiple ClassLoaders#3218
fix: complete methods for classes loaded by multiple ClassLoaders#3218DragonFSKY wants to merge 1 commit into
Conversation
61e081f to
5f5b231
Compare
hengyunabc
left a comment
There was a problem hiding this comment.
I found a performance regression and an observable extra class-loading side effect in the new method-completion path.
| completion.complete(Collections.<String>emptyList()); | ||
| return true; | ||
| } | ||
| for (Method method : clazz.getDeclaredMethods()) { |
There was a problem hiding this comment.
Validate class-name ambiguity before reflecting methods
getAllLoadedClasses() already existed here, but changing from the limit=2 search to the unbounded overload means broad/ambiguous patterns now scan and retain every matching class. More importantly, this loop calls getDeclaredMethods() before it knows that every result has the same FQCN.
On HotSpot, getDeclaredMethods() can resolve/load every type appearing in the declared method signatures (parameter, return, and exception types; see JDK-6823296). That makes tab completion capable of loading additional application classes through each matched ClassLoader, and a missing signature dependency can surface as NoClassDefFoundError.
I reproduced this on JDK 21 and 25: loading the target class and calling getName() loaded no signature dependencies, while the first getDeclaredMethods() loaded its parameter/return and declared-exception types. With two different matched class names, this implementation still loaded the first class's signature dependencies before returning an empty completion result; a two-phase check loaded none.
Please first scan the results, retain only same-FQCN matches, and stop as soon as a second distinct class name is found. Only after that validation should it call getDeclaredMethods() for the retained classes. A regression test with an unresolved signature type (and predictable handling of LinkageError) would cover this side effect.
Summary
Root Cause
completeMethodName()treated multiple matched classes as ambiguous and returned no method candidates. When the same FQCN is loaded by multiple ClassLoaders,watch/tracemethod completion could therefore miss available methods.Tests
./mvnw -pl core -Dtest=CompletionUtilsTest testgit diff --checkNotes
Fixes #2943