GP-3343 Getting dispose to be called on Front-end plugins

Ghidra/Features/Base/src/main/java/ghidra/app/merge/MergeManager.java

@@ -26,8 +26,7 @@ import generic.util.WindowUtilities;
 import ghidra.framework.data.DomainObjectMergeManager;
 import ghidra.framework.model.DomainFile;
 import ghidra.framework.model.UndoableDomainObject;
-import ghidra.framework.plugintool.ModalPluginTool;
+import ghidra.framework.plugintool.*;
-import ghidra.framework.plugintool.PluginTool;
 import ghidra.framework.plugintool.util.PluginException;
 import ghidra.program.model.listing.DomainObjectChangeSet;
 import ghidra.util.*;
@@ -502,7 +501,7 @@ public abstract class MergeManager implements DomainObjectMergeManager {
 				if (mergePlugin != null) {
 					mergePlugin.dispose();
 				}
-				mergeTool.exit(); // cleanup!
+				PluginToolAccessUtils.dispose(mergeTool); // cleanup!
 				mergeTool = null;
 			});
 		}

Ghidra/Features/Base/src/main/java/ghidra/test/TestTool.java

@@ -42,7 +42,7 @@ public class TestTool extends GhidraTool {
 		}
 
 		Runnable r = () -> {
-			exit();
+			dispose();
 			if (getProject().getToolServices() != null) {
 				getProject().getToolServices().closeTool(TestTool.this);
 			}

Ghidra/Features/VersionTracking/src/main/java/ghidra/feature/vt/gui/plugin/VTSubToolManager.java

@@ -336,8 +336,8 @@ public class VTSubToolManager implements VTControllerListener, OptionsChangeList
 		saveSubordinateToolConfig(sourceTool);
 		saveSubordinateToolConfig(destinationTool);
 		pluginList.clear();
-		sourceTool.exit();
+		PluginToolAccessUtils.dispose(sourceTool);
-		destinationTool.exit();
+		PluginToolAccessUtils.dispose(destinationTool);
 		sourceTool = null;
 		destinationTool = null;
 	}

Ghidra/Framework/Project/src/main/java/ghidra/framework/main/AppInfo.java

@@ -46,7 +46,7 @@ public class AppInfo {
 
 	public static void exitGhidra() {
 		assertFrontEndRunning();
-		tool.exit();
+		tool.close(); // closing the front end tool will exit the application
 	}
 
 	private static void assertFrontEndRunning() {

Ghidra/Framework/Project/src/main/java/ghidra/framework/main/FileActionManager.java

@@ -35,6 +35,7 @@ import ghidra.framework.client.RepositoryAdapter;
 import ghidra.framework.model.*;
 import ghidra.framework.options.SaveState;
 import ghidra.framework.plugintool.PluginTool;
+import ghidra.framework.plugintool.PluginToolAccessUtils;
 import ghidra.framework.store.LockException;
 import ghidra.util.*;
 import ghidra.util.exception.NotFoundException;
@@ -123,7 +124,7 @@ class FileActionManager {
 		closeProjectAction = new DockingAction("Close Project", plugin.getName()) {
 			@Override
 			public void actionPerformed(ActionContext context) {
-				closeProject(false); //not exiting
+				closeProject(false); // not exiting
 			}
 		};
 		closeProjectAction.setEnabled(false);
@@ -187,7 +188,7 @@ class FileActionManager {
 			// if all is well and we already have an active project, close it
 			Project activeProject = plugin.getActiveProject();
 			if (activeProject != null) {
-				if (!closeProject(false)) { // false -->not exiting
+				if (!closeProject(false)) { // false --> not exiting
 					return; // user canceled
 				}
 			}
@@ -428,7 +429,7 @@ class FileActionManager {
 		// check for any changes since last saved
 		PluginTool[] runningTools = activeProject.getToolManager().getRunningTools();
 		for (PluginTool runningTool : runningTools) {
-			if (!runningTool.canClose(isExiting)) {
+			if (!PluginToolAccessUtils.canClose(runningTool)) {
 				return false;
 			}
 		}

Ghidra/Framework/Project/src/main/java/ghidra/framework/main/FrontEndPlugin.java

@@ -445,23 +445,7 @@ public class FrontEndPlugin extends Plugin
 		projectDataPanel.readDataState(saveState);
 	}
 
-	/**
+	boolean closeActiveProject() {
-	 * Exit the Ghidra application; the parameter indicates whether
-	 * the user should be prompted to save the project that is about
-	 * to be closed
-	 */
-	void exitGhidra() {
-		boolean okToExit = closeActiveProject();
-		if (okToExit) {
-			System.exit(0);
-
-		}
-		else if (!tool.isVisible()) {
-			tool.setVisible(true);
-		}
-	}
-
-	private boolean closeActiveProject() {
 		if (activeProject == null) {
 			return true;
 		}
@@ -470,7 +454,7 @@ public class FrontEndPlugin extends Plugin
 		}
 		catch (Exception e) {
 			Msg.error(this, "Unexpected Exception: " + e.getMessage(), e); // Keep this.
-			int result = OptionDialog.showOptionDialog(tool.getToolFrame(), "Close Project Failed",
+			int result = OptionDialog.showOptionDialog(null, "Close Project Failed",
 				"Error Description: [ " + e + " ]" + "\n" +
 					"=====> Do you wish to exit Ghidra, possibly losing changes? <=====",
 				"Exit Ghidra (Possibly Lose Changes)", OptionDialog.ERROR_MESSAGE);

Ghidra/Framework/Project/src/main/java/ghidra/framework/main/FrontEndTool.java

@@ -166,12 +166,17 @@ public class FrontEndTool extends PluginTool implements OptionsChangeListener {
 	}
 
 	@Override
-	protected void dispose() {
+	public void dispose() {
 		super.dispose();
 
 		if (logProvider != null) {
 			logProvider.dispose();
 		}
+		shutdown();
+	}
+
+	protected void shutdown() {
+		System.exit(0);
 	}
 
 	private void ensureSize() {
@@ -392,13 +397,8 @@ public class FrontEndTool extends PluginTool implements OptionsChangeListener {
 	}
 
 	@Override
-	public void exit() {
+	public boolean canClose() {
-		plugin.exitGhidra();
+		return super.canClose() && plugin.closeActiveProject();
-	}
-
-	@Override
-	public void close() {
-		close(true);
 	}
 
 	/**

Ghidra/Framework/Project/src/main/java/ghidra/framework/main/ProjectInfoDialog.java

@@ -38,6 +38,7 @@ import ghidra.framework.data.ConvertFileSystem;
 import ghidra.framework.data.TransientDataManager;
 import ghidra.framework.model.*;
 import ghidra.framework.plugintool.PluginTool;
+import ghidra.framework.plugintool.PluginToolAccessUtils;
 import ghidra.framework.remote.User;
 import ghidra.framework.store.local.*;
 import ghidra.util.*;
@@ -95,8 +96,8 @@ public class ProjectInfoDialog extends DialogComponentProvider {
 
 		connectionButton.setContentAreaFilled(false);
 		connectionButton.setSelected(isConnected);
-		connectionButton.setBorder(
+		connectionButton
-			isConnected ? BorderFactory.createBevelBorder(BevelBorder.LOWERED)
+				.setBorder(isConnected ? BorderFactory.createBevelBorder(BevelBorder.LOWERED)
 						: BorderFactory.createBevelBorder(BevelBorder.RAISED));
 		updateConnectButtonToolTip();
 		if (isConnected) {
@@ -181,8 +182,8 @@ public class ProjectInfoDialog extends DialogComponentProvider {
 
 		String toolTipForChange = "Change server information or specify another repository.";
 		String toolTipForConvert = "Convert project to be a shared project.";
-		changeConvertButton.setToolTipText(
+		changeConvertButton
-			repository != null ? toolTipForChange : toolTipForConvert);
+				.setToolTipText(repository != null ? toolTipForChange : toolTipForConvert);
 
 		Class<? extends LocalFileSystem> fsClass = project.getProjectData().getLocalStorageClass();
 		String convertStorageButtonLabel = null;
@@ -198,8 +199,8 @@ public class ProjectInfoDialog extends DialogComponentProvider {
 			convertStorageButton.addActionListener(e -> convertToIndexedFilesystem());
 			help.registerHelp(changeConvertButton,
 				new HelpLocation(GenericHelpTopics.FRONT_END, "Convert_Project_Storage"));
-			convertStorageButton.setToolTipText(
+			convertStorageButton
-				"Convert/Upgrade project storage to latest Indexed Filesystem");
+					.setToolTipText("Convert/Upgrade project storage to latest Indexed Filesystem");
 		}
 
 		JPanel p = new JPanel(new FlowLayout());
@@ -260,8 +261,8 @@ public class ProjectInfoDialog extends DialogComponentProvider {
 		connectionButton.setName("Connect Button");
 		connectionButton.setContentAreaFilled(false);
 		connectionButton.setSelected(isConnected);
-		connectionButton.setBorder(
+		connectionButton
-			isConnected ? BorderFactory.createBevelBorder(BevelBorder.LOWERED)
+				.setBorder(isConnected ? BorderFactory.createBevelBorder(BevelBorder.LOWERED)
 						: BorderFactory.createBevelBorder(BevelBorder.RAISED));
 		updateConnectButtonToolTip();
 		HelpService help = Help.getHelpService();
@@ -343,8 +344,7 @@ public class ProjectInfoDialog extends DialogComponentProvider {
 	private void updateSharedProjectInfo() {
 		int openCount = getOpenFileCount();
 		if (openCount != 0) {
-			Msg.showInfo(getClass(), getComponent(),
+			Msg.showInfo(getClass(), getComponent(), "Cannot Change Project Info with Open Files",
-				"Cannot Change Project Info with Open Files",
 				"Found " + openCount + " open project file(s).\n" +
 					"Before your project info can be updated, you must\n" +
 					"close all open project files and tools.");
@@ -392,7 +392,7 @@ public class ProjectInfoDialog extends DialogComponentProvider {
 	private boolean checkToolsClose() {
 		PluginTool[] runningTools = project.getToolManager().getRunningTools();
 		for (PluginTool runningTool : runningTools) {
-			if (!runningTool.canClose(false)) {
+			if (!PluginToolAccessUtils.canClose(runningTool)) {
 				return false;
 			}
 			runningTool.close();
@@ -448,8 +448,7 @@ public class ProjectInfoDialog extends DialogComponentProvider {
 
 		int openCount = getOpenFileCount();
 		if (openCount != 0) {
-			Msg.showInfo(getClass(), getComponent(),
+			Msg.showInfo(getClass(), getComponent(), "Cannot Convert Project with Open Files",
-				"Cannot Convert Project with Open Files",
 				"Found " + openCount + " open project file(s).\n" +
 					"Before your project can be converted, you must close\n" +
 					"all open project files and tools.");

Ghidra/Framework/Project/src/main/java/ghidra/framework/main/SaveDataDialog.java

@@ -126,8 +126,8 @@ public class SaveDataDialog extends DialogComponentProvider {
 			}
 		}
 		if (list.size() > 0) {
-			DomainFile[] deleteFiles = new DomainFile[list.size()];
+			DomainFile[] saveFiles = new DomainFile[list.size()];
-			SaveTask task = new SaveTask(list.toArray(deleteFiles));
+			SaveTask task = new SaveTask(list.toArray(saveFiles));
 			new TaskLauncher(task, getComponent());
 		}
 		else {

Ghidra/Framework/Project/src/main/java/ghidra/framework/main/TestFrontEndTool.java

@@ -28,6 +28,12 @@ public class TestFrontEndTool extends FrontEndTool {
 
 	@Override
 	public void close() {
-		setVisible(false);
+		// overridden to not ask to save
+		dispose();
+	}
+
+	@Override
+	protected void shutdown() {
+		// let test environment bring the system down
 	}
 }

Ghidra/Framework/Project/src/main/java/ghidra/framework/plugintool/PluginTool.java

@@ -467,10 +467,6 @@ public abstract class PluginTool extends AbstractDockingTool {
 		return eventMgr.hasToolListeners();
 	}
 
-	public void exit() {
-		dispose();
-	}
-
 	protected void dispose() {
 		isDisposed = true;
 
@@ -500,6 +496,7 @@ public abstract class PluginTool extends AbstractDockingTool {
 
 		disposeManagers();
 		winMgr.dispose();
+		toolServices.closeTool(this);
 	}
 
 	private void disposeManagers() {
@@ -1123,46 +1120,80 @@ public abstract class PluginTool extends AbstractDockingTool {
 	}
 
 	/**
-	 * Close this tool:
+	 * Closes this tool, possibly with input from the user. The following conditions are checked
+	 * and can prompt the user for more info and allow them to cancel the close.
 	 * <OL>
-	 * 	<LI>if there are no tasks running.
+	 * 	<LI>Running tasks. Closing with running tasks could lead to data loss.
-	 * 	<LI>resolve the state of any plugins so they can be closed.
+	 *  <LI>Plugins get asked if they can be closed. They may prompt the user to resolve
-	 * 	<LI>Prompt the user to save any changes.
+	 *  some plugin specific state.
-	 * 	<LI>close all associated plugins (this closes the domain object if one is open).
+	 * 	<LI>The user is prompted to save any data changes. 
-	 * 	<LI>pop up dialog to save the configuration if it has changed.
+	 * 	<LI>Tools are saved, possibly asking the user to resolve any conflicts caused by
-	 * 	<LI>notify the project tool services that this tool is going away.
+	 *  changing multiple instances of the same tool in different ways.
+	 * 	<LI>If all the above conditions passed, the tool is closed and disposed.
 	 * </OL>
 	 */
 	@Override
 	public void close() {
-		close(false);
+		if (canClose()) {
+			dispose();
+		}
 	}
 
-	protected void close(boolean isExiting) {
+	protected boolean canClose() {
-		if (canClose(isExiting) && pluginMgr.saveData()) {
+		if (isBusy()) {
-			doClose();
+			return false;
 		}
+		if (!canClosePlugins()) {
+			return false;
+		}
+		if (!pluginMgr.saveData()) {
+			return false;
+		}
+		return doSaveTool();
 	}
 
 	/**
-	 * Close this tool:
+	 * Normally, tools are not allowed to close while tasks are running in that tool as it
-	 * <OL>
+	 * could leave the application in an unstable state. Tools that exit the application 
-	 * 	<LI>if there are no tasks running.
+	 * (such as the FrontEndTool) can override this so that the user can terminate running tasks
-	 * 	<LI>close all associated plugins (this closes the domain object if one is open).
+	 * and not have that prevent exiting the application.
-	 * 	<LI>pop up dialog to save the configuration if it has changed;
+	 * @return whether the user is allowed to terminate tasks so that the tool can be closed.
-	 * 	<LI>notify the project tool services that this tool is going away.
-	 * </OL>
 	 */
-	private void doClose() {
+	protected boolean allowTerminatingTasksWhenClosing() {
-
+		return false;
-		if (!doSaveTool()) {
-			return; // if cancelled, don't close
 	}
 
-		exit();
+	/**
-		toolServices.closeTool(this);
+	 * Checks if this tool's plugins are in a state to be closed.
+	 * @return true if all the plugins in the tool can be closed without further user input.
+	 */
+	protected boolean canClosePlugins() {
+		return pluginMgr.canClose();
 	}
 
+	/**
+	 * Checks if this tool has running tasks, with optionally giving the user an
+	 * opportunity to cancel them.
+	 * 
+	 * @return true if this tool has running tasks
+	 */
+	protected boolean isBusy() {
+		if (taskMgr.isBusy()) {
+			int result = OptionDialog.showYesNoDialog(getToolFrame(), "Tool Busy Executing Task",
+				"The tool is busy performing a background task.\n If you continue the" +
+					" task may be terminated and some work may be lost!\n\nContinue anyway?");
+			if (result != OptionDialog.YES_OPTION) {
+				return true;
+			}
+			taskMgr.stop(false);
+		}
+		return false;
+	}
+
+	/**
+	 * Returns true if this tool needs saving
+	 * @return true if this tool needs saving
+	 */
 	public boolean shouldSave() {
 		return hasConfigChanged(); // ignore the window layout changes
 	}
@@ -1199,41 +1230,6 @@ public abstract class PluginTool extends AbstractDockingTool {
 		return true;
 	}
 
-	/**
-	 * Can this tool be closed?
-	 * <br>Note: This forces plugins to terminate any tasks they have running and
-	 * apply any unsaved data to domain objects or files. If they can't do
-	 * this or the user cancels then this returns false.
-	 *
-	 * @param isExiting whether the tool is exiting
-	 * @return false if this tool has tasks in progress or can't be closed
-	 * since the user has unfinished/unsaved changes.
-	 */
-	public boolean canClose(boolean isExiting) {
-		if (taskMgr.isBusy()) {
-			if (isExiting) {
-				int result = OptionDialog.showYesNoDialog(getToolFrame(),
-					"Tool Busy Executing Task",
-					"The tool is busy performing a background task.\n If you continue the" +
-						" task may be terminated and some work may be lost!\n\nContinue anyway?");
-				if (result == OptionDialog.NO_OPTION) {
-					return false;
-				}
-				taskMgr.stop(false);
-			}
-			else {
-				beep();
-				Msg.showInfo(getClass(), getToolFrame(), "Tool Busy",
-					"You must stop all background tasks before tool may close.");
-				return false;
-			}
-		}
-		if (!pluginMgr.canClose()) {
-			return false;
-		}
-		return true;
-	}
-
 	/**
 	 * Can the domain object be closed?
 	 * <br>Note: This forces plugins to terminate any tasks they have running for the

Ghidra/Framework/Project/src/main/java/ghidra/framework/plugintool/PluginToolAccessUtils.java

@@ -0,0 +1,48 @@
+/* ###
+ * IP: GHIDRA
+ *
+ * Licensed 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 ghidra.framework.plugintool;
+
+/**
+ * Utility class to provide access to non-public methods on PluginTool. There are a number of
+ * methods that internal classes need access to but we don't want on the public interface of
+ * PluginTool.This is a stopgap approach until we clean up the package structure for tool related
+ * classes and interfaces. This class should only be used by internal tool manager classes.
+ */
+public class PluginToolAccessUtils {
+
+	private PluginToolAccessUtils() {
+		// Can't be constructed
+	}
+
+	/**
+	 * Disposes the tool.
+	 * @param tool the tool to dispose
+	 */
+	public static void dispose(PluginTool tool) {
+		tool.dispose();
+	}
+
+	/**
+	 * Returns true if the tool can be closed. Note this does not handle any data saving. It only
+	 * checks that there are no tasks running and the plugins can be closed.
+	 * @param tool the tool to close
+	 * @return true if the tool can be closed
+	 */
+	public static boolean canClose(PluginTool tool) {
+		return !tool.isBusy() && tool.canClosePlugins();
+	}
+
+}

Ghidra/Framework/Project/src/main/java/ghidra/framework/project/DefaultProject.java

@@ -303,8 +303,7 @@ public class DefaultProject implements Project {
 		ProjectFileManager projectData = (ProjectFileManager) c.getProjectData();
 		if (projectData == null) {
 			throw new IOException(
-				"Failed to view specified project/repository: " +
+				"Failed to view specified project/repository: " + GhidraURL.getDisplayString(url));
-					GhidraURL.getDisplayString(url));
 		}
 		url = projectData.getProjectLocator().getURL(); // transform to repository root URL
 
@@ -423,7 +422,6 @@ public class DefaultProject implements Project {
 		try {
 
 			if (toolManager != null) {
-				toolManager.close();
 				toolManager.dispose();
 			}
 			if (projectManager != null) {

Ghidra/Framework/Project/src/main/java/ghidra/framework/project/tool/GhidraTool.java

@@ -167,12 +167,12 @@ public class GhidraTool extends PluginTool {
 	}
 
 	@Override
-	public void exit() {
+	public void dispose() {
 		if (fileOpenDropHandler != null) {
 			fileOpenDropHandler.dispose();
 			fileOpenDropHandler = null;
 		}
-		super.exit();
+		super.dispose();
 	}
 
 	private void addCloseAction() {

Ghidra/Framework/Project/src/main/java/ghidra/framework/project/tool/ToolManagerImpl.java

@@ -375,16 +375,6 @@ public class ToolManagerImpl implements ToolManager, PropertyChangeListener {
 		return ((changedWorkspaces.size() > 0) || activeWorkspaceChanged);
 	}
 
-	/**
-	 * Close all running tools in the project.
-	 */
-	public void close() {
-		for (Workspace element : workspaces) {
-			WorkspaceImpl w = (WorkspaceImpl) element;
-			w.close();
-		}
-	}
-
 	/**
 	 * Save the tools that are opened and changed, that will be brought back up when the project
 	 * is reopened
@@ -445,6 +435,10 @@ public class ToolManagerImpl implements ToolManager, PropertyChangeListener {
 	}
 
 	public void dispose() {
+		for (Workspace element : workspaces) {
+			WorkspaceImpl w = (WorkspaceImpl) element;
+			w.dispose();
+		}
 		toolServices.dispose();
 	}
 

Ghidra/Framework/Project/src/main/java/ghidra/framework/project/tool/WorkspaceImpl.java

@@ -15,13 +15,16 @@
  */
 package ghidra.framework.project.tool;
 
-import java.util.*;
+import java.util.Iterator;
+import java.util.Set;
+import java.util.concurrent.CopyOnWriteArraySet;
 
 import org.jdom.Element;
 
 import ghidra.framework.model.ToolTemplate;
 import ghidra.framework.model.Workspace;
 import ghidra.framework.plugintool.PluginTool;
+import ghidra.framework.plugintool.PluginToolAccessUtils;
 import ghidra.util.exception.DuplicateNameException;
 
 /**
@@ -31,11 +34,10 @@ import ghidra.util.exception.DuplicateNameException;
  * 
  */
 class WorkspaceImpl implements Workspace {
-	private final static int TYPICAL_NUM_RUNNING_TOOLS = 5;
 
 	private String name;
 	private ToolManagerImpl toolManager;
-	private Set<PluginTool> runningTools = new HashSet<PluginTool>(TYPICAL_NUM_RUNNING_TOOLS);
+	private Set<PluginTool> runningTools = new CopyOnWriteArraySet<>();
 	private boolean isActive;
 
 	WorkspaceImpl(String name, ToolManagerImpl toolManager) {
@@ -223,14 +225,9 @@ class WorkspaceImpl implements Workspace {
 	 * Close all running tools; called from the close() method in
 	 * ToolManagerImpl which is called from the Project's close()
 	 */
-	void close() {
+	void dispose() {
 		for (PluginTool tool : runningTools) {
-			try {
+			PluginToolAccessUtils.dispose(tool);
-				tool.exit();
-			}
-			finally {
-				toolManager.toolRemoved(this, tool);
-			}
 		}
 		runningTools.clear();
 	}

Ghidra/Framework/SoftwareModeling/src/test/java/ghidra/test/DummyTool.java

@@ -64,17 +64,11 @@ public class DummyTool extends PluginTool {
 		name = typeName;
 	}
 
-	@Override
-	public void exit() {
-		//do nothing
-	}
-
 	@Override
 	public void close() {
 		if (project != null) {
 			project.getToolServices().closeTool(this);
 		}
-
 	}
 
 	@Override
@@ -266,7 +260,7 @@ public class DummyTool extends PluginTool {
 	}
 
 	@Override
-	public boolean canClose(boolean isExiting) {
+	public boolean canClose() {
 		return true;
 	}