response to review, resource leak fixes, disable means disable

This commit is contained in:
Jason P. Leasure 2020-07-01 18:33:25 -04:00
parent a3320ef3ab
commit b3ee8cacfa
9 changed files with 135 additions and 139 deletions

View file

@ -586,7 +586,7 @@ public class BundleHost {
try {
dependentBundle.uninstall();
if (dependentBundle.getState() != Bundle.UNINSTALLED) {
System.err.printf("It ain't uninstalled!\n");
Msg.error(this, "Failed to uninstall bundle: " + dependentBundle.getLocation());
}
refreshBundlesSynchronously(new ArrayList<>(dependentBundles));
}
@ -799,7 +799,7 @@ public class BundleHost {
}
if (isSystem != bundle.isSystemBundle()) {
bundle.systemBundle = isSystem;
Msg.error(this, String.format("%s went from %system to %system", bundleFile,
Msg.error(this, String.format("Error, bundle %s went from %system to %system", bundleFile,
isSystem ? "not " : "", isSystem ? "" : "not "));
}
}
@ -878,7 +878,7 @@ public class BundleHost {
}
else {
Msg.error(this,
String.format("not a GhidraBundle: %s\n", osgiBundle.getLocation()));
String.format("Error, bundle event for non-GhidraBundle: %s\n", osgiBundle.getLocation()));
}
break;
// force "inactive" updates for all other states
@ -889,7 +889,7 @@ public class BundleHost {
}
else {
Msg.error(this,
String.format("not a GhidraBundle: %s\n", osgiBundle.getLocation()));
String.format("Error, bundle event for non-GhidraBundle: %s\n", osgiBundle.getLocation()));
}
break;
}

View file

@ -26,7 +26,6 @@ import javax.swing.*;
import javax.swing.event.TableModelEvent;
import docking.action.builder.ActionBuilder;
import docking.util.AnimationUtils;
import docking.widgets.filechooser.GhidraFileChooser;
import docking.widgets.filechooser.GhidraFileChooserMode;
import docking.widgets.table.GTableFilterPanel;
@ -200,8 +199,6 @@ public class BundleStatusComponentProvider extends ComponentProviderAdapter {
}
private void doRefresh() {
PrintWriter errOut = getTool().getService(ConsoleService.class).getStdErr();
List<BundleStatus> statuses = bundleStatusTableModel.getModelData()
.stream()
.filter(BundleStatus::isEnabled)
@ -216,7 +213,7 @@ public class BundleStatusComponentProvider extends ComponentProviderAdapter {
bundleHost.deactivateSynchronously(bundle.getLocationIdentifier());
}
catch (GhidraBundleException | InterruptedException e) {
e.printStackTrace(errOut);
Msg.error(this, "Error while deactivating bundle", e);
}
}
@ -238,7 +235,6 @@ public class BundleStatusComponentProvider extends ComponentProviderAdapter {
}
if (anythingCleaned) {
bundleStatusTableModel.fireTableDataChanged();
AnimationUtils.shakeComponent(getComponent());
}
}
@ -293,13 +289,10 @@ public class BundleStatusComponentProvider extends ComponentProviderAdapter {
files.stream().map(ResourceFile::new).collect(Collectors.toUnmodifiableList());
Collection<GhidraBundle> bundles = bundleHost.add(resourceFiles, true, false);
new TaskLauncher(new Task("activating new bundles") {
@Override
public void run(TaskMonitor monitor) throws CancelledException {
bundleHost.activateAll(bundles, monitor,
getTool().getService(ConsoleService.class).getStdErr());
}
}, getComponent(), 1000);
TaskLauncher.launchNonModal("activating new bundles", (monitor) -> {
bundleHost.activateAll(bundles, monitor,
getTool().getService(ConsoleService.class).getStdErr());
});
}
}
@ -308,12 +301,13 @@ public class BundleStatusComponentProvider extends ComponentProviderAdapter {
}
protected void doEnableBundles() {
new TaskLauncher(new EnableAndActivateBundlesTask("activating", true, true, false,
getSelectedStatuses()), getComponent(), 1000);
new TaskLauncher(
new EnableAndActivateBundlesTask("enabling", true, true, false, getSelectedStatuses()),
getComponent(), 1000);
}
protected void doDisableBundles() {
new TaskLauncher(new DeactivateAndDisableBundlesTask("deactivating", true, true, false,
new TaskLauncher(new DeactivateAndDisableBundlesTask("disabling", true, true, false,
getSelectedStatuses()), getComponent(), 1000);
}
@ -456,7 +450,7 @@ public class BundleStatusComponentProvider extends ComponentProviderAdapter {
@Override
public void run(TaskMonitor monitor) throws CancelledException {
List<GhidraBundle> bundles = statuses.stream()
.filter(status -> status.isActive())
.filter(status -> status.isEnabled())
.map(status -> bundleHost.getExistingGhidraBundle(status.getFile()))
.collect(Collectors.toList());
@ -467,8 +461,7 @@ public class BundleStatusComponentProvider extends ComponentProviderAdapter {
bundleHost.disable(bundle);
}
catch (GhidraBundleException | InterruptedException e) {
ConsoleService console = getTool().getService(ConsoleService.class);
e.printStackTrace(console.getStdErr());
Msg.error(this, "Error while deactivating and disabling bundle", e);
}
monitor.incrementProgress(1);
}
@ -503,7 +496,7 @@ public class BundleStatusComponentProvider extends ComponentProviderAdapter {
}
}
catch (Exception e) {
e.printStackTrace(console.getStdErr());
Msg.error(this, "Error during activation/deactivation of bundle", e);
}
finally {
status.setBusy(false);

View file

@ -67,6 +67,9 @@ public class BundleStatusTableModel
bundleHostListener = new MyBundleHostListener();
bundleHost.addListener(bundleHostListener);
// sort by path
setDefaultTableSortState(TableSortState.createDefaultSortState(1, true));
}
BundleStatus getStatus(GhidraBundle bundle) {
@ -564,26 +567,4 @@ public class BundleStatusTableModel
}
private class BusyBooleanRenderer extends GBooleanCellRenderer
implements AbstractWrapperTypeColumnRenderer<Boolean> {
@Override
public Component getTableCellRendererComponent(GTableCellRenderingData data) {
BundleStatus status = (BundleStatus) data.getRowObject();
Component component = super.getTableCellRendererComponent(data);
if (status.isBusy()) {
cb.setVisible(false);
cb.setEnabled(false);
setHorizontalAlignment(SwingConstants.CENTER);
setText("...");
}
else {
cb.setVisible(true);
cb.setEnabled(true);
setText("");
}
return component;
}
}
}

View file

@ -55,6 +55,12 @@ import ghidra.util.Msg;
*/
public class GhidraSourceBundle extends GhidraBundle {
private static final String GENERATED_ACTIVATOR_CLASSNAME = "GeneratedActivator";
/*
* Match the leftover part of a class file on removing the class name, e.g.
* we've found "MyClass.java", so we match "MyClass.class" by removing "MyClass" then
* computing IS_CLASS_FILE.test(".class") == true. We want to match inner
* class files like "MyClass$2.class" too, so IS_CLASS_FILE.test("$2.class") is also true.
*/
private static final Predicate<String> IS_CLASS_FILE =
Pattern.compile("(\\$.*)?\\.class", Pattern.CASE_INSENSITIVE).asMatchPredicate();
@ -170,7 +176,7 @@ public class GhidraSourceBundle extends GhidraBundle {
return path.replace(File.separatorChar, '.');
}
catch (IOException e) {
e.printStackTrace();
Msg.error(this, "getting class name for script", e);
return null;
}
}
@ -201,9 +207,9 @@ public class GhidraSourceBundle extends GhidraBundle {
private String getPreviousBuildErrors() {
return buildErrors.values()
.stream()
.map(BuildError::getMessage)
.collect(Collectors.joining());
.stream()
.map(BuildError::getMessage)
.collect(Collectors.joining());
}
private String parseImportPackageMetadata(ResourceFile javaSource) {
@ -233,14 +239,14 @@ public class GhidraSourceBundle extends GhidraBundle {
requirements = OSGiUtils.parseImportPackage(importPackage);
}
catch (BundleException e) {
throw new GhidraBundleException(getLocationIdentifier(), "parsing manifest",
throw new GhidraBundleException(getLocationIdentifier(), "parsing metadata",
e);
}
sourceFileToRequirements.put(rootSourceFile, requirements);
for (BundleRequirement requirement : requirements) {
requirementToSourceFileMap
.computeIfAbsent(requirement.toString(), x -> new ArrayList<>())
.add(rootSourceFile);
.computeIfAbsent(requirement.toString(), x -> new ArrayList<>())
.add(rootSourceFile);
}
}
}
@ -250,9 +256,9 @@ public class GhidraSourceBundle extends GhidraBundle {
private Map<String, BundleRequirement> getComputedReqs() {
Map<String, BundleRequirement> dedupedReqs = new HashMap<>();
sourceFileToRequirements.values()
.stream()
.flatMap(List::stream)
.forEach(r -> dedupedReqs.putIfAbsent(r.toString(), r));
.stream()
.flatMap(List::stream)
.forEach(r -> dedupedReqs.putIfAbsent(r.toString(), r));
return dedupedReqs;
}
@ -270,10 +276,12 @@ public class GhidraSourceBundle extends GhidraBundle {
ResourceFile manifestFile = getSourceManifestFile();
if (manifestFile.exists()) {
try {
Manifest manifest = new Manifest(manifestFile.getInputStream());
String importPackage = manifest.getMainAttributes().getValue("Import-Package");
for (BundleRequirement r : OSGiUtils.parseImportPackage(importPackage)) {
reqs.putIfAbsent(r.toString(), r);
try (InputStream manifestInputStream = manifestFile.getInputStream()) {
Manifest manifest = new Manifest(manifestInputStream);
String importPackage = manifest.getMainAttributes().getValue("Import-Package");
for (BundleRequirement r : OSGiUtils.parseImportPackage(importPackage)) {
reqs.putIfAbsent(r.toString(), r);
}
}
}
catch (IOException | BundleException e) {
@ -385,18 +393,18 @@ public class GhidraSourceBundle extends GhidraBundle {
}
boolean hasNewManifest() {
ResourceFile sourceManfiest = getSourceManifestFile();
ResourceFile sourceManifest = getSourceManifestFile();
Path binaryManifest = getBinaryManifestPath();
return sourceManfiest.exists() && (Files.notExists(binaryManifest) ||
sourceManfiest.lastModified() > binaryManifest.toFile().lastModified());
return sourceManifest.exists() && (Files.notExists(binaryManifest) ||
sourceManifest.lastModified() > binaryManifest.toFile().lastModified());
}
protected static boolean wipeContents(Path path) throws IOException {
if (Files.exists(path)) {
boolean anythingDeleted = false;
for (Path p : (Iterable<Path>) Files.walk(path)
.sorted(Comparator.reverseOrder())::iterator) {
.sorted(Comparator.reverseOrder())::iterator) {
anythingDeleted |= Files.deleteIfExists(p);
}
return anythingDeleted;
@ -445,8 +453,8 @@ public class GhidraSourceBundle extends GhidraBundle {
}
Arrays.stream(correspondingBinaries(sourceFile))
.map(rf -> rf.getFile(false).toPath())
.forEach(oldBinaries::add);
.map(rf -> rf.getFile(false).toPath())
.forEach(oldBinaries::add);
}
}
}
@ -524,8 +532,8 @@ public class GhidraSourceBundle extends GhidraBundle {
return anythingChanged | wipeBinDir();
}
catch (IOException | GhidraBundleException e) {
Msg.showError(this, null, "source bundle clean error",
"while attempting to delete the compiled directory, an exception was thrown", e);
Msg.showError(this, null, "Source bundle clean error",
"While attempting to delete the compiled directory, an exception was thrown", e);
}
return anythingChanged;
}
@ -582,7 +590,7 @@ public class GhidraSourceBundle extends GhidraBundle {
while (!stack.isEmpty()) {
ResourceFile sourceSubdir = stack.pop();
String relPath = sourceSubdir.getAbsolutePath()
.substring(getSourceDirectory().getAbsolutePath().length());
.substring(getSourceDirectory().getAbsolutePath().length());
if (relPath.startsWith(File.separator)) {
relPath = relPath.substring(1);
}
@ -646,10 +654,11 @@ public class GhidraSourceBundle extends GhidraBundle {
summary.printf("%d missing @importpackage%s:%s", requirements.size(),
requirements.size() > 1 ? "s" : "",
requirements.stream()
.flatMap(r -> OSGiUtils.extractPackageNamesFromFailedResolution(r.toString())
.stream())
.distinct()
.collect(Collectors.joining(",")));
.flatMap(
r -> OSGiUtils.extractPackageNamesFromFailedResolution(r.toString())
.stream())
.distinct()
.collect(Collectors.joining(",")));
}
// send the capabilities to phidias
bundleWirings.forEach(bundleJavaManager::addBundleWiring);
@ -845,15 +854,15 @@ public class GhidraSourceBundle extends GhidraBundle {
options.add("-sourcepath");
options.add(getSourceDirectory().toString());
options.add("-classpath");
options
.add(System.getProperty("java.class.path") + File.pathSeparator + binaryDir.toString());
options.add(
System.getProperty("java.class.path") + File.pathSeparator + binaryDir.toString());
options.add("-proc:none");
BundleJavaManager bundleJavaManager = createBundleJavaManager(writer, summary, options);
final List<ResourceFileJavaFileObject> sourceFiles = newSources.stream()
.map(sf -> new ResourceFileJavaFileObject(sf.getParentFile(), sf, Kind.SOURCE))
.collect(Collectors.toList());
.map(sf -> new ResourceFileJavaFileObject(sf.getParentFile(), sf, Kind.SOURCE))
.collect(Collectors.toList());
Path binaryManifest = getBinaryManifestPath();
if (Files.exists(binaryManifest)) {
@ -880,8 +889,9 @@ public class GhidraSourceBundle extends GhidraBundle {
ResourceFile sourceManifest = getSourceManifestFile();
if (sourceManifest.exists()) {
Files.createDirectories(binaryManifest.getParent());
Files.copy(sourceManifest.getInputStream(), binaryManifest,
StandardCopyOption.REPLACE_EXISTING);
try (InputStream inStream = sourceManifest.getInputStream()) {
Files.copy(inStream, binaryManifest, StandardCopyOption.REPLACE_EXISTING);
}
return summary.getValue();
}
@ -943,18 +953,18 @@ public class GhidraSourceBundle extends GhidraBundle {
*/
ClassMapper(Path directory) throws IOException {
classToClassFilesMap = Files.exists(directory) ? Files.list(directory)
.filter(
f -> Files.isRegularFile(f) && f.getFileName().toString().endsWith(".class"))
.collect(groupingBy(f -> {
String fileName = f.getFileName().toString();
// if f is the class file of an inner class, use the class name
int money = fileName.indexOf('$');
if (money >= 0) {
return fileName.substring(0, money);
}
// drop ".class"
return fileName.substring(0, fileName.length() - 6);
})) : Collections.emptyMap();
.filter(f -> Files.isRegularFile(f) &&
f.getFileName().toString().endsWith(".class"))
.collect(groupingBy(f -> {
String fileName = f.getFileName().toString();
// if f is the class file of an inner class, use the class name
int money = fileName.indexOf('$');
if (money >= 0) {
return fileName.substring(0, money);
}
// drop ".class"
return fileName.substring(0, fileName.length() - 6);
})) : Collections.emptyMap();
}
List<Path> findAndRemove(ResourceFile sourceFile) {
@ -968,9 +978,9 @@ public class GhidraSourceBundle extends GhidraBundle {
}
long lastModifiedClassFile = classFiles.isEmpty() ? -1
: classFiles.stream()
.mapToLong(p -> p.toFile().lastModified())
.min()
.getAsLong();
.mapToLong(p -> p.toFile().lastModified())
.min()
.getAsLong();
// if source is newer than the oldest binary, report
if (lastModifiedSource > lastModifiedClassFile) {
return classFiles;
@ -985,9 +995,9 @@ public class GhidraSourceBundle extends GhidraBundle {
public Collection<Path> extraClassFiles() {
return classToClassFilesMap.values()
.stream()
.flatMap(l -> l.stream())
.collect(Collectors.toList());
.stream()
.flatMap(l -> l.stream())
.collect(Collectors.toList());
}
}

View file

@ -30,7 +30,16 @@ import org.apache.felix.framework.util.manifestparser.ManifestParser;
import org.osgi.framework.*;
import org.osgi.framework.wiring.BundleRequirement;
import ghidra.util.Msg;
public class OSGiUtils {
/*
* Match group 1 contains the file name from a resource string, e.g. from
* file:/path/to/some.jar!/Some.class
* we get "/path/to/some.jar", everything between ':' and '!'
*
*/
private static Pattern JAR_FILENAME_EXTRACTOR = Pattern.compile("^.*:(.*)!.*$");
/**
* The syntax of the error generated when OSGi requirements cannot be resolved is
@ -111,9 +120,9 @@ public class OSGiUtils {
// from https://dzone.com/articles/locate-jar-classpath-given
static String findJarForClass(Class<?> c) {
final URL location;
final String classLocation = c.getName().replace('.', '/') + ".class";
final ClassLoader loader = c.getClassLoader();
URL location;
String classLocation = c.getName().replace('.', '/') + ".class";
ClassLoader loader = c.getClassLoader();
if (loader == null) {
location = ClassLoader.getSystemResource(classLocation);
}
@ -121,8 +130,7 @@ public class OSGiUtils {
location = loader.getResource(classLocation);
}
if (location != null) {
Pattern pattern = Pattern.compile("^.*:(.*)!.*$");
Matcher matcher = pattern.matcher(location.toString());
Matcher matcher = JAR_FILENAME_EXTRACTOR.matcher(location.toString());
if (matcher.find()) {
return matcher.group(1);
}
@ -160,10 +168,9 @@ public class OSGiUtils {
? relativePath.substring(0, lastSlash).replace(File.separatorChar, '.')
: "");
});
}
catch (IOException e) {
e.printStackTrace();
Msg.error(OSGiUtils.class, "Error while collecting packages from directory", e);
}
}
@ -179,7 +186,7 @@ public class OSGiUtils {
}
}
catch (IOException e) {
e.printStackTrace();
Msg.error(OSGiUtils.class, "Error while collecting packages from jar", e);
}
}

View file

@ -48,9 +48,6 @@ import ghidra.util.task.TaskListener;
)
//@formatter:on
public class GhidraScriptMgrPlugin extends ProgramPlugin implements GhidraScriptService {
/** number of GhidraScriptMgrPlugin references to the BundleHost owned by {@link GhidraScriptUtil} */
private static int referenceCount = 0;
private final GhidraScriptComponentProvider provider;
private final BundleHost bundleHost;
@ -62,16 +59,10 @@ public class GhidraScriptMgrPlugin extends ProgramPlugin implements GhidraScript
*/
public GhidraScriptMgrPlugin(PluginTool tool) {
super(tool, true, true, true);
// Each tool starts a new script manager plugin, but we only ever want one bundle host.
// We store the one BundleHost in GhidraScriptUtil and keep a count of references to it.
if (referenceCount == 0) {
bundleHost = new BundleHost();
GhidraScriptUtil.initialize(bundleHost, null);
}
else {
bundleHost = GhidraScriptUtil.getBundleHost();
}
referenceCount += 1;
// GhidraScriptUtil (creates and) manages one instance.
bundleHost = GhidraScriptUtil.acquireBundleHostReference();
provider = new GhidraScriptComponentProvider(this, bundleHost);
}
@ -80,10 +71,7 @@ public class GhidraScriptMgrPlugin extends ProgramPlugin implements GhidraScript
protected void dispose() {
super.dispose();
provider.dispose();
referenceCount -= 1;
if (referenceCount == 0) {
GhidraScriptUtil.dispose();
}
GhidraScriptUtil.releaseBundleHostReference();
}
@Override

View file

@ -53,14 +53,7 @@ public class GhidraScriptInfoManager {
String name = scriptFile.getName();
List<ResourceFile> files = scriptNameToFilesMap.get(name);
if (files != null) {
Iterator<ResourceFile> iter = files.iterator();
while (iter.hasNext()) {
ResourceFile rFile = iter.next();
if (scriptFile.equals(rFile)) {
iter.remove();
break;
}
}
files.remove(scriptFile);
if (files.isEmpty()) {
scriptNameToFilesMap.remove(name);
}

View file

@ -19,6 +19,7 @@ import java.io.File;
import java.io.IOException;
import java.nio.file.Files;
import java.util.*;
import java.util.concurrent.atomic.AtomicInteger;
import java.util.stream.Collectors;
import generic.jar.ResourceFile;
@ -48,6 +49,9 @@ public class GhidraScriptUtil {
private static List<GhidraScriptProvider> providers;
// number of references from the GUI to bundleHost
private static AtomicInteger referenceCount = new AtomicInteger(0);
/**
* @return the bundle host used for scripting
*/
@ -70,8 +74,7 @@ public class GhidraScriptUtil {
bundleHost.startFramework();
}
catch (OSGiException | IOException e) {
e.printStackTrace();
Msg.error(GhidraScript.class, "failed to initialize BundleHost", e);
Msg.error(GhidraScript.class, "Failed to initialize BundleHost", e);
}
}
@ -111,9 +114,9 @@ public class GhidraScriptUtil {
*/
public static List<ResourceFile> getScriptSourceDirectories() {
return bundleHost.getBundleFiles()
.stream()
.filter(ResourceFile::isDirectory)
.collect(Collectors.toList());
.stream()
.filter(ResourceFile::isDirectory)
.collect(Collectors.toList());
}
/**
@ -229,9 +232,9 @@ public class GhidraScriptUtil {
public static List<ResourceFile> getExplodedCompiledSourceBundlePaths() {
try {
return Files.list(BundleHost.getOsgiDir())
.filter(Files::isDirectory)
.map(x -> new ResourceFile(x.toFile()))
.collect(Collectors.toList());
.filter(Files::isDirectory)
.map(x -> new ResourceFile(x.toFile()))
.collect(Collectors.toList());
}
catch (IOException e) {
Msg.showError(GhidraScriptUtil.class, null, "error",
@ -385,4 +388,26 @@ public class GhidraScriptUtil {
return null;
}
/**
* When running the GUI, {@link GhidraScriptUtil} manages a single {@link BundleHost} instance.
*
* @return the BundleHost singleton
*/
public static BundleHost acquireBundleHostReference() {
if (referenceCount.getAndIncrement() == 0) {
initialize(new BundleHost(), null);
}
return bundleHost;
}
/**
* release the reference the BundleHost reference. When no references remain,
* {@link #dispose()} is called.
*/
public static void releaseBundleHostReference() {
if (referenceCount.getAndDecrement() == 1) {
dispose();
}
}
}

View file

@ -66,8 +66,7 @@ public class JavaScriptProvider extends GhidraScriptProvider {
}
}
catch (GhidraBundleException e) {
e.printStackTrace();
Msg.error(this, "while deactivating bundle for delete", e);
Msg.error(this, "Error while deactivating bundle for delete", e);
return false;
}
return super.deleteScript(sourceFile);