GP-443 - Tasks - Fixed potential unnecessary forced wait when using the

TaskLauncher

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;
@@ -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);
@@ -811,13 +818,13 @@ 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

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,19 +17,21 @@ 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
@@ -48,42 +50,49 @@ import ghidra.util.timer.GTimer;
  */
 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;
 	}
 
 	/**
@@ -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,7 +157,6 @@ 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"));
 	}
 
@@ -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

@@ -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,6 +123,11 @@ 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

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.
@@ -18,9 +17,31 @@ package ghidra.util.timer;
 
 /**
  * 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

@@ -509,7 +509,6 @@ public class ToolTaskManager implements Runnable {
 
 		toolTaskMonitor.dispose();
 		if (modalTaskDialog != null) {
-			modalTaskDialog.cancel();
 			modalTaskDialog.dispose();
 		}