From 70015332ab20238737226c4a5f8b5761fdf93de6 Mon Sep 17 00:00:00 2001 From: Eirik Bakke Date: Wed, 8 Apr 2026 09:12:21 -0400 Subject: [PATCH 1/4] Fix missing "Go to Symbol" results in Java projects. For the past years, the "Go to Symbol" feature has appeared broken for me; searching for the name of a class that exists would frequently turn up no results. Today I managed to reproduce the problem. I put Claude Opus 4.6 on the task of investigating the issue, and had it produce both a fix and a regression test. The problem was that ".sig" files were written to the cache using a .class format with with various NetBeans enhancements, but then loaded again in a way that failed when these enhancements were encountered. Only classes with certain contents would be affected, though here is a rather trivial example class that couldn't be found with "Go to Symbol" prior to the fix in this commit: public class DifficultClass19 { private static final String SOMECONST_STR = "hello"; public void someMethod() { } } (If you try searching for this class in Go to Symbol, you may still get one hit in lowercase "difficultclass19". That's from the text-based Lucene index, which is used as a fallback when no Java symbols appear.) The fix is in AsyncJavaSymbolDescriptor. I have confirmed manually that the new JavaSymbolProviderTest fails without the fix in AsyncJavaSymbolDescriptor, and passes after the fix has been applied. An unrelated potential race condition in Go to Symbol's ContentProviderImpl is also fixed. (The commit description above was written by me, Eirik. The code in this commit is by Claude Opus 4.6, written under my supervision. See the PR for Claude's summary of the investigation.) Co-Authored-By: Claude Opus 4.6 (1M context) --- .../jumpto/symbol/ContentProviderImpl.java | 15 +- .../source/ui/AsyncJavaSymbolDescriptor.java | 280 ++++----------- .../source/ui/JavaSymbolProviderTest.java | 339 ++++++++++++++++++ 3 files changed, 423 insertions(+), 211 deletions(-) create mode 100644 java/java.sourceui/test/unit/src/org/netbeans/modules/java/source/ui/JavaSymbolProviderTest.java diff --git a/ide/jumpto/src/org/netbeans/modules/jumpto/symbol/ContentProviderImpl.java b/ide/jumpto/src/org/netbeans/modules/jumpto/symbol/ContentProviderImpl.java index a01b32ad6557..8f56ad3e51e5 100644 --- a/ide/jumpto/src/org/netbeans/modules/jumpto/symbol/ContentProviderImpl.java +++ b/ide/jumpto/src/org/netbeans/modules/jumpto/symbol/ContentProviderImpl.java @@ -391,8 +391,6 @@ public void run() { if (done || resultChanged) { lastSize = newSize[0]; lastProvCount = newProvCount; - model.remove(toRemove); - model.add(toAdd); attrCopier.checkWrongCase(toRemove, toAdd); if ( isCanceled ) { LOG.log( @@ -405,6 +403,17 @@ public void run() { Level.FINE, "Worker for text {0} finished after {1} ms.", //NOI18N new Object[]{text, System.currentTimeMillis() - createTime}); + /* NB: model.remove and model.add must happen inside + the invokeLater runnable, guarded by !isCanceled, + so that + (a) the remove and add are a single EDT turn (no + intervening repaint/keystroke between them), + and + (b) a Worker that was cancelled after + getSymbolNames cannot mutate the live model + one final time. The cancel flag is set from + the EDT in setListModel, so checking it on + the EDT here is race-free. */ SwingUtilities.invokeLater(() -> { if (done) { final Pair nameAndScope = Utils.splitNameAndScope(text); @@ -418,6 +427,8 @@ public void run() { nameAndScope.second()); } if (!isCanceled) { + model.remove(toRemove); + model.add(toAdd); enableOK(panel.setModel(model, done)); } }); diff --git a/java/java.sourceui/src/org/netbeans/modules/java/source/ui/AsyncJavaSymbolDescriptor.java b/java/java.sourceui/src/org/netbeans/modules/java/source/ui/AsyncJavaSymbolDescriptor.java index 2254f7a47850..7a041922806d 100644 --- a/java/java.sourceui/src/org/netbeans/modules/java/source/ui/AsyncJavaSymbolDescriptor.java +++ b/java/java.sourceui/src/org/netbeans/modules/java/source/ui/AsyncJavaSymbolDescriptor.java @@ -19,59 +19,37 @@ package org.netbeans.modules.java.source.ui; -import com.sun.tools.javac.api.ClientCodeWrapper; import com.sun.tools.javac.api.JavacTaskImpl; -import com.sun.tools.javac.api.JavacTool; -import com.sun.tools.javac.code.Symtab; -import com.sun.tools.javac.jvm.ClassReader; -import com.sun.tools.javac.model.JavacElements; -import java.io.File; -import java.io.IOException; -import java.lang.ref.Reference; -import java.lang.ref.WeakReference; -import java.lang.reflect.Field; import java.util.ArrayList; -import java.util.Arrays; import java.util.Collection; import java.util.Collections; -import java.util.HashSet; -import java.util.Iterator; import java.util.List; -import java.util.Map; import java.util.Set; import java.util.concurrent.CopyOnWriteArrayList; import java.util.concurrent.atomic.AtomicBoolean; -import java.util.logging.Logger; import javax.lang.model.element.Element; import javax.lang.model.element.ElementKind; import javax.lang.model.element.Modifier; import javax.lang.model.element.TypeElement; import javax.swing.Icon; -import javax.tools.Diagnostic; -import javax.tools.DiagnosticListener; -import javax.tools.JavaFileManager; import javax.tools.JavaFileObject; -import javax.tools.StandardLocation; import org.netbeans.api.annotations.common.NonNull; import org.netbeans.api.annotations.common.NullAllowed; -import org.netbeans.api.java.queries.SourceLevelQuery; +import org.netbeans.api.java.source.ClasspathInfo; import org.netbeans.api.java.source.ElementHandle; +import org.netbeans.api.java.source.ui.ElementOpen; import org.netbeans.api.project.ProjectInformation; import org.netbeans.modules.java.source.ElementHandleAccessor; import org.netbeans.modules.java.source.ElementUtils; -import org.netbeans.modules.java.source.indexing.JavaIndex; -import org.netbeans.modules.java.source.parsing.CachingArchiveProvider; -import org.netbeans.modules.java.source.parsing.CachingFileManager; +import org.netbeans.modules.java.source.parsing.JavacParser; import org.netbeans.modules.java.source.usages.ClassIndexImpl; -import org.netbeans.spi.java.classpath.support.ClassPathSupport; import org.netbeans.spi.jumpto.support.AsyncDescriptor; import org.netbeans.spi.jumpto.support.DescriptorChangeEvent; import org.netbeans.spi.jumpto.support.DescriptorChangeListener; import org.netbeans.spi.jumpto.symbol.SymbolDescriptor; import org.openide.filesystems.FileObject; -import org.openide.util.BaseUtilities; import org.openide.util.Exceptions; import org.openide.util.Pair; import org.openide.util.Parameters; @@ -85,11 +63,6 @@ final class AsyncJavaSymbolDescriptor extends JavaSymbolDescriptorBase implement private static final RequestProcessor WORKER = new RequestProcessor(AsyncJavaSymbolDescriptor.class); private static final String INIT = ""; //NOI18N - private static final Logger LOG = Logger.getLogger(AsyncJavaSymbolDescriptor.class.getName()); - private static volatile boolean pkgROELogged = false; - private static volatile boolean clzROELogged = false; - - private static Reference javacRef; private final String ident; private final boolean caseSensitive; @@ -131,6 +104,19 @@ public String getSimpleName() { @Override public void open() { final Collection symbols = resolve(); + /* resolve() falls back to Collections.singleton(this) if it could + not enrich the descriptor (e.g. javac threw CompletionFailure + for some transient reason). Detect that case explicitly to + avoid recursing into our own open(); fall back to opening the + type by its ElementHandle via ElementOpen, which goes through + ClassIndex/SourceUtils. */ + if (symbols.size() == 1 && symbols.iterator().next() == this) { + final FileObject file = getFileObject(); + if (file != null) { + ElementOpen.open(ClasspathInfo.create(file), getOwner()); + } + return; + } if (!symbols.isEmpty()) { symbols.iterator().next().open(); } @@ -197,26 +183,40 @@ private void fireDescriptorChange(Collection replace @NonNull private Collection resolve() { + final List symbols = new ArrayList<>(); try { - final List symbols = new ArrayList<>(); - final JavacTaskImpl jt = getJavac(getRoot()); -// final JavaFileManager fm = jt.getContext().get(JavaFileManager.class); -// final ClassReader cr = ClassReader.instance(jt.getContext()); -// final Names names = Names.instance(jt.getContext()); -// final String binName = ElementHandleAccessor.getInstance().getJVMSignature(getOwner())[0]; -// final Name jcName = names.fromString(binName); -// final Symbol.ClassSymbol te = cr.enterClass((com.sun.tools.javac.util.Name) jcName); -// te.owner.completer = null; -// te.classfile = fm.getJavaFileForInput( -// StandardLocation.CLASS_PATH, -// FileObjects.convertFolder2Package(binName), -// JavaFileObject.Kind.CLASS); - final Symtab syms = Symtab.instance(jt.getContext()); - final Set pkgs = new HashSet<>(getPackages(syms).keySet()); - final Set clzs = new HashSet<>(getClasses(syms).keySet()); - jt.getElements().getTypeElement("java.lang.Object"); // Ensure proper javac initialization - final TypeElement te = ElementUtils.getTypeElementByBinaryName(jt, - ElementHandleAccessor.getInstance().getJVMSignature(getOwner())[0]); + final String binName = ElementHandleAccessor.getInstance().getJVMSignature(getOwner())[0]; + /* Build the JavacTask via JavacParser.createJavacTask + (which pre-registers NetBeans' javac context enhancers -- + most importantly NBClassReader, plus NBAttr, NBEnter, + NBMemberEnter, NBResolve, NBClassFinder, NBJavaCompiler, + NBClassWriter, etc.) instead of calling + JavacTool.create().getTask(...) directly. NetBeans .sig + files (the per-root cached form of indexed Java classes) + are written by NBClassWriter and use a relaxed/extended + class-file format that only NBClassReader knows how to + read; stock javac's ClassReader rejects them with + BadClassFile, which propagates as CompletionFailure. + ElementUtils.getTypeElementByBinaryName silently catches + the CompletionFailure and returns null, and resolve() + therefore returned an empty collection for every Java + type whose .sig had any NB-specific encoding -- the + actual cause of the long-standing Go to Symbol + disappearance bug. */ + final ClasspathInfo cpInfo = ClasspathInfo.create(getRoot()); + final JavacTaskImpl jt = JavacParser.createJavacTask( + cpInfo, + null, + null, + null, + null, + null, + null, + null, + Collections.emptyList()); + //Force JTImpl.prepareCompiler to get JTImpl into Context + jt.enter(); + final TypeElement te = ElementUtils.getTypeElementByBinaryName(jt, binName); if (te != null) { if (ident.equals(getSimpleName(te, null, caseSensitive))) { final String simpleName = te.getSimpleName().toString(); @@ -252,69 +252,31 @@ private Collection resolve() { } } } - getClasses(syms).keySet().retainAll(clzs); - getPackages(syms).keySet().retainAll(pkgs); - return symbols; - } catch (IOException e) { + } catch (RuntimeException e) { + /* Swallow so that unexpected javac failures (e.g. + CompletionFailure, which is a RuntimeException) fall through + to the "no enriched symbols" path below rather than + propagating into the WORKER thread and leaving the descriptor + in a half-initialized state. Previously this was a catch for + IOException, thrown by the old hand-rolled JavaFileManager + wiring that has now been deleted. */ Exceptions.printStackTrace(e); - return Collections.emptyList(); } - } - - @NonNull - private Map getPackages(final Symtab cr) { - Map res = Collections.emptyMap(); - try { - final Field fld = ClassReader.class.getDeclaredField("packages"); //NOI18N - fld.setAccessible(true); - final Map pkgs = (Map) fld.get(cr); - if (pkgs != null) { - res = pkgs; - } - } catch (ReflectiveOperationException e) { - if (!pkgROELogged) { - LOG.warning(e.getMessage()); - pkgROELogged = true; - } + /* The async path is meant to *enrich* a descriptor that the + (Lucene-backed) JavaSymbolProvider already produced -- not to + delete it. If javac couldn't load the TypeElement (te==null), + no enclosed element matched ident, or the lookup threw, the + entry is still a valid match from the index and must remain + in the list. Returning an empty collection here would cause + Models.MutableListModelImpl#descriptorChanged to remove the + source from the live model, producing the long-standing + "search results appear briefly then disappear" Go to Symbol + symptom. Fall back to keeping the AsyncJavaSymbolDescriptor + itself. */ + if (symbols.isEmpty()) { + return Collections.singleton(this); } - return res; - } - - @NonNull - private Map getClasses(final Symtab cr) { - Map res = Collections.emptyMap(); - try { - final Field fld = ClassReader.class.getDeclaredField("classes"); //NOI18N - fld.setAccessible(true); - Map clzs = (Map) fld.get(cr); - if (clzs != null) { - res = clzs; - } - } catch (ReflectiveOperationException e) { - if (!clzROELogged) { - LOG.warning(e.getMessage()); - clzROELogged = true; - } - } - return res; - } - - private static JavacTaskImpl getJavac(FileObject root) throws IOException { - JavacTaskImpl javac; - Reference ref = javacRef; - if (ref == null || (javac = ref.get()) == null) { - String sourceLevel = SourceLevelQuery.getSourceLevel(root); - javac = (JavacTaskImpl)JavacTool.create().getTask(null, - new RootChange(root), - new Listener(), - sourceLevel != null ? Arrays.asList("-source", sourceLevel) : Collections.emptySet(), //NOI18N - Collections.emptySet(), - Collections.emptySet()); - javacRef = new WeakReference<>(javac); - } - final JavaFileManager fm = javac.getContext().get(JavaFileManager.class); - ((RootChange)fm).setRoot(root); - return javac; + return symbols; } @NonNull @@ -332,104 +294,4 @@ private static String getSimpleName ( return result; } - @ClientCodeWrapper.Trusted - private static final class RootChange implements JavaFileManager { - - private FileObject currentRoot; - private JavaFileManager delegate; - - RootChange(@NonNull final FileObject root) throws IOException { - setRoot(root); - } - - void setRoot(@NonNull final FileObject root) throws IOException { - if (root != currentRoot) { - final File classes = JavaIndex.getClassFolder(root.toURL()); - final CachingFileManager fm = new CachingFileManager( - CachingArchiveProvider.getDefault(), - ClassPathSupport.createClassPath(BaseUtilities.toURI(classes).toURL()), - null, - false, - true); - this.delegate = fm; - this.currentRoot = root; - } - } - - @Override - public ClassLoader getClassLoader(Location location) { - return delegate.getClassLoader(location); - } - - @Override - public Iterable list(Location location, String packageName, Set kinds, boolean recurse) throws IOException { - return delegate.list(location, packageName, kinds, recurse); - } - - @Override - public String inferBinaryName(Location location, JavaFileObject file) { - return delegate.inferBinaryName(location, file); - } - - @Override - public boolean isSameFile(javax.tools.FileObject a, javax.tools.FileObject b) { - return delegate.isSameFile(a, b); - } - - @Override - public boolean handleOption(String current, Iterator remaining) { - return delegate.handleOption(current, remaining); - } - - @Override - public boolean hasLocation(Location location) { - return location == StandardLocation.CLASS_PATH || location == StandardLocation.PLATFORM_CLASS_PATH; - } - - @Override - public JavaFileObject getJavaFileForInput(Location location, String className, JavaFileObject.Kind kind) throws IOException { - return delegate.getJavaFileForInput(location, className, kind); - } - - @Override - public JavaFileObject getJavaFileForOutput(Location location, String className, JavaFileObject.Kind kind, javax.tools.FileObject sibling) throws IOException { - return delegate.getJavaFileForOutput(location, className, kind, sibling); - } - - @Override - public javax.tools.FileObject getFileForInput(Location location, String packageName, String relativeName) throws IOException { - return delegate.getFileForInput(location, packageName, relativeName); - } - - @Override - public javax.tools.FileObject getFileForOutput(Location location, String packageName, String relativeName, javax.tools.FileObject sibling) throws IOException { - return delegate.getFileForOutput(location, packageName, relativeName, sibling); - } - - @Override - public void flush() throws IOException { - delegate.flush(); - } - - @Override - public void close() throws IOException { - delegate.close(); - } - - @Override - public int isSupportedOption(String option) { - return delegate.isSupportedOption(option); - } - - @Override - public Iterable> listLocationsForModules(Location location) throws IOException { - return Collections.emptyList(); - } - } - - private static final class Listener implements DiagnosticListener { - @Override - public void report(Diagnostic diagnostic) { - } - } } diff --git a/java/java.sourceui/test/unit/src/org/netbeans/modules/java/source/ui/JavaSymbolProviderTest.java b/java/java.sourceui/test/unit/src/org/netbeans/modules/java/source/ui/JavaSymbolProviderTest.java new file mode 100644 index 000000000000..a9e9c4b27a49 --- /dev/null +++ b/java/java.sourceui/test/unit/src/org/netbeans/modules/java/source/ui/JavaSymbolProviderTest.java @@ -0,0 +1,339 @@ +/* + * Licensed to the Apache Software Foundation (ASF) under one + * or more contributor license agreements. See the NOTICE file + * distributed with this work for additional information + * regarding copyright ownership. The ASF licenses this file + * to you under the Apache License, Version 2.0 (the + * "License"); you may not use this file except in compliance + * with the License. You may obtain a copy of the License at + * + * http://www.apache.org/licenses/LICENSE-2.0 + * + * Unless required by applicable law or agreed to in writing, + * software distributed under the License is distributed on an + * "AS IS" BASIS, WITHOUT WARRANTIES OR CONDITIONS OF ANY + * KIND, either express or implied. See the License for the + * specific language governing permissions and limitations + * under the License. + */ +package org.netbeans.modules.java.source.ui; + +import java.beans.PropertyChangeListener; +import java.beans.PropertyChangeSupport; +import java.io.File; +import java.io.IOException; +import java.io.OutputStreamWriter; +import java.io.PrintWriter; +import java.util.ArrayList; +import java.util.Collection; +import java.util.Collections; +import java.util.List; +import java.util.concurrent.CountDownLatch; +import java.util.concurrent.TimeUnit; +import java.util.concurrent.atomic.AtomicReference; +import org.netbeans.api.java.classpath.ClassPath; +import org.netbeans.api.java.classpath.GlobalPathRegistry; +import org.netbeans.api.java.platform.JavaPlatformManager; +import org.netbeans.junit.MockServices; +import org.netbeans.junit.NbTestCase; +import org.netbeans.modules.java.source.parsing.FileObjects; +import org.netbeans.modules.java.source.usages.IndexUtil; +import org.netbeans.modules.jumpto.symbol.SymbolProviderAccessor; +import org.netbeans.modules.parsing.api.indexing.IndexingManager; +import org.netbeans.spi.java.classpath.ClassPathFactory; +import org.netbeans.spi.java.classpath.ClassPathImplementation; +import org.netbeans.spi.java.classpath.ClassPathProvider; +import org.netbeans.spi.java.classpath.PathResourceImplementation; +import org.netbeans.spi.java.classpath.support.ClassPathSupport; +import org.netbeans.spi.jumpto.support.AsyncDescriptor; +import org.netbeans.spi.jumpto.support.DescriptorChangeEvent; +import org.netbeans.spi.jumpto.support.DescriptorChangeListener; +import org.netbeans.spi.jumpto.symbol.SymbolDescriptor; +import org.netbeans.spi.jumpto.symbol.SymbolProvider; +import org.netbeans.spi.jumpto.type.SearchType; +import org.openide.filesystems.FileLock; +import org.openide.filesystems.FileObject; +import org.openide.filesystems.FileUtil; + +/* This test was written entirely by Claude Opus 4.6, prompted by Eirik Bakke (ebakke@ultorg.com). +Eirik confirmed that the test failed before the fix to the bug in AsyncJavaSymbolDescriptor, and +passed after the fix was applied. */ + +/** + * Regression test for the long-standing Go to Symbol bug where + * AsyncJavaSymbolDescriptor.resolve() silently returned empty for any + * Java type whose .sig file used a NetBeans-specific class-file + * encoding, causing Lucene-indexed matches to flash briefly in the Go + * to Symbol list and then be deleted by + * Models.MutableListModelImpl#descriptorChanged. Root cause was that + * the old hand-rolled javac construction in resolve() called + * JavacTool.create().getTask(...) directly, which does not pre-register + * NetBeans' javac context enhancers (NBClassReader, NBAttr, NBEnter, + * NBMemberEnter, NBResolve, NBClassFinder, NBJavaCompiler, + * NBClassWriter, etc.). NetBeans .sig files (the per-root cached form + * of indexed Java classes) are written by NBClassWriter and use a + * relaxed/extended class-file format that only NBClassReader knows how + * to read; stock javac's ClassReader rejects them with BadClassFile + * (CompletionFailure), and ElementUtils.getTypeElementByBinaryName + * silently returned null. + * + *

The test creates a single-class source root whose source is + * intentionally rich enough (a lambda, generics, a static-final String + * constant, and a separate annotation type) that the indexer's .sig + * output is empirically rejected by stock javac. The exact attribute + * that trips stock ClassReader for this particular class has not been + * pinned down -- a trivial empty class produces a .sig that vanilla + * javac reads without trouble, so the richer source is required to + * exercise the regression. The test indexes the source, asks + * JavaSymbolProvider to find the class by name, triggers + * AsyncJavaSymbolDescriptor's async resolve() path via getSymbolName(), + * and asserts that the resulting descriptor-change event carries a + * non-empty, enriched replacement (not the original + * AsyncJavaSymbolDescriptor instance). Verified to fail against the + * pre-fix code path and pass against the fix. + */ +public class JavaSymbolProviderTest extends NbTestCase { + + private static FileObject srcRoot; + private static ClassPath sourcePath; + private static ClassPath compilePath; + private static ClassPath bootPath; + private static MutableCp spiCp; + private static MutableCp spiSrc; + + public JavaSymbolProviderTest(String name) { + super(name); + } + + @Override + protected void setUp() throws Exception { + clearWorkDir(); + File cache = new File(getWorkDir(), "cache"); //NOI18N + cache.mkdirs(); + IndexUtil.setCacheFolder(cache); + File src = new File(getWorkDir(), "src"); //NOI18N + src.mkdirs(); + srcRoot = FileUtil.toFileObject(src); + spiSrc = new MutableCp(Collections.singletonList(ClassPathSupport.createResource(srcRoot.getURL()))); + sourcePath = ClassPathFactory.createClassPath(spiSrc); + spiCp = new MutableCp(); + compilePath = ClassPathFactory.createClassPath(spiCp); + bootPath = JavaPlatformManager.getDefault().getDefaultPlatform().getBootstrapLibraries(); + MockServices.setServices(ClassPathProviderImpl.class); + + /* The class is intentionally rich (a lambda, generics, a + static-final String constant, and a separate annotation + type) so that the .sig file the indexer writes is + empirically rejected by stock javac's ClassReader. A trivial + empty class produces a .sig that vanilla javac can read, and + so would not exercise the bug we are regressing against. + The exact attribute that trips stock ClassReader for this + class has not been pinned down -- it is enough to know that + the differential exists. */ + createJavaFile(srcRoot, "org.me.test", "FooBar", + "package org.me.test;\n" + + "import java.lang.annotation.ElementType;\n" + + "import java.lang.annotation.Retention;\n" + + "import java.lang.annotation.RetentionPolicy;\n" + + "import java.lang.annotation.Target;\n" + + "@Retention(RetentionPolicy.SOURCE)\n" + + "@Target({ElementType.TYPE, ElementType.METHOD, ElementType.FIELD})\n" + + "@interface SourceMark {}\n" + + "@SourceMark\n" + + "public class FooBar {\n" + + " public static final String GREETING = \"hello\";\n" + + " @SourceMark public final java.util.function.Supplier SUPPLIER = () -> GREETING;\n" + + " @SourceMark public void doSomething() {}\n" + + "}\n"); + /* JavaSymbolProvider queries GlobalPathRegistry for SOURCE + roots; it does not take a ClasspathInfo the way + JavaTypeProvider does. Register our test source path there + so findRoots(SOURCE, ...) picks it up. */ + GlobalPathRegistry.getDefault().register(ClassPath.SOURCE, new ClassPath[] {sourcePath}); + GlobalPathRegistry.getDefault().register(ClassPath.BOOT, new ClassPath[] {bootPath}); + GlobalPathRegistry.getDefault().register(ClassPath.COMPILE, new ClassPath[] {compilePath}); + IndexingManager.getDefault().refreshIndexAndWait(srcRoot.getURL(), null, true); + } + + @Override + protected void tearDown() throws Exception { + GlobalPathRegistry.getDefault().unregister(ClassPath.SOURCE, new ClassPath[] {sourcePath}); + GlobalPathRegistry.getDefault().unregister(ClassPath.BOOT, new ClassPath[] {bootPath}); + GlobalPathRegistry.getDefault().unregister(ClassPath.COMPILE, new ClassPath[] {compilePath}); + MockServices.setServices(); + } + + /** + * Regression test: asks JavaSymbolProvider for "FooBar", lets the + * returned AsyncJavaSymbolDescriptor fire its async resolve(), and + * asserts the resulting replacement is non-empty and not the + * original async descriptor instance. Prior to the fix, resolve() + * silently returned an empty collection and this assertion failed. + */ + public void testAsyncDescriptorResolvesForIndexedType() throws Exception { + final JavaSymbolProvider provider = new JavaSymbolProvider(); + final List results = new ArrayList<>(); + final String[] message = new String[1]; + final SymbolProvider.Context ctx = SymbolProviderAccessor.DEFAULT + .createContext(null, "FooBar", SearchType.PREFIX); //NOI18N + final SymbolProvider.Result res = SymbolProviderAccessor.DEFAULT + .createResult(results, message, ctx, provider); + provider.computeSymbolNames(ctx, res); + assertFalse("JavaSymbolProvider returned no hits for FooBar; " + + "the index may not have been populated in setUp", results.isEmpty()); + + // Find the AsyncDescriptor corresponding to the class itself. + AsyncDescriptor async = null; + for (SymbolDescriptor d : results) { + if (d instanceof AsyncDescriptor && "FooBar".equals(d.getSimpleName())) { //NOI18N + @SuppressWarnings("unchecked") + final AsyncDescriptor cast = (AsyncDescriptor) d; + async = cast; + break; + } + } + assertNotNull("JavaSymbolProvider did not return an AsyncDescriptor " + + "for class FooBar; results=" + results, async); + final AsyncDescriptor original = async; + + // Subscribe to the descriptor-change event that initialize() -> + // resolve() -> fireDescriptorChange() will fire on the WORKER + // thread once we touch getSymbolName() / getIcon(). + final CountDownLatch latch = new CountDownLatch(1); + final AtomicReference> replacementRef = new AtomicReference<>(); + final DescriptorChangeListener listener = + new DescriptorChangeListener() { + @Override + public void descriptorChanged(DescriptorChangeEvent event) { + replacementRef.set(event.getReplacement()); + latch.countDown(); + } + }; + original.addDescriptorChangeListener(listener); + try { + // Triggers AsyncJavaSymbolDescriptor.initialize() which + // schedules resolve() on the WORKER RequestProcessor. + ((SymbolDescriptor) original).getSymbolName(); + assertTrue("Async resolve() did not complete within 30s.", + latch.await(30, TimeUnit.SECONDS)); + } finally { + original.removeDescriptorChangeListener(listener); + } + + final Collection replacement = replacementRef.get(); + assertNotNull("replacement was null", replacement); + + /* The actual regression assertion. + Before the fix: resolve()'s hand-rolled + JavacTool.create().getTask(...) call had no NB + context enhancers, so javac used the stock + ClassReader, which threw CompletionFailure on + the .sig file (NetBeans .sig files use a relaxed + class-file format that only NBClassReader can + read). ElementUtils.getTypeElementByBinaryName + silently caught that and returned null, so + resolve() returned an empty collection and the + assertion below tripped. + After the fix: resolve() goes through + JavacParser.createJavacTask(ClasspathInfo.create( + root), ...), which pre-registers NBClassReader + and the rest of the NB javac context, so the + .sig file loads correctly and resolve() returns a + ResolvedJavaSymbolDescriptor distinct from the + source AsyncJavaSymbolDescriptor. */ + assertFalse("resolve() returned an empty replacement -- the bug is " + + "still present: AsyncJavaSymbolDescriptor.resolve() is " + + "silently failing to enrich the Lucene-indexed hit. Most " + + "likely cause: the JavacTask was built without " + + "pre-registering NBClassReader, so stock javac could not " + + "parse the per-root .sig file.", + replacement.isEmpty()); + for (SymbolDescriptor d : replacement) { + assertNotSame("resolve() fell back to its singleton(this) " + + "defense-in-depth path instead of actually enriching " + + "the descriptor. This still means javac could not " + + "complete() the type; most likely the JavacTask was " + + "not built with NB context enhancers (NBClassReader, " + + "etc.) and so cannot read NetBeans .sig files.", + original, d); + } + } + + private static FileObject createJavaFile( + final FileObject root, + final String pkg, + final String name, + final String content) throws IOException { + final FileObject file = FileUtil.createData( + root, + String.format("%s/%s.java", + FileObjects.convertPackage2Folder(pkg), + name)); + final FileLock lck = file.lock(); + try { + final PrintWriter out = new PrintWriter(new OutputStreamWriter(file.getOutputStream(lck))); + try { + out.print(content); + } finally { + out.close(); + } + } finally { + lck.releaseLock(); + } + return file; + } + + public static class ClassPathProviderImpl implements ClassPathProvider { + @Override + public ClassPath findClassPath(final FileObject file, final String type) { + final FileObject[] roots = sourcePath.getRoots(); + for (FileObject root : roots) { + if (root.equals(file) || FileUtil.isParentOf(root, file)) { + if (type == ClassPath.SOURCE) { + return sourcePath; + } + if (type == ClassPath.COMPILE) { + return compilePath; + } + if (type == ClassPath.BOOT) { + return bootPath; + } + } + } + return null; + } + } + + private static final class MutableCp implements ClassPathImplementation { + + private final PropertyChangeSupport support; + private List impls; + + MutableCp() { + this(Collections.emptyList()); + } + + MutableCp(final List impls) { + assert impls != null; + support = new PropertyChangeSupport(this); + this.impls = impls; + } + + @Override + public List getResources() { + return impls; + } + + @Override + public void addPropertyChangeListener(final PropertyChangeListener listener) { + assert listener != null; + support.addPropertyChangeListener(listener); + } + + @Override + public void removePropertyChangeListener(final PropertyChangeListener listener) { + assert listener != null; + support.removePropertyChangeListener(listener); + } + } +} From 4161040b6fbb6890ca74ac7c8ce223b501a4981c Mon Sep 17 00:00:00 2001 From: Eirik Bakke Date: Sat, 11 Apr 2026 11:48:54 -0400 Subject: [PATCH 2/4] Revert the last commit's change to ContentProviderImpl; it was not actually needed to fix the motivating bug. --- .../jumpto/symbol/ContentProviderImpl.java | 15 ++------------- 1 file changed, 2 insertions(+), 13 deletions(-) diff --git a/ide/jumpto/src/org/netbeans/modules/jumpto/symbol/ContentProviderImpl.java b/ide/jumpto/src/org/netbeans/modules/jumpto/symbol/ContentProviderImpl.java index 8f56ad3e51e5..a01b32ad6557 100644 --- a/ide/jumpto/src/org/netbeans/modules/jumpto/symbol/ContentProviderImpl.java +++ b/ide/jumpto/src/org/netbeans/modules/jumpto/symbol/ContentProviderImpl.java @@ -391,6 +391,8 @@ public void run() { if (done || resultChanged) { lastSize = newSize[0]; lastProvCount = newProvCount; + model.remove(toRemove); + model.add(toAdd); attrCopier.checkWrongCase(toRemove, toAdd); if ( isCanceled ) { LOG.log( @@ -403,17 +405,6 @@ public void run() { Level.FINE, "Worker for text {0} finished after {1} ms.", //NOI18N new Object[]{text, System.currentTimeMillis() - createTime}); - /* NB: model.remove and model.add must happen inside - the invokeLater runnable, guarded by !isCanceled, - so that - (a) the remove and add are a single EDT turn (no - intervening repaint/keystroke between them), - and - (b) a Worker that was cancelled after - getSymbolNames cannot mutate the live model - one final time. The cancel flag is set from - the EDT in setListModel, so checking it on - the EDT here is race-free. */ SwingUtilities.invokeLater(() -> { if (done) { final Pair nameAndScope = Utils.splitNameAndScope(text); @@ -427,8 +418,6 @@ public void run() { nameAndScope.second()); } if (!isCanceled) { - model.remove(toRemove); - model.add(toAdd); enableOK(panel.setModel(model, done)); } }); From 86c0ee8a0e5b7bf65eae71e026cdb089e2ccd4e6 Mon Sep 17 00:00:00 2001 From: Eirik Bakke Date: Sat, 11 Apr 2026 12:28:14 -0400 Subject: [PATCH 3/4] Some cleanup and comment adjustments in AsyncJavaSymbolDescriptor, after Eirik reviewed Claude's patch. --- .../source/ui/AsyncJavaSymbolDescriptor.java | 85 +++++++------------ 1 file changed, 31 insertions(+), 54 deletions(-) diff --git a/java/java.sourceui/src/org/netbeans/modules/java/source/ui/AsyncJavaSymbolDescriptor.java b/java/java.sourceui/src/org/netbeans/modules/java/source/ui/AsyncJavaSymbolDescriptor.java index 7a041922806d..692f40db3131 100644 --- a/java/java.sourceui/src/org/netbeans/modules/java/source/ui/AsyncJavaSymbolDescriptor.java +++ b/java/java.sourceui/src/org/netbeans/modules/java/source/ui/AsyncJavaSymbolDescriptor.java @@ -104,21 +104,20 @@ public String getSimpleName() { @Override public void open() { final Collection symbols = resolve(); - /* resolve() falls back to Collections.singleton(this) if it could - not enrich the descriptor (e.g. javac threw CompletionFailure - for some transient reason). Detect that case explicitly to - avoid recursing into our own open(); fall back to opening the - type by its ElementHandle via ElementOpen, which goes through - ClassIndex/SourceUtils. */ - if (symbols.size() == 1 && symbols.iterator().next() == this) { + if (symbols.isEmpty()) { + return; + } + SymbolDescriptor sd = symbols.iterator().next(); + if (sd != this) { + sd.open(); + } else { + /* The case where resolve() returns an un-enriched Collections.singleton(this) because + of some error (e.g. javac threw CompletionFailure). Fall back to opening the type by + its ElementHandle via ElementOpen, which goes through ClassIndex/SourceUtils. */ final FileObject file = getFileObject(); if (file != null) { ElementOpen.open(ClasspathInfo.create(file), getOwner()); } - return; - } - if (!symbols.isEmpty()) { - symbols.iterator().next().open(); } } @@ -186,33 +185,17 @@ private Collection resolve() { final List symbols = new ArrayList<>(); try { final String binName = ElementHandleAccessor.getInstance().getJVMSignature(getOwner())[0]; - /* Build the JavacTask via JavacParser.createJavacTask - (which pre-registers NetBeans' javac context enhancers -- - most importantly NBClassReader, plus NBAttr, NBEnter, - NBMemberEnter, NBResolve, NBClassFinder, NBJavaCompiler, - NBClassWriter, etc.) instead of calling - JavacTool.create().getTask(...) directly. NetBeans .sig - files (the per-root cached form of indexed Java classes) - are written by NBClassWriter and use a relaxed/extended - class-file format that only NBClassReader knows how to - read; stock javac's ClassReader rejects them with - BadClassFile, which propagates as CompletionFailure. - ElementUtils.getTypeElementByBinaryName silently catches - the CompletionFailure and returns null, and resolve() - therefore returned an empty collection for every Java - type whose .sig had any NB-specific encoding -- the - actual cause of the long-standing Go to Symbol - disappearance bug. */ + /* Build the JavacTask via JavacParser.createJavacTask, which pre-registers NetBeans' + javac context enhancers, e.g. NBClassReader plus NBAttr, NBEnter, NBMemberEnter, + NBResolve, NBClassFinder, NBJavaCompiler, NBClassWriter, instead of calling + JavacTool.create().getTask(...) directly. + + NetBeans .sig files (the per-root cached form of indexed Java classes) are written by + NBClassWriter and use a relaxed/extended class-file format that only NBClassReader knows + how to read. Stock javac's ClassReader would rejects them with BadClassFile. */ final ClasspathInfo cpInfo = ClasspathInfo.create(getRoot()); final JavacTaskImpl jt = JavacParser.createJavacTask( - cpInfo, - null, - null, - null, - null, - null, - null, - null, + cpInfo, null, null, null, null, null, null, null, Collections.emptyList()); //Force JTImpl.prepareCompiler to get JTImpl into Context jt.enter(); @@ -253,26 +236,20 @@ the CompletionFailure and returns null, and resolve() } } } catch (RuntimeException e) { - /* Swallow so that unexpected javac failures (e.g. - CompletionFailure, which is a RuntimeException) fall through - to the "no enriched symbols" path below rather than - propagating into the WORKER thread and leaving the descriptor - in a half-initialized state. Previously this was a catch for - IOException, thrown by the old hand-rolled JavaFileManager - wiring that has now been deleted. */ + /* Swallow so that unexpected javac failures (e.g. CompletionFailure, which is a + RuntimeException) fall through to the "no enriched symbols" path below rather than + propagating into the WORKER thread and leaving the descriptor in a half-initialized + state. */ Exceptions.printStackTrace(e); } - /* The async path is meant to *enrich* a descriptor that the - (Lucene-backed) JavaSymbolProvider already produced -- not to - delete it. If javac couldn't load the TypeElement (te==null), - no enclosed element matched ident, or the lookup threw, the - entry is still a valid match from the index and must remain - in the list. Returning an empty collection here would cause - Models.MutableListModelImpl#descriptorChanged to remove the - source from the live model, producing the long-standing - "search results appear briefly then disappear" Go to Symbol - symptom. Fall back to keeping the AsyncJavaSymbolDescriptor - itself. */ + /* The async path is meant to *enrich* a descriptor that the (Lucene-backed) + JavaSymbolProvider already produced, not to delete it. If javac couldn't load the + TypeElement (te==null), no enclosed element matched ident, or the lookup threw, the entry is + still a valid match from the index and must remain in the list. Returning an empty + collection here would cause Models.MutableListModelImpl#descriptorChanged to remove the + source from the live model, causing Lucene-based search results to appear briefly then + disappear in the Go to Symbol dialog. Fall back to keeping the AsyncJavaSymbolDescriptor + itself. */ if (symbols.isEmpty()) { return Collections.singleton(this); } From 4281df11f561b3e82798ee6bdcfc16b99a9d91e4 Mon Sep 17 00:00:00 2001 From: Eirik Bakke Date: Mon, 13 Apr 2026 10:19:27 -0400 Subject: [PATCH 4/4] Cache javac instances per FileObject root to avoid recreating on every symbol. --- .../source/ui/AsyncJavaSymbolDescriptor.java | 46 ++++++++++++------- .../java/source/ui/JavaSymbolProvider.java | 9 +++- 2 files changed, 38 insertions(+), 17 deletions(-) diff --git a/java/java.sourceui/src/org/netbeans/modules/java/source/ui/AsyncJavaSymbolDescriptor.java b/java/java.sourceui/src/org/netbeans/modules/java/source/ui/AsyncJavaSymbolDescriptor.java index 692f40db3131..96e7803b3193 100644 --- a/java/java.sourceui/src/org/netbeans/modules/java/source/ui/AsyncJavaSymbolDescriptor.java +++ b/java/java.sourceui/src/org/netbeans/modules/java/source/ui/AsyncJavaSymbolDescriptor.java @@ -26,6 +26,7 @@ import java.util.Collections; import java.util.List; import java.util.Set; +import java.util.concurrent.ConcurrentHashMap; import java.util.concurrent.CopyOnWriteArrayList; import java.util.concurrent.atomic.AtomicBoolean; @@ -63,25 +64,33 @@ final class AsyncJavaSymbolDescriptor extends JavaSymbolDescriptorBase implement private static final RequestProcessor WORKER = new RequestProcessor(AsyncJavaSymbolDescriptor.class); private static final String INIT = ""; //NOI18N + private final ConcurrentHashMap javacCache; private final String ident; private final boolean caseSensitive; private final List> listeners; private final AtomicBoolean initialized; + /** + * @param javacCache a cache map to be shared between instances during a single search, to avoid + * creating a JavacTaskImpl for every symbol + */ AsyncJavaSymbolDescriptor ( @NullAllowed final ProjectInformation projectInformation, @NonNull final FileObject root, @NonNull final ClassIndexImpl ci, @NonNull final ElementHandle owner, @NonNull final String ident, - final boolean caseSensitive) { + final boolean caseSensitive, + ConcurrentHashMap javacCache) + { super(owner, projectInformation, root, ci); assert ident != null; this.ident = ident; this.listeners = new CopyOnWriteArrayList<>(); this.initialized = new AtomicBoolean(); this.caseSensitive = caseSensitive; + this.javacCache = javacCache; } @Override @@ -180,26 +189,31 @@ private void fireDescriptorChange(Collection replace } } + @NonNull + private JavacTaskImpl getOrCreateJavac(@NonNull FileObject root) { + return javacCache.computeIfAbsent(root, r -> { + final ClasspathInfo cpInfo = ClasspathInfo.create(root); + JavacTaskImpl ret = JavacParser.createJavacTask( + cpInfo, null, null, null, null, null, null, null, + Collections.emptyList()); + // Force JTImpl.prepareCompiler to get JTImpl into Context + ret.enter(); + return ret; + }); + } + @NonNull private Collection resolve() { final List symbols = new ArrayList<>(); try { final String binName = ElementHandleAccessor.getInstance().getJVMSignature(getOwner())[0]; - /* Build the JavacTask via JavacParser.createJavacTask, which pre-registers NetBeans' - javac context enhancers, e.g. NBClassReader plus NBAttr, NBEnter, NBMemberEnter, - NBResolve, NBClassFinder, NBJavaCompiler, NBClassWriter, instead of calling - JavacTool.create().getTask(...) directly. - - NetBeans .sig files (the per-root cached form of indexed Java classes) are written by - NBClassWriter and use a relaxed/extended class-file format that only NBClassReader knows - how to read. Stock javac's ClassReader would rejects them with BadClassFile. */ - final ClasspathInfo cpInfo = ClasspathInfo.create(getRoot()); - final JavacTaskImpl jt = JavacParser.createJavacTask( - cpInfo, null, null, null, null, null, null, null, - Collections.emptyList()); - //Force JTImpl.prepareCompiler to get JTImpl into Context - jt.enter(); - final TypeElement te = ElementUtils.getTypeElementByBinaryName(jt, binName); + final JavacTaskImpl jt = getOrCreateJavac(getRoot()); + final TypeElement te; + /* Keep access to shared JavacTaskImpl instances thread-safe in case javacCache gets + shared across threads. */ + synchronized (jt) { + te = ElementUtils.getTypeElementByBinaryName(jt, binName); + } if (te != null) { if (ident.equals(getSimpleName(te, null, caseSensitive))) { final String simpleName = te.getSimpleName().toString(); diff --git a/java/java.sourceui/src/org/netbeans/modules/java/source/ui/JavaSymbolProvider.java b/java/java.sourceui/src/org/netbeans/modules/java/source/ui/JavaSymbolProvider.java index 8a568566d304..5e8eab2a6ab7 100644 --- a/java/java.sourceui/src/org/netbeans/modules/java/source/ui/JavaSymbolProvider.java +++ b/java/java.sourceui/src/org/netbeans/modules/java/source/ui/JavaSymbolProvider.java @@ -19,6 +19,7 @@ package org.netbeans.modules.java.source.ui; +import com.sun.tools.javac.api.JavacTaskImpl; import java.io.IOException; import java.net.URL; import java.util.Collection; @@ -29,6 +30,7 @@ import java.util.List; import java.util.Map; import java.util.Set; +import java.util.concurrent.ConcurrentHashMap; import java.util.concurrent.atomic.AtomicBoolean; import java.util.logging.Level; import java.util.logging.Logger; @@ -118,6 +120,7 @@ public void computeSymbolNames(final Context context, final Result result) { final Cache cache = scanInProgress ? Cache.create(textToSearch, st) : null; + ConcurrentHashMap javacCache = new ConcurrentHashMap<>(); doComputeSymbols(st, textToSearch, new ResultHandler() { private FileObject root; private ProjectInformation projectInfo; @@ -129,6 +132,9 @@ public void setHighlightText(String text) { @Override public void runRoot(FileObject root, ClassIndexImpl ci, Exec exec) throws IOException, InterruptedException { + /* In practice, one root is processed at a time, so we can clear the cache + whenever the root changes, and let the previous JavacTaskImpl be GCed. */ + javacCache.clear(); try { Project project = FileOwnerQuery.getOwner(root); @@ -153,7 +159,8 @@ public void handleResult(ElementHandle owner, String ident, boolean ci, owner, ident, - caseSensitive); + caseSensitive, + javacCache); result.addResult(d); if (cache != null) { cache.offer(d);