Skip to content

Commit

Permalink
GP-4608: Improving how we handle loading classes with the same name
Browse files Browse the repository at this point in the history
  • Loading branch information
ryanmkurtz committed May 21, 2024
1 parent 72d9d0e commit 87131b4
Show file tree
Hide file tree
Showing 6 changed files with 41 additions and 25 deletions.
Original file line number Diff line number Diff line change
Expand Up @@ -16,7 +16,7 @@
package ghidra.util.classfinder;

import java.io.File;
import java.util.Set;
import java.util.List;

import ghidra.util.exception.CancelledException;
import ghidra.util.task.TaskMonitor;
Expand All @@ -33,8 +33,8 @@ class ClassDir {
classPackage = new ClassPackage(dir, "", monitor);
}

void getClasses(Set<ClassFileInfo> set, TaskMonitor monitor) throws CancelledException {
classPackage.getClasses(set, monitor);
void getClasses(List<ClassFileInfo> list, TaskMonitor monitor) throws CancelledException {
classPackage.getClasses(list, monitor);
}

String getDirPath() {
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -62,8 +62,8 @@ class ClassJar implements ClassLocation {
}

@Override
public void getClasses(Set<ClassFileInfo> set, TaskMonitor monitor) {
set.addAll(classes);
public void getClasses(List<ClassFileInfo> list, TaskMonitor monitor) {
list.addAll(classes);
}

private void scanJar(TaskMonitor monitor) throws CancelledException {
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -15,7 +15,7 @@
*/
package ghidra.util.classfinder;

import java.util.Set;
import java.util.List;

import ghidra.util.exception.CancelledException;
import ghidra.util.task.TaskMonitor;
Expand All @@ -27,5 +27,5 @@ interface ClassLocation {

public static final String CLASS_EXT = ".class";

public void getClasses(Set<ClassFileInfo> set, TaskMonitor monitor) throws CancelledException;
public void getClasses(List<ClassFileInfo> list, TaskMonitor monitor) throws CancelledException;
}
Original file line number Diff line number Diff line change
Expand Up @@ -89,15 +89,16 @@ private File getPackageDir(File lRootDir, String lPackageName) {
}

@Override
public void getClasses(Set<ClassFileInfo> set, TaskMonitor monitor) throws CancelledException {
public void getClasses(List<ClassFileInfo> list, TaskMonitor monitor)
throws CancelledException {

set.addAll(classes);
list.addAll(classes);

Iterator<ClassPackage> it = children.iterator();
while (it.hasNext()) {
monitor.checkCancelled();
ClassPackage subPkg = it.next();
subPkg.getClasses(set, monitor);
subPkg.getClasses(list, monitor);
}
}

Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -30,8 +30,6 @@

import javax.swing.event.ChangeListener;

import org.apache.commons.collections4.BidiMap;
import org.apache.commons.collections4.bidimap.DualHashBidiMap;
import org.apache.commons.lang3.StringUtils;
import org.apache.logging.log4j.LogManager;
import org.apache.logging.log4j.Logger;
Expand Down Expand Up @@ -81,7 +79,7 @@ public class ClassSearcher {
private static List<Class<?>> FILTER_CLASSES = Arrays.asList(ExtensionPoint.class);
private static Pattern extensionPointSuffixPattern;
private static Map<String, Set<ClassFileInfo>> extensionPointSuffixToInfoMap;
private static BidiMap<ClassFileInfo, Class<?>> loadedCache = new DualHashBidiMap<>();
private static Map<ClassFileInfo, Class<?>> loadedCache = new HashMap<>();
private static Set<ClassFileInfo> falsePositiveCache = new HashSet<>();
private static volatile boolean hasSearched;
private static volatile boolean isSearching;
Expand Down Expand Up @@ -187,12 +185,6 @@ public static <T> List<Class<? extends T>> getClasses(Class<T> ancestorClass,
Class<?> c = loadedCache.get(info);
if (c == null) {
c = loadExtensionPoint(info.path(), info.name());
ClassFileInfo existing = loadedCache.getKey(c);
if (existing != null) {
log.info(
"Skipping load of class '%s' from '%s'. Already loaded from '%s'."
.formatted(info.name(), info.path(), existing.path()));
}
if (c == null) {
falsePositiveCache.add(info);
continue;
Expand Down Expand Up @@ -418,8 +410,8 @@ private static Map<String, Set<ClassFileInfo>> findClasses(TaskMonitor monitor)
throws CancelledException {
log.info("Searching for classes...");

Set<ClassDir> classDirs = new HashSet<>();
Set<ClassJar> classJars = new HashSet<>();
List<ClassDir> classDirs = new ArrayList<>();
List<ClassJar> classJars = new ArrayList<>();

for (String searchPath : gatherSearchPaths()) {
String lcSearchPath = searchPath.toLowerCase();
Expand All @@ -441,17 +433,31 @@ else if (searchFile.isDirectory()) {
}
}

Set<ClassFileInfo> classSet = new HashSet<>();
List<ClassFileInfo> classList = new ArrayList<>();
for (ClassDir dir : classDirs) {
monitor.checkCancelled();
dir.getClasses(classSet, monitor);
dir.getClasses(classList, monitor);
}
for (ClassJar jar : classJars) {
monitor.checkCancelled();
jar.getClasses(classSet, monitor);
jar.getClasses(classList, monitor);
}

// We can't load more than one class with the same name, so de-duplicate them
Map<String, ClassFileInfo> uniqueClassMap = new HashMap<>();
for (ClassFileInfo info : classList) {
ClassFileInfo existing = uniqueClassMap.get(info.name());
if (existing == null) {
uniqueClassMap.put(info.name(), info);
}
else {
log.info("Ignoring class '%s' from '%s'. Already found at '%s'."
.formatted(info.name(), info.path(), existing.path()));
}
}

return classSet.stream()
return uniqueClassMap.values()
.stream()
.collect(Collectors.groupingBy(ClassFileInfo::suffix, Collectors.toSet()));
}

Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -165,6 +165,15 @@ private static List<String> buildClasspath(GhidraApplicationLayout layout) throw
else { /* Eclipse dev mode */
// Support loading pre-built, jar-based, non-repo extensions in Eclipse dev mode
addExtensionJarPaths(classpathList, modules, layout);

// Eclipse launches the Utility module, so it's already on the classpath. We don't
// want to add it a second time, so remove the one we discovered.
GModule utilityModule = modules.get("Utility");
if (utilityModule == null) {
throw new IOException("Failed to find the 'Utility' module!");
}
classpathList.removeIf(
e -> e.startsWith(utilityModule.getModuleRoot().getAbsolutePath()));
}

// In development mode, jars do not live in module directories. Instead, each jar lives
Expand Down

0 comments on commit 87131b4

Please sign in to comment.