GT-3302 - Front-end UI freeze - review fixes: removed unusual 'tracker'

concept

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

@@ -1,40 +0,0 @@
-/* ###
- * 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.main;
-
-/**
- * This class helps implement a workaround to deal with a UI lockup issue.  Clients performing
- * long-running version control actions that require a lock have the potential to block 
- * the UI thread when the action context is updated.  This is because some actions will check
- * their enablement by examining file version control state, which requires a lock.   The enablement
- * check happens in the Swing thread.  Thus, when the required lock is already in use for a 
- * long-running operation, the UI is blocked.   This class effectively provides a global state
- * flag that can be maintained by the the owner of the aforementioned actions.   When a check
- * for action enablement happens, if this flag is marked 'busy', then the enablement check
- * will not take place. 
- */
-public class DomainFileOperationTracker {
-
-	private volatile boolean isBusy;
-
-	public void setBusy(boolean isBusy) {
-		this.isBusy = isBusy;
-	}
-
-	public boolean isBusy() {
-		return isBusy;
-	}
-}

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

@@ -86,7 +86,6 @@ public class FrontEndPlugin extends Plugin
 	private WorkspacePanel workspacePanel;
 	private Project activeProject;
 	private ProjectManager projectManager;
-	private DomainFileOperationTracker fileTracker = new DomainFileOperationTracker();
 
 	/**
 	 * the sash panel that contains the active project data and
@@ -1010,9 +1009,6 @@ public class FrontEndPlugin extends Plugin
 		tool.addLocalAction(frontEndProvider, propertiesAction);
 	}
 
-	/**
-	 * Delete the tool template from the tool chest.
-	 */
 	private void delete(String toolName) {
 		if (!confirmDelete(toolName + " from your local tool chest?")) {
 			return;
@@ -1050,17 +1046,11 @@ public class FrontEndPlugin extends Plugin
 		}
 	}
 
-	/**
-	 * @see ghidra.framework.main.FrontEndService#addProjectListener(ghidra.framework.model.ProjectListener)
-	 */
 	@Override
 	public void addProjectListener(ProjectListener l) {
 		((FrontEndTool) tool).addProjectListener(l);
 	}
 
-	/**
-	 * @see ghidra.framework.main.FrontEndService#removeProjectListener(ghidra.framework.model.ProjectListener)
-	 */
 	@Override
 	public void removeProjectListener(ProjectListener l) {
 		if (tool != null) { // tool is null when we've been disposed
@@ -1068,10 +1058,6 @@ public class FrontEndPlugin extends Plugin
 		}
 	}
 
-	public DomainFileOperationTracker getFileOperationTracker() {
-		return fileTracker;
-	}
-
 	class FrontEndProvider extends ComponentProvider {
 		public FrontEndProvider(PluginTool tool) {
 			super(tool, "FrontEnd", "FrontEnd Tool");

Ghidra/Framework/Project/src/main/java/ghidra/framework/main/datatable/DomainFileContext.java

@@ -20,7 +20,7 @@ import java.util.List;
 import ghidra.framework.model.DomainFile;
 
 /**
- * A context that provides to the client
+ * A context that provides information to actions about domain files that are selected in the tool
  */
 public interface DomainFileContext {
 
@@ -43,23 +43,4 @@ public interface DomainFileContext {
 	 * @return true if in the active project
 	 */
 	public boolean isInActiveProject();
-
-	/**
-	 * Returns true if the the current context is busy.  This is used by actions to signal to
-	 * the environment that they are performing a long-running operation.
-	 * @return true if busy
-	 */
-	public boolean isBusy();
-
-	/**
-	 * Sets this context to busy.  This is used by actions to signal to
-	 * the environment that they are performing a long-running operation.
-	 * <p>
-	 * Note: context state is not maintained by the tool.  Thus, the notion of being busy will
-	 * not be maintained across calls to <code>getActionContext()</code>.  Further, if implementors
-	 * wish to track business, then they must do so themselves.
-	 * 
-	 * @param isBusy true if busy
-	 */
-	public void setBusy(boolean isBusy);
 }

Ghidra/Framework/Project/src/main/java/ghidra/framework/main/datatable/DomainFileProviderContextAction.java

@@ -17,6 +17,8 @@ package ghidra.framework.main.datatable;
 
 import docking.ActionContext;
 import docking.action.DockingAction;
+import ghidra.framework.main.AppInfo;
+import ghidra.framework.main.FrontEndTool;
 
 public abstract class DomainFileProviderContextAction extends DockingAction {
 
@@ -30,12 +32,12 @@ public abstract class DomainFileProviderContextAction extends DockingAction {
 			return false;
 		}
 
-		DomainFileContext context = (DomainFileContext) actionContext;
-		if (context.isBusy()) {
+		FrontEndTool tool = AppInfo.getFrontEndTool();
+		if (tool.isExecutingCommand()) {
 			return false;
 		}
 
-		return isEnabledForContext(context);
+		return isEnabledForContext((DomainFileContext) actionContext);
 	}
 
 	protected boolean isEnabledForContext(DomainFileContext context) {
@@ -67,11 +69,12 @@ public abstract class DomainFileProviderContextAction extends DockingAction {
 			return false;
 		}
 
-		DomainFileContext fileContext = (DomainFileContext) context;
-		if (fileContext.isBusy()) {
+		FrontEndTool tool = AppInfo.getFrontEndTool();
+		if (tool.isExecutingCommand()) {
 			return false;
 		}
-		return isAddToPopup(fileContext);
+
+		return isAddToPopup((DomainFileContext) context);
 	}
 
 	protected boolean isAddToPopup(DomainFileContext context) {

Ghidra/Framework/Project/src/main/java/ghidra/framework/main/datatable/ProjectDataActionContext.java

@@ -21,7 +21,6 @@ import java.util.List;
 
 import docking.ActionContext;
 import docking.ComponentProvider;
-import ghidra.framework.main.DomainFileOperationTracker;
 import ghidra.framework.model.*;
 
 public class ProjectDataActionContext extends ActionContext implements DomainFileContext {
@@ -32,40 +31,19 @@ public class ProjectDataActionContext extends ActionContext implements DomainFil
 	private boolean isActiveProject;
 	private ProjectData projectData;
 	private boolean isTransient;
-	private DomainFileOperationTracker fileTracker;
 
 	public ProjectDataActionContext(ComponentProvider provider, ProjectData projectData,
 			Object contextObject, List<DomainFolder> selectedFolders,
 			List<DomainFile> selectedFiles, Component comp, boolean isActiveProject) {
-		this(provider, projectData, null, contextObject, selectedFolders, selectedFiles, comp,
-			isActiveProject);
-	}
 
-	public ProjectDataActionContext(ComponentProvider provider, ProjectData projectData,
-			DomainFileOperationTracker fileTracker, Object contextObject,
-			List<DomainFolder> selectedFolders, List<DomainFile> selectedFiles, Component comp,
-			boolean isActiveProject) {
 		super(provider, contextObject, comp);
 		this.projectData = projectData;
-		this.fileTracker = fileTracker;
 		this.selectedFolders = selectedFolders;
 		this.selectedFiles = selectedFiles;
 		this.comp = comp;
 		this.isActiveProject = isActiveProject;
 	}
 
-	@Override
-	public boolean isBusy() {
-		return fileTracker != null && fileTracker.isBusy();
-	}
-
-	@Override
-	public void setBusy(boolean isBusy) {
-		if (fileTracker != null) {
-			fileTracker.setBusy(isBusy);
-		}
-	}
-
 	@Override
 	public List<DomainFile> getSelectedFiles() {
 		if (selectedFiles == null) {

Ghidra/Framework/Project/src/main/java/ghidra/framework/main/datatable/ProjectDataTablePanel.java

@@ -30,7 +30,6 @@ import docking.help.HelpService;
 import docking.widgets.label.GHtmlLabel;
 import docking.widgets.table.*;
 import docking.widgets.table.threaded.*;
-import ghidra.framework.main.DomainFileOperationTracker;
 import ghidra.framework.main.FrontEndPlugin;
 import ghidra.framework.model.*;
 import ghidra.framework.plugintool.PluginTool;
@@ -103,8 +102,9 @@ public class ProjectDataTablePanel extends JPanel {
 				checkOpen(e);
 			}
 		});
-		gTable.getSelectionModel().addListSelectionListener(
-			e -> plugin.getTool().contextChanged(null));
+		gTable.getSelectionModel()
+				.addListSelectionListener(
+					e -> plugin.getTool().contextChanged(null));
 		gTable.setDefaultRenderer(Date.class, new DateCellRenderer());
 		gTable.setDefaultRenderer(DomainFileType.class, new TypeCellRenderer());
 
@@ -282,8 +282,7 @@ public class ProjectDataTablePanel extends JPanel {
 			list.add(info.getDomainFile());
 		}
 
-		DomainFileOperationTracker fileTracker = plugin.getFileOperationTracker();
-		return new ProjectDataActionContext(provider, projectData, fileTracker,
+		return new ProjectDataActionContext(provider, projectData,
 			model.getRowObject(selectedRows[0]), null, list, gTable, true);
 	}
 

Ghidra/Framework/Project/src/main/java/ghidra/framework/main/datatree/ProjectDataTreeActionContext.java

@@ -20,7 +20,6 @@ import java.util.List;
 import javax.swing.tree.TreePath;
 
 import docking.ComponentProvider;
-import ghidra.framework.main.DomainFileOperationTracker;
 import ghidra.framework.main.datatable.ProjectDataActionContext;
 import ghidra.framework.model.*;
 
@@ -30,10 +29,10 @@ public class ProjectDataTreeActionContext extends ProjectDataActionContext {
 	private DataTree tree;
 
 	public ProjectDataTreeActionContext(ComponentProvider provider, ProjectData projectData,
-			DomainFileOperationTracker fileTracker, TreePath[] selectionPaths,
+			TreePath[] selectionPaths,
 			List<DomainFolder> folderList, List<DomainFile> fileList, DataTree tree,
 			boolean isActiveProject) {
-		super(provider, projectData, fileTracker, getContextObject(selectionPaths), folderList,
+		super(provider, projectData, getContextObject(selectionPaths), folderList,
 			fileList, tree, isActiveProject);
 		this.selectionPaths = selectionPaths;
 		this.tree = tree;

Ghidra/Framework/Project/src/main/java/ghidra/framework/main/datatree/ProjectDataTreePanel.java

@@ -32,7 +32,8 @@ import docking.help.HelpService;
 import docking.widgets.tree.*;
 import docking.widgets.tree.support.DepthFirstIterator;
 import docking.widgets.tree.support.GTreeSelectionListener;
-import ghidra.framework.main.*;
+import ghidra.framework.main.FrontEndPlugin;
+import ghidra.framework.main.FrontEndTool;
 import ghidra.framework.model.*;
 import ghidra.framework.plugintool.PluginTool;
 import ghidra.util.HelpLocation;
@@ -337,9 +338,8 @@ public class ProjectDataTreePanel extends JPanel {
 			}
 		}
 
-		DomainFileOperationTracker fileTracker = plugin.getFileOperationTracker();
 		ProjectDataTreeActionContext context =
-			new ProjectDataTreeActionContext(provider, projectData, fileTracker, selectionPaths,
+			new ProjectDataTreeActionContext(provider, projectData, selectionPaths,
 				domainFolderList, domainFileList, tree, isActiveProject);
 		boolean isTransient = tool == null; // null for stand-alone dialog, not the project's tree
 		context.setTransient(isTransient);

Ghidra/Framework/Project/src/main/java/ghidra/framework/main/projectdata/actions/VersionControlCheckOutAction.java

@@ -61,14 +61,7 @@ public class VersionControlCheckOutAction extends VersionControlAction {
 
 	@Override
 	public void actionPerformed(DomainFileContext context) {
-
-		try {
-			context.setBusy(true);
-			checkOut(context.getSelectedFiles());
-		}
-		finally {
-			context.setBusy(false);
-		}
+		checkOut(context.getSelectedFiles());
 	}
 
 	/**

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

@@ -21,8 +21,8 @@ import java.awt.*;
 import java.awt.event.KeyEvent;
 import java.beans.PropertyChangeListener;
 import java.beans.PropertyChangeSupport;
+import java.util.*;
 import java.util.List;
-import java.util.Set;
 import java.util.concurrent.atomic.AtomicReference;
 
 import javax.swing.ImageIcon;
@@ -57,8 +57,7 @@ import ghidra.framework.plugintool.mgr.*;
 import ghidra.framework.plugintool.util.*;
 import ghidra.framework.project.ProjectDataService;
 import ghidra.util.*;
-import ghidra.util.task.Task;
-import ghidra.util.task.TaskLauncher;
+import ghidra.util.task.*;
 
 /**
  * Base class that is a container to manage plugins and their actions, and to coordinate the 
@@ -88,12 +87,13 @@ public abstract class PluginTool extends AbstractDockingTool implements Tool, Se
 	private String subTitle;
 
 	private ServiceManager serviceMgr;
-	private ToolTaskManager taskMgr;
 	private OptionsManager optionsMgr;
 	private PluginManager pluginMgr;
 	private EventManager eventMgr;
 	private DialogManager dialogMgr;
 	private PropertyChangeSupport propertyChangeMgr;
+	private ToolTaskManager taskMgr;
+	private Set<TaskListener> executingTaskListeners = Collections.synchronizedSet(new HashSet<>());
 
 	private OptionsChangeListener optionsListener = new ToolOptionsListener();
 	protected ManagePluginsDialog manageDialog;
@@ -455,6 +455,7 @@ public abstract class PluginTool extends AbstractDockingTool implements Tool, Se
 
 	private void disposeManagers() {
 		taskMgr.dispose();
+		executingTaskListeners.clear();
 	}
 
 	@Override
@@ -647,7 +648,7 @@ public abstract class PluginTool extends AbstractDockingTool implements Tool, Se
 	 * @return true if there is a command being executed
 	 */
 	public boolean isExecutingCommand() {
-		return taskMgr.isBusy();
+		return taskMgr.isBusy() || !executingTaskListeners.isEmpty();
 	}
 
 	/**
@@ -694,6 +695,25 @@ public abstract class PluginTool extends AbstractDockingTool implements Tool, Se
 		taskMgr.scheduleFollowOnCommand(cmd, obj);
 	}
 
+	/**
+	 * Launch the task in a new thread
+	 * @param task task to run in a new thread
+	 * @param delay number of milliseconds to delay the display of task monitor dialog
+	 */
+	public void execute(Task task, int delay) {
+		task.addTaskListener(new TaskBusyListener());
+		new TaskLauncher(task, getToolFrame(), delay);
+	}
+
+	/**
+	 * Launch the task in a new thread
+	 * @param task task to run in a new thread
+	 */
+	public void execute(Task task) {
+		task.addTaskListener(new TaskBusyListener());
+		new TaskLauncher(task, winMgr.getActiveWindow());
+	}
+
 	/**
 	 * Get the options for the given category name; if no options exist with
 	 * the given name, then one is created.
@@ -738,23 +758,6 @@ public abstract class PluginTool extends AbstractDockingTool implements Tool, Se
 		return optionsMgr.getOptions();
 	}
 
-	/**
-	 * Launch the task in a new thread.
-	 * @param task task to run in a new thread
-	 * @param delay number of milliseconds to delay the display of task monitor dialog
-	 */
-	public void execute(Task task, int delay) {
-		new TaskLauncher(task, getToolFrame(), delay);
-	}
-
-	/**
-	 * Launch the task in a new thread.
-	 * @param task task to run in a new thread
-	 */
-	public void execute(Task task) {
-		new TaskLauncher(task, winMgr.getActiveWindow());
-	}
-
 	/**
 	 * Get the project associated with this tool.  Null will be returned if there is no
 	 * project open or if this tool does not use projects.
@@ -1497,4 +1500,20 @@ public abstract class PluginTool extends AbstractDockingTool implements Tool, Se
 		}
 	}
 
+	private class TaskBusyListener implements TaskListener {
+
+		TaskBusyListener() {
+			executingTaskListeners.add(this);
+		}
+
+		@Override
+		public void taskCompleted(Task t) {
+			executingTaskListeners.remove(this);
+		}
+
+		@Override
+		public void taskCancelled(Task t) {
+			executingTaskListeners.remove(this);
+		}
+	}
 }