Merge remote-tracking branch 'origin/GP-443-dragonmacher-task-launcher-slowness' into patch

Ghidra/Features/Base/src/main/java/ghidra/app/plugin/core/script/GhidraScriptComponentProvider.java

@@ -20,7 +20,9 @@ import java.awt.Rectangle;
 import java.awt.event.*;
 import java.io.*;
 import java.util.*;
+import java.util.concurrent.atomic.AtomicReference;
 import java.util.function.Predicate;
+import java.util.function.Supplier;
 import java.util.stream.Collectors;
 
 import javax.swing.*;
@@ -50,7 +52,6 @@ import ghidra.program.model.listing.Program;
 import ghidra.util.*;
 import ghidra.util.datastruct.WeakDataStructureFactory;
 import ghidra.util.datastruct.WeakSet;
-import ghidra.util.exception.CancelledException;
 import ghidra.util.table.GhidraTableFilterPanel;
 import ghidra.util.task.*;
 import resources.ResourceManager;
@@ -238,7 +239,7 @@ public class GhidraScriptComponentProvider extends ComponentProviderAdapter {
 
 	/**
 	 * Restore state for bundles, user actions, and filter.
-	 * 
+	 *
 	 * @param saveState the state object
 	 */
 	public void readConfigState(SaveState saveState) {
@@ -267,7 +268,7 @@ public class GhidraScriptComponentProvider extends ComponentProviderAdapter {
 
 	/**
 	 * Save state for bundles, user actions, and filter.
-	 * 
+	 *
 	 * @param saveState the state object
 	 */
 
@@ -374,7 +375,7 @@ public class GhidraScriptComponentProvider extends ComponentProviderAdapter {
 
 	/**
 	 * Copy a script, renaming references to the class name.
-	 * 
+	 *
 	 * @param sourceScript source script
 	 * @param destinationScript destination script
 	 * @throws IOException if we fail to create temp, write contents, copy, or delete temp
@@ -605,28 +606,34 @@ public class GhidraScriptComponentProvider extends ComponentProviderAdapter {
 	}
 
 	void runScript(ResourceFile scriptFile, TaskListener listener) {
-		if (SystemUtilities.isEventDispatchThread()) {
+		lastRunScript = scriptFile;
-			new TaskLauncher(new Task("compiling script directory", true, false, true, true) {
+		GhidraScript script = doGetScriptInstance(scriptFile);
-				@Override
+		doRunScript(script, listener);
-				public void run(TaskMonitor monitor) throws CancelledException {
-					doRunScript(scriptFile, listener);
-				}
-			}, plugin.getTool().getToolFrame(), 1000);
-		}
-		else {
-			doRunScript(scriptFile, listener);
-		}
 	}
 
-	private void doRunScript(ResourceFile scriptFile, TaskListener listener) {
+	private GhidraScript doGetScriptInstance(ResourceFile scriptFile) {
-		lastRunScript = scriptFile;
 
-		ConsoleService console = plugin.getConsoleService();
+		Supplier<GhidraScript> scriptSupplier = () -> {
-		GhidraScript script = getScriptInstance(scriptFile, console);
+			ConsoleService console = plugin.getConsoleService();
-		if (script == null) {
+			return getScriptInstance(scriptFile, console);
-			return;
+		};
+
+		if (!Swing.isSwingThread()) {
+			return scriptSupplier.get();
 		}
 
+		AtomicReference<GhidraScript> ref = new AtomicReference<>();
+		TaskBuilder.withRunnable(monitor -> ref.set(scriptSupplier.get()))
+				.setTitle("Compiling Script Directory")
+				.setLaunchDelay(1000)
+				.launchModal();
+
+		return ref.get();
+	}
+
+	private void doRunScript(GhidraScript script, TaskListener listener) {
+
+		ConsoleService console = plugin.getConsoleService();
 		RunScriptTask task = new RunScriptTask(script, plugin.getCurrentState(), console);
 		runningScriptTaskSet.add(task);
 		task.addTaskListener(listener);
@@ -715,7 +722,7 @@ public class GhidraScriptComponentProvider extends ComponentProviderAdapter {
 
 	/**
 	 * refresh the list of scripts by listing files in each script directory.
-	 * 
+	 *
 	 * Note: this method can be used off the swing event thread.
 	 */
 	void refresh() {
@@ -724,7 +731,7 @@ public class GhidraScriptComponentProvider extends ComponentProviderAdapter {
 
 	/**
 	 * refresh the list of scripts by listing files in each script directory.
-	 * 
+	 *
 	 * Note: this method MUST NOT BE USED off the swing event thread.
 	 */
 	private void doRefresh() {
@@ -810,14 +817,14 @@ public class GhidraScriptComponentProvider extends ComponentProviderAdapter {
 
 		/*
 		 			Unusual Algorithm
-		 			
+		
-		 	The tree nodes represent categories, but do not contain nodes for individual 
+			The tree nodes represent categories, but do not contain nodes for individual
 		 	scripts.  We wish to remove any of the tree nodes that no longer represent script
 		 	categories.  (This can happen when a script is deleted or its category is changed.)
 		 	This algorithm will assume that all nodes need to be deleted.  Then, each script is
 		 	examined, using its category to mark a given node as 'safe'; that node's parents are
-		 	also marked as safe.   Any nodes remaining unmarked have no reference script and 
+			also marked as safe.   Any nodes remaining unmarked have no reference script and
-		 	will be deleted. 
+			will be deleted.
 		 */
 
 		// note: turn String[] to List<String> to use hashing
@@ -899,8 +906,8 @@ public class GhidraScriptComponentProvider extends ComponentProviderAdapter {
 	}
 
 	/**
-	 * reassign an existing editor component 
+	 * reassign an existing editor component
-	 * 
+	 *
 	 * @param oldScript who the editor is currently assigned to
 	 * @param newScript the new script to assign it to
 	 */
@@ -1169,7 +1176,7 @@ public class GhidraScriptComponentProvider extends ComponentProviderAdapter {
 
 		@Override
 		public void bundleBuilt(GhidraBundle bundle, String summary) {
-			// on enable, build can happen before the refresh populates the info manager with 
+			// on enable, build can happen before the refresh populates the info manager with
 			// this bundle's scripts, so allow for the possibility and create the info here.
 			if (bundle instanceof GhidraSourceBundle) {
 				GhidraSourceBundle sourceBundle = (GhidraSourceBundle) bundle;

Ghidra/Framework/Docking/src/main/java/docking/DockingWindowManager.java

@@ -1380,6 +1380,10 @@ public class DockingWindowManager implements PropertyChangeListener, Placeholder
 			focusedPlaceholder.setSelected(false);
 		}
 
+		// Activating placeholders is done to help users find widgets hiding in plain sight. 
+		// Assume that the user is no longer seeking a provider if they are clicking around.
+		activatedInfo.clear();
+
 		focusedPlaceholder = placeholder;
 
 		// put the last focused placeholder at the front of the list for restoring focus work later
@@ -2276,5 +2280,10 @@ public class DockingWindowManager implements PropertyChangeListener, Placeholder
 			}
 			lastCalledTimestamp = System.currentTimeMillis();
 		}
+
+		void clear() {
+			lastActivatedPlaceholder = null;
+			lastCalledTimestamp = 0;
+		}
 	}
 }

Ghidra/Framework/Docking/src/main/java/ghidra/util/task/TaskDialog.java

@@ -17,78 +17,87 @@ package ghidra.util.task;
 
 import java.awt.BorderLayout;
 import java.awt.Component;
-import java.util.concurrent.atomic.AtomicInteger;
+import java.util.concurrent.CountDownLatch;
+import java.util.concurrent.TimeUnit;
+import java.util.concurrent.atomic.AtomicBoolean;
 import java.util.concurrent.atomic.AtomicReference;
 
-import javax.swing.*;
+import javax.swing.JPanel;
 
 import docking.DialogComponentProvider;
 import docking.DockingWindowManager;
 import docking.tool.ToolConstants;
 import docking.widgets.OptionDialog;
-import ghidra.util.HelpLocation;
+import ghidra.util.*;
-import ghidra.util.Swing;
 import ghidra.util.exception.CancelledException;
 import ghidra.util.timer.GTimer;
+import ghidra.util.timer.GTimerMonitor;
 
 /**
- * Dialog that is displayed to show activity for a Task that is running outside of the 
+ * Dialog that is displayed to show activity for a Task that is running outside of the
  * Swing Thread.
- * 
+ *
- * <p>Implementation note: 
+ * <p>Implementation note:
  * if this class is constructed with a {@code hasProgress} value of {@code false},
- * then an activity component will be shown, not a progress monitor.   Any calls to update 
+ * then an activity component will be shown, not a progress monitor.   Any calls to update
  * progress will not affect the display.   However, the display can be converted to use progress
  * by first calling {@link #setIndeterminate(boolean)} with a {@code false} value and then calling
  * {@link #initialize(long)}.    Once this has happened, this dialog will no longer use the
- * activity display--the progress bar is in effect for the duration of this dialog's usage.   
+ * activity display--the progress bar is in effect for the duration of this dialog's usage.
- * 
+ *
- * <p>This dialog can be toggled between indeterminate mode and progress mode via calls to 
+ * <p>This dialog can be toggled between indeterminate mode and progress mode via calls to
  * {@link #setIndeterminate(boolean)}.
  */
 public class TaskDialog extends DialogComponentProvider implements TaskMonitor {
 
-	/** Timer used to give the task a chance to complete */
-	private static final int SLEEPY_TIME = 10;
-
 	/** Amount of time to wait before showing the monitor dialog */
 	private final static int MAX_DELAY = 200000;
 
 	public final static int DEFAULT_WIDTH = 275;
 
-	private Timer showTimer;
+	private Runnable closeDialog = () -> {
-	private AtomicInteger taskID = new AtomicInteger();
+		close();
-	private Runnable closeDialog;
+		dispose();
-	private Component centerOnComp;
+	};
-	private Runnable shouldCancelRunnable;
+	private Runnable verifyCancel = () -> {
-	private boolean taskDone;
+		if (promptToVerifyCancel()) {
+			cancel();
+		}
+	};
+
+	private GTimerMonitor showTimer = GTimerMonitor.DUMMY;
+	private CountDownLatch finished = new CountDownLatch(1);
 	private boolean supportsProgress;
 
 	private JPanel mainPanel;
 	private JPanel activityPanel;
 	private TaskMonitorComponent monitorComponent;
+	private Component centerOnComponent;
 
 	/** If not null, then the value of the string has yet to be rendered */
 	private AtomicReference<String> newMessage = new AtomicReference<>();
 	private SwingUpdateManager messageUpdater =
 		new SwingUpdateManager(100, 250, () -> setStatusText(newMessage.getAndSet(null)));
 
-	/** 
+	private AtomicBoolean shown = new AtomicBoolean();
+
+	/**
 	 * Constructor
-	 * 
+	 *
-	 * @param centerOnComp component to be centered over when shown,
+	 * @param centerOnComp component to be centered over when shown, otherwise center over parent.
-	 * otherwise center over parent.  If both centerOnComp and parent
+	 * If both centerOnComp and parent are null, dialog will be centered on screen.
-	 * are null, dialog will be centered on screen.
 	 * @param task the Task that this dialog will be associated with
+	 * @param finished overrides the latch used by this dialog to know when the task is finished
 	 */
-	public TaskDialog(Component centerOnComp, Task task) {
+	TaskDialog(Component centerOnComp, Task task, CountDownLatch finished) {
 		this(centerOnComp, task.getTaskTitle(), task.isModal(), task.canCancel(),
 			task.hasProgress());
+		this.finished = finished;
 	}
 
 	/**
 	 * Constructor
-	 *  
+	 *
 	 * @param task the Task that this dialog will be associated with
 	 */
 	public TaskDialog(Task task) {
@@ -97,7 +106,7 @@ public class TaskDialog extends DialogComponentProvider implements TaskMonitor {
 
 	/**
 	 * Constructor
-	 * 
+	 *
 	 * @param title title for the dialog
 	 * @param canCancel true if the task can be canceled
 	 * @param isModal true if the dialog should be modal
@@ -109,8 +118,8 @@ public class TaskDialog extends DialogComponentProvider implements TaskMonitor {
 
 	/**
 	 * Constructor
-	 * 
+	 *
-	 * @param centerOnComp component to be centered over when shown, otherwise center over 
+	 * @param centerOnComp component to be centered over when shown, otherwise center over
 	 *        parent.  If both centerOnComp is null, then the active window will be used
 	 * @param title title for the dialog
 	 * @param isModal true if the dialog should be modal
@@ -120,7 +129,7 @@ public class TaskDialog extends DialogComponentProvider implements TaskMonitor {
 	private TaskDialog(Component centerOnComp, String title, boolean isModal, boolean canCancel,
 			boolean hasProgress) {
 		super(title, isModal, true, canCancel, true);
-		this.centerOnComp = centerOnComp;
+		this.centerOnComponent = centerOnComp;
 		this.supportsProgress = hasProgress;
 		setup(canCancel);
 	}
@@ -133,19 +142,6 @@ public class TaskDialog extends DialogComponentProvider implements TaskMonitor {
 		setRememberLocation(false);
 		setRememberSize(false);
 		setTransient(true);
-		closeDialog = () -> {
-			close();
-			dispose();
-		};
-
-		shouldCancelRunnable = () -> {
-			int currentTaskID = taskID.get();
-
-			boolean doCancel = promptToVerifyCancel();
-			if (doCancel && currentTaskID == taskID.get()) {
-				cancel();
-			}
-		};
 
 		mainPanel = new JPanel(new BorderLayout());
 		addWorkPanel(mainPanel);
@@ -161,13 +157,12 @@ public class TaskDialog extends DialogComponentProvider implements TaskMonitor {
 			addCancelButton();
 		}
 
-		// SPLIT the help for this dialog should not be in the front end plugin.
 		setHelpLocation(new HelpLocation(ToolConstants.TOOL_HELP_TOPIC, "TaskDialog"));
 	}
 
 	/**
 	 * Shows a dialog asking the user if they really, really want to cancel the task
-	 * 
+	 *
 	 * @return true if the task should be cancelled
 	 */
 	private boolean promptToVerifyCancel() {
@@ -188,7 +183,7 @@ public class TaskDialog extends DialogComponentProvider implements TaskMonitor {
 	}
 
 	/**
-	 * Adds the panel that contains the activity panel (e.g., the eating bits animation) to the 
+	 * Adds the panel that contains the activity panel (e.g., the eating bits animation) to the
 	 * dialog. This should only be called if the dialog has no need to display progress.
 	 */
 	private void installActivityDisplay() {
@@ -199,22 +194,9 @@ public class TaskDialog extends DialogComponentProvider implements TaskMonitor {
 		});
 	}
 
-	@Override
-	protected void dialogShown() {
-		// our task may have completed while we were queued up to be shown
-		if (isCompleted()) {
-			close();
-		}
-	}
-
-	@Override
-	protected void dialogClosed() {
-		close();
-	}
-
 	@Override
 	protected void cancelCallback() {
-		SwingUtilities.invokeLater(shouldCancelRunnable);
+		Swing.runLater(verifyCancel);
 	}
 
 	@Override
@@ -228,35 +210,49 @@ public class TaskDialog extends DialogComponentProvider implements TaskMonitor {
 		return monitorComponent.isCancelEnabled();
 	}
 
-	public synchronized void taskProcessed() {
+	/**
-		taskDone = true;
+	 * Called after the task has been executed
+	 */
+	public void taskProcessed() {
+		finished.countDown();
 		monitorComponent.notifyChangeListeners();
-		SwingUtilities.invokeLater(closeDialog);
+		Swing.runLater(closeDialog);
-	}
-
-	public synchronized void reset() {
-		taskDone = false;
-		taskID.incrementAndGet();
-	}
-
-	public synchronized boolean isCompleted() {
-		return taskDone;
 	}
 
 	/**
-	 * Shows the dialog window centered on the parent window.
+	 * Returns true if this dialog's task has completed normally or been cancelled
-	 * Dialog display is delayed if delay greater than zero.
+	 * @return true if this dialog's task has completed normally or been cancelled
+	 */
+	public boolean isCompleted() {
+		return finished.getCount() == 0 || isCancelled();
+	}
+
+	/**
+	 * Shows the dialog window centered on the parent window. Dialog display is delayed if delay
+	 * greater than zero.
+	 *
 	 * @param delay number of milliseconds to delay displaying of the task dialog.  If the delay is
 	 * greater than {@link #MAX_DELAY}, then the delay will be {@link #MAX_DELAY};
+	 * @throws IllegalArgumentException if {@code delay} is negative
 	 */
 	public void show(int delay) {
+		if (delay < 0) {
+			throw new IllegalArgumentException("Task Dialog delay cannot be negative");
+		}
 		if (isModal()) {
 			doShowModal(delay);
 		}
 		else {
 			doShowNonModal(delay);
 		}
+	}
 
+	/**
+	 * Returns true if this dialog was ever made visible
+	 * @return true if shown
+	 */
+	public boolean wasShown() {
+		return shown.get();
 	}
 
 	private void doShowModal(int delay) {
@@ -278,7 +274,8 @@ public class TaskDialog extends DialogComponentProvider implements TaskMonitor {
 		// Note: we must not block, as we are not modal.  Clients want control back.  Our job is
 		//       only to show a progress dialog if enough time has elapsed.
 		//
-		GTimer.scheduleRunnable(delay, () -> {
+		int waitTime = Math.min(delay, MAX_DELAY);
+		showTimer = GTimer.scheduleRunnable(waitTime, () -> {
 			if (isCompleted()) {
 				return;
 			}
@@ -289,35 +286,28 @@ public class TaskDialog extends DialogComponentProvider implements TaskMonitor {
 
 	protected void doShow() {
 		Swing.runIfSwingOrRunLater(() -> {
-			DockingWindowManager.showDialog(centerOnComp, TaskDialog.this);
+			if (!isCompleted()) {
+				shown.set(true);
+				DockingWindowManager.showDialog(centerOnComponent, TaskDialog.this);
+			}
 		});
 	}
 
 	private void giveTheTaskThreadAChanceToComplete(int delay) {
 
-		delay = Math.min(delay, MAX_DELAY);
+		int waitTime = Math.min(delay, MAX_DELAY);
-		int elapsedTime = 0;
+		try {
-		while (!isCompleted() && elapsedTime < delay) {
+			finished.await(waitTime, TimeUnit.MILLISECONDS);
-			try {
+		}
-				Thread.sleep(SLEEPY_TIME);
+		catch (InterruptedException e) {
-			}
+			Msg.debug(this, "Interrupted waiting for task '" + getTitle() + "'", e);
-			catch (InterruptedException e) {
-				// don't care; we will try again
-			}
-			elapsedTime += SLEEPY_TIME;
 		}
 	}
 
 	public void dispose() {
-
+		cancel();
-		Runnable disposeTask = () -> {
+		showTimer.cancel();
-			if (showTimer != null) {
+		messageUpdater.dispose();
-				showTimer.stop();
-				showTimer = null;
-			}
-		};
-
-		Swing.runNow(disposeTask);
 	}
 
 //==================================================================================================

Ghidra/Framework/Docking/src/main/java/ghidra/util/task/TaskLauncher.java

@@ -17,8 +17,6 @@ package ghidra.util.task;
 
 import java.awt.Component;
 
-import ghidra.util.Swing;
-
 /**
  * Class to initiate a Task in a new Thread, and to show a progress dialog that indicates
  * activity <b>if the task takes too long</b>.  The progress dialog will show an 
@@ -229,20 +227,6 @@ public class TaskLauncher {
 		return new TaskRunner(task, parent, delayMs, dialogWidth);
 	}
 
-	/**
-	 * Runs the given task in the current thread, which <b>cannot be the Swing thread</b>
-	 * 
-	 * @param task the task to run
-	 * @throws IllegalStateException if the given thread is the Swing thread
-	 */
-	protected void runInThisBackgroundThread(Task task) {
-		if (Swing.isSwingThread()) {
-			throw new IllegalStateException("Must not call this method from the Swing thread");
-		}
-
-		task.monitoredRun(TaskMonitor.DUMMY);
-	}
-
 	private static Component getParent(Component parent) {
 		if (parent == null) {
 			return null;

Ghidra/Framework/Docking/src/main/java/ghidra/util/task/TaskRunner.java

@@ -24,7 +24,7 @@ import generic.util.WindowUtilities;
 import ghidra.util.*;
 
 /**
- * Helper class to launch the given task in a background thread, showing a task dialog if 
+ * Helper class to launch the given task in a background thread, showing a task dialog if
  * this task takes to long. See {@link TaskLauncher}.
  */
 class TaskRunner {
@@ -49,8 +49,7 @@ class TaskRunner {
 		BasicTaskMonitor internalMonitor = new BasicTaskMonitor();
 		WrappingTaskMonitor monitor = new WrappingTaskMonitor(internalMonitor);
 		startTaskThread(monitor);
-
+		showTaskDialog(monitor);
-		Swing.runIfSwingOrRunLater(() -> showTaskDialog(monitor));
 		waitForModalTask();
 	}
 
@@ -71,9 +70,21 @@ class TaskRunner {
 	}
 
 	// protected to allow for dependency injection
-	protected TaskDialog buildTaskDialog(Component comp, TaskMonitor monitor) {
+	protected TaskDialog buildTaskDialog() {
 
-		TaskDialog dialog = createTaskDialog(comp);
+		Component centerOverComponent = parent;
+		Component currentParent = centerOverComponent;
+		if (currentParent != null) {
+			currentParent = WindowUtilities.windowForComponent(parent);
+		}
+
+		if (currentParent == null) {
+			centerOverComponent = null;
+		}
+
+		// we pass in our 'finished' latch here to avoid relying on a Swing.runLater() callback
+		// (see taskFinished())
+		TaskDialog dialog = new TaskDialog(centerOverComponent, task, finished);
 		dialog.setMinimumSize(dialogWidth, 0);
 		dialog.setStatusJustification(task.getStatusTextAlignment());
 		return dialog;
@@ -89,7 +100,6 @@ class TaskRunner {
 		Executor executor = pool.getExecutor();
 		executor.execute(() -> {
 			Thread.currentThread().setName(name);
-
 			try {
 				task.monitoredRun(monitor);
 			}
@@ -99,36 +109,13 @@ class TaskRunner {
 		});
 	}
 
-	private TaskDialog createTaskDialog(Component comp) {
-		Component centerOverComponent = comp;
-		Component currentParent = centerOverComponent;
-		if (currentParent != null) {
-			currentParent = WindowUtilities.windowForComponent(comp);
-		}
-
-		if (currentParent == null) {
-			centerOverComponent = null;
-		}
-
-		return new TaskDialog(centerOverComponent, task) {
-
-			// note: we override this method here to help with the race condition where the 
-			//       TaskRunner does not yet know about the task dialog, but the background
-			//       thread has actually finished the work.
-			@Override
-			public synchronized boolean isCompleted() {
-				return super.isCompleted() || isFinished();
-			}
-		};
-	}
-
 	private void showTaskDialog(WrappingTaskMonitor monitor) {
 
-		Swing.assertSwingThread("Must be on the Swing thread build the Task Dialog");
+		Swing.runIfSwingOrRunLater(() -> {
-
+			taskDialog = buildTaskDialog();
-		taskDialog = buildTaskDialog(parent, monitor);
+			monitor.setDelegate(taskDialog); // initialize the dialog to the current monitor state 
-		monitor.setDelegate(taskDialog); // initialize the dialog to the current state of the monitor
+			taskDialog.show(Math.max(delayMs, 0));
-		taskDialog.show(Math.max(delayMs, 0));
+		});
 	}
 
 	/*testing*/ boolean isFinished() {
@@ -136,9 +123,14 @@ class TaskRunner {
 	}
 
 	private void taskFinished() {
+
+		// This will release the the task dialog.  We passed this latch to the dialog at
+		// construction so that does not block until we notify it in the Swing.runLater() below.
+		// If we only rely on that notification, then the notification will be blocked when the
+		// dialog is waiting in the Swing thread.
 		finished.countDown();
 
-		// Do this later on the Swing thread to handle the race condition where the dialog 
+		// Do this later on the Swing thread to handle the race condition where the dialog
 		// did not exist at the time of this call, but was in the process of being created
 		Swing.runLater(() -> {
 			if (taskDialog != null) {

Ghidra/Framework/Docking/src/test/java/ghidra/util/task/AbstractTaskTest.java

@@ -20,7 +20,6 @@ import static org.junit.Assert.*;
 import java.awt.Component;
 import java.util.Deque;
 import java.util.concurrent.*;
-import java.util.concurrent.atomic.AtomicBoolean;
 
 import docking.test.AbstractDockingTest;
 import ghidra.util.Swing;
@@ -37,8 +36,7 @@ public class AbstractTaskTest extends AbstractDockingTest {
 	protected Deque<TDEvent> eventQueue = new LinkedBlockingDeque<>();
 
 	protected volatile TaskLauncherSpy taskLauncherSpy;
-	protected volatile TaskDialogSpy dialogSpy;
+	protected volatile TaskDialog taskDialog;
-	protected AtomicBoolean didRunInBackground = new AtomicBoolean();
 
 	protected void assertDidNotRunInSwing() {
 		for (TDEvent e : eventQueue) {
@@ -46,11 +44,9 @@ public class AbstractTaskTest extends AbstractDockingTest {
 		}
 	}
 
-	protected void assertRanInSwingThread() {
-		assertFalse("Task was not run in the Swing thread", didRunInBackground.get());
-	}
-
 	protected void assertSwingThreadBlockedForTask() {
+		// if the last event is the swing thread, then we know the task blocked that thread, since
+		// the task delay would have made it run last had it not blocked
 		waitForSwing();
 		TDEvent lastEvent = eventQueue.peekLast();
 		boolean swingIsLast = lastEvent.getThreadName().contains("AWT");
@@ -70,26 +66,35 @@ public class AbstractTaskTest extends AbstractDockingTest {
 	}
 
 	protected void assertNoDialogShown() {
-		if (dialogSpy == null) {
+		if (taskDialog == null) {
 			return; // not shown
 		}
 
 		assertFalse("Dialog should not have been shown.\nEvents: " + eventQueue,
-			dialogSpy.wasShown());
+			taskDialog.wasShown());
 	}
 
 	protected void assertDialogShown() {
-		assertTrue("Dialog should have been shown.\nEvents: " + eventQueue, dialogSpy.wasShown());
+		assertTrue("Dialog should have been shown.\nEvents: " + eventQueue, taskDialog.wasShown());
 	}
 
 	protected void waitForTask() throws Exception {
 		threadsFinished.await(2, TimeUnit.SECONDS);
 	}
 
+	/**
+	 * Launches the task and waits until the dialog is shown
+	 * @param task the task to launch
+	 */
 	protected void launchTask(Task task) {
 		launchTaskFromSwing(task);
 	}
 
+	protected void launchTaskWithoutBlocking(Task task) {
+		runSwing(() -> launchTaskFromSwing(task), false);
+		waitFor(() -> taskDialog != null);
+	}
+
 	protected void launchTaskFromSwing(Task task) {
 
 		runSwing(() -> {
@@ -99,14 +104,30 @@ public class AbstractTaskTest extends AbstractDockingTest {
 		});
 	}
 
+	protected void launchTaskFromSwing(FastModalTask task, int dialogDelay) {
+		runSwing(() -> {
+			taskLauncherSpy = new TaskLauncherSpy(task, dialogDelay);
+			postEvent("After task launcher");
+			threadsFinished.countDown();
+		});
+	}
+
 	protected void postEvent(String message) {
 		eventQueue.add(new TDEvent(message));
 	}
 
+//==================================================================================================
+// Inner Classes
+//==================================================================================================
+
 	protected class TaskLauncherSpy extends TaskLauncher {
 
 		public TaskLauncherSpy(Task task) {
-			super(task, null, DELAY_LAUNCHER);
+			this(task, DELAY_LAUNCHER);
+		}
+
+		public TaskLauncherSpy(Task task, int dialogDelay) {
+			super(task, null, dialogDelay);
 		}
 
 		@Override
@@ -115,31 +136,12 @@ public class AbstractTaskTest extends AbstractDockingTest {
 
 			return new TaskRunner(task, parent, delay, dialogWidth) {
 				@Override
-				protected TaskDialog buildTaskDialog(Component comp, TaskMonitor monitor) {
+				protected TaskDialog buildTaskDialog() {
-					dialogSpy = new TaskDialogSpy(task) {
+					taskDialog = super.buildTaskDialog();
-						@Override
+					return taskDialog;
-						public synchronized boolean isCompleted() {
-							return super.isCompleted() || isFinished();
-						}
-					};
-					return dialogSpy;
 				}
 			};
 		}
-
-		@Override
-		protected void runInThisBackgroundThread(Task task) {
-			didRunInBackground.set(true);
-			super.runInThisBackgroundThread(task);
-		}
-
-		TaskDialogSpy getDialogSpy() {
-			return dialogSpy;
-		}
-
-		boolean didRunInBackground() {
-			return didRunInBackground.get();
-		}
 	}
 
 	protected class FastModalTask extends Task {
@@ -203,6 +205,25 @@ public class AbstractTaskTest extends AbstractDockingTest {
 		}
 	}
 
+	protected class LatchedModalTask extends Task {
+
+		private CountDownLatch latch;
+
+		public LatchedModalTask(CountDownLatch latch) {
+			super("Latched Modal Task", true, true, true);
+			this.latch = latch;
+		}
+
+		@Override
+		public void run(TaskMonitor monitor) {
+			postEvent(getName() + " started...");
+			waitFor(latch);
+			sleep(DELAY_SLOW);
+			threadsFinished.countDown();
+			postEvent(getName() + " finished.");
+		}
+	}
+
 	protected class TDEvent {
 
 		protected String threadName = Thread.currentThread().getName();

Ghidra/Framework/Docking/src/test/java/ghidra/util/task/TaskDialogSpy.java

@@ -1,36 +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.util.task;
-
-import java.util.concurrent.atomic.AtomicBoolean;
-
-public class TaskDialogSpy extends TaskDialog {
-	private AtomicBoolean shown = new AtomicBoolean();
-
-	public TaskDialogSpy(Task task) {
-		super(task);
-	}
-
-	@Override
-	protected void doShow() {
-		shown.set(true);
-		super.doShow();
-	}
-
-	boolean wasShown() {
-		return shown.get();
-	}
-}

Ghidra/Framework/Docking/src/test/java/ghidra/util/task/TaskDialogTest.java

@@ -17,6 +17,8 @@ package ghidra.util.task;
 
 import static org.junit.Assert.*;
 
+import java.util.concurrent.CountDownLatch;
+
 import org.junit.After;
 import org.junit.Test;
 
@@ -51,7 +53,7 @@ public class TaskDialogTest extends AbstractTaskTest {
 
 		waitForTask();
 
-		assertFalse(dialogSpy.wasShown());
+		assertFalse(taskDialog.wasShown());
 		assertSwingThreadBlockedForTask();
 	}
 
@@ -76,7 +78,7 @@ public class TaskDialogTest extends AbstractTaskTest {
 
 		waitForTask();
 
-		assertFalse(dialogSpy.wasShown());
+		assertFalse(taskDialog.wasShown());
 		assertNoDialogShown();
 	}
 
@@ -98,16 +100,16 @@ public class TaskDialogTest extends AbstractTaskTest {
 	 */
 	@Test
 	public void testTaskCancel() throws Exception {
-		SlowModalTask task = new SlowModalTask();
+		CountDownLatch latch = new CountDownLatch(1);
-		launchTask(task);
+		LatchedModalTask task = new LatchedModalTask(latch);
+		launchTaskWithoutBlocking(task);
 
-		dialogSpy.doShow();
+		taskDialog.doShow();
+		latch.countDown();
 
-		waitForTask();
+		assertFalse(taskDialog.isCancelled());
-
+		taskDialog.cancel();
-		assertFalse(dialogSpy.isCancelled());
+		assertTrue(taskDialog.isCancelled());
-		dialogSpy.cancel();
-		assertTrue(dialogSpy.isCancelled());
 	}
 
 	/*
@@ -119,11 +121,11 @@ public class TaskDialogTest extends AbstractTaskTest {
 		SlowModalTask task = new SlowModalTask();
 		launchTask(task);
 
-		dialogSpy.doShow();
+		taskDialog.doShow();
-		dialogSpy.setCancelEnabled(false);
+		taskDialog.setCancelEnabled(false);
 
 		waitForTask();
 
-		assertFalse(dialogSpy.isCancelEnabled());
+		assertFalse(taskDialog.isCancelEnabled());
 	}
 }

Ghidra/Framework/Docking/src/test/java/ghidra/util/task/TaskLauncherTest.java

@@ -52,7 +52,7 @@ public class TaskLauncherTest extends AbstractTaskTest {
 		FastModalTask task = new FastModalTask();
 		launchTaskFromBackground(task);
 		waitForTask();
-		assertRanInSwingThread();
+		assertDidNotRunInSwing();
 	}
 
 	@Test
@@ -93,11 +93,36 @@ public class TaskLauncherTest extends AbstractTaskTest {
 		assertDidNotRunInSwing();
 	}
 
+	@Test
+	public void testLaunchFromSwingThreadWithModalTaskDoesNotBlockForFullDelay() throws Exception {
+
+		//
+		// Tests that a short-lived task does not block for the full dialog delay
+		//
+
+		FastModalTask task = new FastModalTask();
+		int dialogDelay = 3000;
+		long start = System.nanoTime();
+		launchTaskFromSwing(task, dialogDelay);
+		waitForTask();
+		long end = System.nanoTime();
+		long totalTime = TimeUnit.NANOSECONDS.toMillis(end - start);
+
+		assertSwingThreadBlockedForTask();
+		assertTrue(
+			"Time waited is longer that the dialog delay: " + totalTime + " vs " + dialogDelay,
+			totalTime < dialogDelay);
+	}
+
+//==================================================================================================
+// Private Methods
+//==================================================================================================
+
 	private int getWaitTimeoutInSeconds() {
 		return (int) TimeUnit.SECONDS.convert(DEFAULT_WAIT_TIMEOUT, TimeUnit.MILLISECONDS) * 2;
 	}
 
-	protected void launchTaskFromBackground(Task task) throws InterruptedException {
+	private void launchTaskFromBackground(Task task) throws InterruptedException {
 
 		CountDownLatch start = new CountDownLatch(1);
 		new Thread("Test Task Launcher Background Client") {
@@ -114,7 +139,7 @@ public class TaskLauncherTest extends AbstractTaskTest {
 			start.await(getWaitTimeoutInSeconds(), TimeUnit.SECONDS));
 	}
 
-	protected void launchTaskFromTask() throws InterruptedException {
+	private void launchTaskFromTask() throws InterruptedException {
 
 		TaskLaunchingTask task = new TaskLaunchingTask();
 
@@ -133,6 +158,10 @@ public class TaskLauncherTest extends AbstractTaskTest {
 			start.await(getWaitTimeoutInSeconds(), TimeUnit.SECONDS));
 	}
 
+//==================================================================================================
+// Inner Classes
+//==================================================================================================
+
 	private class TaskLaunchingTask extends Task {
 
 		public TaskLaunchingTask() {

Ghidra/Framework/Generic/src/main/java/ghidra/util/timer/GTimer.java

@@ -1,6 +1,5 @@
 /* ###
  * IP: GHIDRA
- * REVIEWED: YES
  *
  * Licensed under the Apache License, Version 2.0 (the "License");
  * you may not use this file except in compliance with the License.
@@ -16,14 +15,14 @@
  */
 package ghidra.util.timer;
 
-import ghidra.util.Msg;
-
 import java.util.Timer;
 import java.util.TimerTask;
 
+import ghidra.util.Msg;
+
 public class GTimer {
 	private static Timer timer;
-	private static GTimerMonitor DO_NOTHING_MONITOR = new DoNothingMonitor();
+	private static GTimerMonitor DO_NOTHING_MONITOR = GTimerMonitor.DUMMY;
 
 	/**
 	 * Schedules a runnable for execution after the specified delay.
@@ -48,7 +47,8 @@ public class GTimer {
 	 * @param callback the runnable to be executed.
 	 * @return a GTimerMonitor which allows the caller to cancel the timer and check its status.
 	 */
-	public static GTimerMonitor scheduleRepeatingRunnable(long delay, long period, Runnable callback) {
+	public static GTimerMonitor scheduleRepeatingRunnable(long delay, long period,
+			Runnable callback) {
 		GTimerTask gTimerTask = new GTimerTask(callback);
 		getTimer().schedule(gTimerTask, delay, period);
 		return gTimerTask;
@@ -61,24 +61,6 @@ public class GTimer {
 		return timer;
 	}
 
-	static class DoNothingMonitor implements GTimerMonitor {
-		@Override
-		public boolean cancel() {
-			return false;
-		}
-
-		@Override
-		public boolean didRun() {
-			return false;
-		}
-
-		@Override
-		public boolean wasCancelled() {
-			return false;
-		}
-
-	}
-
 	static class GTimerTask extends TimerTask implements GTimerMonitor {
 
 		private final Runnable runnable;

Ghidra/Framework/Generic/src/main/java/ghidra/util/timer/GTimerMonitor.java

@@ -1,6 +1,5 @@
 /* ###
  * IP: GHIDRA
- * REVIEWED: YES
  *
  * Licensed under the Apache License, Version 2.0 (the "License");
  * you may not use this file except in compliance with the License.
@@ -17,10 +16,32 @@
 package ghidra.util.timer;
 
 /**
- * Monitor object returned from a GTimer.schedule() call 
+ * Monitor object returned from a GTimer.schedule() call
- *
  */
 public interface GTimerMonitor {
+
+	/**
+	 * A dummy implementation of this interface
+	 */
+	public static GTimerMonitor DUMMY =
+		new GTimerMonitor() {
+
+			@Override
+			public boolean wasCancelled() {
+				return false;
+			}
+
+			@Override
+			public boolean didRun() {
+				return false;
+			}
+
+			@Override
+			public boolean cancel() {
+				return false;
+			}
+		};
+
 	/**
 	 * Cancels the scheduled runnable associated with this GTimerMonitor if it has not already run.
 	 * @return true if the scheduled runnable was cancelled before it had a chance to execute.
@@ -35,7 +56,7 @@ public interface GTimerMonitor {
 
 	/**
 	 * Return true if the scheduled runnable was cancelled before it had a chance to run.
-	 * @return
+	 * @return true if the scheduled runnable was cancelled before it had a chance to run.
 	 */
 	public boolean wasCancelled();
 }

Ghidra/Framework/Project/src/main/java/ghidra/framework/plugintool/mgr/ToolTaskManager.java

@@ -50,7 +50,7 @@ public class ToolTaskManager implements Runnable {
 
 	/**
 	 * Construct a new ToolTaskManager.
-	 * 
+	 *
 	 * @param tool tool associated with this ToolTaskManager
 	 */
 	public ToolTaskManager(PluginTool tool) {
@@ -64,7 +64,7 @@ public class ToolTaskManager implements Runnable {
 	/**
 	 * Returns the thread group associated with all background tasks run by this
 	 * manager and their instantiated threads.
-	 * 
+	 *
 	 * @return task thread group
 	 */
 	public ThreadGroup getTaskThreadGroup() {
@@ -73,7 +73,7 @@ public class ToolTaskManager implements Runnable {
 
 	/**
 	 * Get the monitor component that shows progress and has a cancel button.
-	 * 
+	 *
 	 * @return the monitor component
 	 */
 	public JComponent getMonitorComponent() {
@@ -82,7 +82,7 @@ public class ToolTaskManager implements Runnable {
 
 	/**
 	 * Return true if a task is executing
-	 * 
+	 *
 	 * @return true if a task is executing
 	 */
 	public synchronized boolean isBusy() {
@@ -91,11 +91,11 @@ public class ToolTaskManager implements Runnable {
 
 	/**
 	 * Execute the given command in the foreground
-	 * 
+	 *
 	 * @param cmd command to execute
 	 * @param obj domain object to which the command will be applied
 	 * @return the completion status of the command
-	 * 
+	 *
 	 * @see Command#applyTo(DomainObject)
 	 */
 	public boolean execute(Command cmd, DomainObject obj) {
@@ -141,7 +141,7 @@ public class ToolTaskManager implements Runnable {
 			try {
 				success = cmd.applyTo(domainObject);
 
-				// TODO Ok, this seems bad--why track the success of the given command, but 
+				// TODO Ok, this seems bad--why track the success of the given command, but
 				// not any of the queued commands?  (Are they considered unrelated follow-up
 				// commands?)
 				executeQueueCommands(domainObject, cmdName);
@@ -174,7 +174,7 @@ public class ToolTaskManager implements Runnable {
 
 	/**
 	 * Execute the given command in the background
-	 * 
+	 *
 	 * @param cmd background command
 	 * @param obj domain object that supports undo/redo
 	 */
@@ -207,7 +207,7 @@ public class ToolTaskManager implements Runnable {
 
 	/**
 	 * Schedule the given background command when the current command completes.
-	 * 
+	 *
 	 * @param cmd background command to be scheduled
 	 * @param obj domain object that supports undo/redo
 	 */
@@ -239,7 +239,7 @@ public class ToolTaskManager implements Runnable {
 			// any queued task will process queued follow-on commands
 			return true;
 		}
-		// NOTE: while current task may not have completed (not null) it may be 
+		// NOTE: while current task may not have completed (not null) it may be
 		// done processing queued commands
 		return currentTask != null && !currentTask.isDoneQueueProcessing();
 	}
@@ -265,7 +265,7 @@ public class ToolTaskManager implements Runnable {
 	/**
 	 * Cancel the currently running task and clear all commands that are
 	 * scheduled to run. Block until the currently running task ends.
-	 * 
+	 *
 	 * @param wait if true wait for current task to cancel cleanly
 	 */
 	public void stop(boolean wait) {
@@ -346,7 +346,7 @@ public class ToolTaskManager implements Runnable {
 	/**
 	 * Notification from the BackgroundCommandTask that it has completed; queued
 	 * or scheduled commands are executed.
-	 * 
+	 *
 	 * @param obj domain object that supports undo/redo
 	 * @param task background command task that has completed
 	 * @param monitor task monitor
@@ -424,7 +424,7 @@ public class ToolTaskManager implements Runnable {
 
 	/**
 	 * Clear all tasks associated with specified domain object.
-	 * 
+	 *
 	 * @param obj domain object
 	 */
 	public synchronized void clearTasks(UndoableDomainObject obj) {
@@ -440,7 +440,7 @@ public class ToolTaskManager implements Runnable {
 	/**
 	 * Notification from the BackgroundCommandTask that the given command
 	 * failed. Any scheduled commands are cleared from the queue.
-	 * 
+	 *
 	 * @param obj domain object that supports undo/redo
 	 * @param taskCmd background command that failed
 	 * @param monitor task monitor for the background task
@@ -509,7 +509,6 @@ public class ToolTaskManager implements Runnable {
 
 		toolTaskMonitor.dispose();
 		if (modalTaskDialog != null) {
-			modalTaskDialog.cancel();
 			modalTaskDialog.dispose();
 		}
 
@@ -627,8 +626,8 @@ class ToolTaskMonitor extends TaskMonitorComponent implements TaskListener {
 	public Dimension getPreferredSize() {
 		Dimension preferredSize = super.getPreferredSize();
 
-		// Somewhat arbitrary value, but the default is too small to read most messages. So, 
+		// Somewhat arbitrary value, but the default is too small to read most messages. So,
-		// give some extra width, but not so much as to too badly push off the status area of 
+		// give some extra width, but not so much as to too badly push off the status area of
 		// the tool window.  This value is based upon some of the longer messages that we use.
 		preferredSize.width += 200;
 		return preferredSize;