GP-249 - Task Dialog - fixed issue with task dialog appearing over user input dialog; fixed spin/sleep code

Ghidra/Features/PDB/src/main/java/pdb/LoadPdbTask.java

@@ -47,7 +47,7 @@ class LoadPdbTask extends Task {
 
 	LoadPdbTask(Program program, File pdbFile, boolean useMsDiaParser,
 			PdbApplicatorRestrictions restrictions, DataTypeManagerService service) {
-		super("Loading PDB...", true, false, false);
+		super("Load PDB", true, false, false);
 		this.program = program;
 		this.pdbFile = pdbFile;
 		this.useMsDiaParser = useMsDiaParser;
@@ -88,12 +88,9 @@ class LoadPdbTask extends Task {
 			}
 		};
 
-
-
 		try {
 			AutoAnalysisManager.getAnalysisManager(program)
-					.scheduleWorker(worker, null, true,
+					.scheduleWorker(worker, null, true, monitor);
-						monitor);
 			if (log.hasMessages()) {
 				MultiLineMessageDialog dialog = new MultiLineMessageDialog("Load PDB File",
 					"There were warnings/errors loading the PDB file.", log.toString(),
@@ -151,9 +148,8 @@ class LoadPdbTask extends Task {
 
 		pdbApplicatorOptions.setRestrictions(restrictions);
 
-		try (AbstractPdb pdb =
+		try (AbstractPdb pdb = ghidra.app.util.bin.format.pdb2.pdbreader.PdbParser
-			ghidra.app.util.bin.format.pdb2.pdbreader.PdbParser.parse(pdbFile.getAbsolutePath(),
+				.parse(pdbFile.getAbsolutePath(), pdbReaderOptions, monitor)) {
-				pdbReaderOptions, monitor)) {
 			monitor.setMessage("PDB: Parsing " + pdbFile + "...");
 			pdb.deserialize(monitor);
 			PdbApplicator applicator = new PdbApplicator(pdbFile.getAbsolutePath(), pdb);
@@ -166,7 +162,7 @@ class LoadPdbTask extends Task {
 		}
 		return false;
 	}
-	
+
 	private void analyzeSymbols(TaskMonitor monitor, MessageLog log) {
 
 		MicrosoftDemanglerAnalyzer demanglerAnalyzer = new MicrosoftDemanglerAnalyzer();

Ghidra/Features/PDB/src/main/java/pdb/PdbPlugin.java

@@ -20,6 +20,7 @@ import java.io.File;
 import docking.action.MenuData;
 import docking.widgets.OptionDialog;
 import docking.widgets.filechooser.GhidraFileChooser;
+import docking.widgets.filechooser.GhidraFileChooserMode;
 import ghidra.app.CorePluginPackage;
 import ghidra.app.context.ProgramActionContext;
 import ghidra.app.context.ProgramContextAction;
@@ -131,11 +132,14 @@ public class PdbPlugin extends Plugin {
 				return;
 			}
 
-			TaskLauncher
+			// note: We intentionally use a 0-delay here.  Our underlying task may show modal
-					.launch(new LoadPdbTask(program, pdb, useMsDiaParser, restrictions, service));
+			//       dialog prompts.  We want the task progress dialog to be showing before any
+			//       promts appear.
+			LoadPdbTask task = new LoadPdbTask(program, pdb, useMsDiaParser, restrictions, service);
+			new TaskLauncher(task, null, 0);
 		}
 		catch (Exception pe) {
-			Msg.showError(getClass(), null, "Error", pe.getMessage());
+			Msg.showError(getClass(), null, "Error Loading PDB", pe.getMessage(), pe);
 		}
 	}
 
@@ -145,7 +149,7 @@ public class PdbPlugin extends Plugin {
 			pdbChooser = new GhidraFileChooser(tool.getToolFrame());
 			pdbChooser.setTitle("Select PDB file to load:");
 			pdbChooser.setApproveButtonText("Select PDB");
-			pdbChooser.setFileSelectionMode(GhidraFileChooser.FILES_ONLY);
+			pdbChooser.setFileSelectionMode(GhidraFileChooserMode.FILES_ONLY);
 			pdbChooser.setFileFilter(new ExtensionFileFilter(new String[] { "pdb", "xml" },
 				"Program Database Files and PDB XML Representations"));
 		}

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

@@ -1757,7 +1757,7 @@ public class DockingWindowManager implements PropertyChangeListener, Placeholder
 		 	This method seeks to accomplish 2 goals:
 		 		1) find a suitable component over which to center, and
 		 		2) ensure that the chosen component is in the parent hierarchy
-		
+
 		 */
 		Component bestComponent = centeredOnComponent;
 		if (SwingUtilities.isDescendingFrom(parent, bestComponent)) {
@@ -1791,7 +1791,7 @@ public class DockingWindowManager implements PropertyChangeListener, Placeholder
 
 		/*
 		 	Note: Which window should be the parent of the dialog when the user does not specify?
-		
+
 		 	Some use cases; a dialog is shown from:
 		 		1) A toolbar action
 		 		2) A component provider's code
@@ -1799,7 +1799,7 @@ public class DockingWindowManager implements PropertyChangeListener, Placeholder
 		 		4) A background thread
 		 		5) The help window
 		 		6) A modal password dialog appears over the splash screen
-		
+
 		 	It seems like the parent should be the active window for 1-2.
 		 	Case 3 should probably use the window of the dialog provider.
 		 	Case 4 should probably use the main tool frame, since the user may be
@@ -1807,12 +1807,12 @@ public class DockingWindowManager implements PropertyChangeListener, Placeholder
 		 	active window, we can default to the tool's frame.
 		 	Case 5 should use the help window.
 		 	Case 6 should use the splash screen as the parent.
-		
+
 		 	We have not yet solidified how we should parent.  This documentation is meant to
 		 	move us towards clarity as we find Use Cases that don't make sense.  (Once we
 		 	finalize our understanding, we should update the javadoc to list exactly where
 		 	the given Dialog Component will be shown.)
-		
+
 		 	Use Case
 		 		A -The user presses an action on a toolbar from a window on screen 1, while the
 		 		   main tool frame is on screen 2.  We want the popup window to appear on screen
@@ -1824,8 +1824,8 @@ public class DockingWindowManager implements PropertyChangeListener, Placeholder
 		 		 	-modal - Java handles this correctly, allowing the new dialog to be used
 		 		 	-non-modal - Java prevents the non-modal from being editing if not parented
 		 		 	             correctly
-		
+
-		
+
 		 	For now, the easiest mental model to use is to always prefer the active window so
 		 	that a dialog will appear in the user's view.  If we find a case where this is
 		 	not desired, then document it here.

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

@@ -17,65 +17,70 @@ 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.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;
+	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)));
 
-	/** 
+	/**
 	 * Constructor
-	 * 
+	 *
 	 * @param centerOnComp component to be centered over when shown,
 	 * otherwise center over parent.  If both centerOnComp and parent
 	 * are null, dialog will be centered on screen.
@@ -88,7 +93,7 @@ public class TaskDialog extends DialogComponentProvider implements TaskMonitor {
 
 	/**
 	 * Constructor
-	 *  
+	 *
 	 * @param task the Task that this dialog will be associated with
 	 */
 	public TaskDialog(Task task) {
@@ -97,7 +102,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 +114,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 +125,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 +138,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 +153,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 +179,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 +190,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,19 +206,17 @@ public class TaskDialog extends DialogComponentProvider implements TaskMonitor {
 		return monitorComponent.isCancelEnabled();
 	}
 
+	/**
+	 * Called after the task has been executed
+	 */
 	public synchronized void taskProcessed() {
-		taskDone = true;
+		finished.countDown();
 		monitorComponent.notifyChangeListeners();
-		SwingUtilities.invokeLater(closeDialog);
+		Swing.runLater(closeDialog);
-	}
-
-	public synchronized void reset() {
-		taskDone = false;
-		taskID.incrementAndGet();
 	}
 
 	public synchronized boolean isCompleted() {
-		return taskDone;
+		return finished.getCount() == 0;
 	}
 
 	/**
@@ -256,7 +232,6 @@ public class TaskDialog extends DialogComponentProvider implements TaskMonitor {
 		else {
 			doShowNonModal(delay);
 		}
-
 	}
 
 	private void doShowModal(int delay) {
@@ -278,7 +253,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;
 			}
@@ -288,31 +264,32 @@ public class TaskDialog extends DialogComponentProvider implements TaskMonitor {
 	}
 
 	protected void doShow() {
+
 		Swing.runIfSwingOrRunLater(() -> {
-			DockingWindowManager.showDialog(centerOnComp, TaskDialog.this);
+			if (!isCompleted()) {
+				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() {
 
+		messageUpdater.dispose();
+
 		Runnable disposeTask = () -> {
 			if (showTimer != null) {
-				showTimer.stop();
+				showTimer.cancel();
 				showTimer = 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();
 	}
 
@@ -112,7 +111,7 @@ class TaskRunner {
 
 		return new TaskDialog(centerOverComponent, task) {
 
-			// note: we override this method here to help with the race condition where the 
+			// 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
@@ -124,11 +123,11 @@ class TaskRunner {
 
 	private void showTaskDialog(WrappingTaskMonitor monitor) {
 
-		Swing.assertSwingThread("Must be on the Swing thread build the Task Dialog");
+		Swing.runIfSwingOrRunLater(() -> {
-
+			taskDialog = buildTaskDialog(parent, monitor);
-		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() {
@@ -138,7 +137,7 @@ class TaskRunner {
 	private void taskFinished() {
 		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/Generic/src/main/java/generic/cache/CachingPool.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,51 +15,54 @@
  */
 package generic.cache;
 
+import java.util.*;
+
 import ghidra.util.timer.GTimer;
 import ghidra.util.timer.GTimerMonitor;
 
-import java.util.ArrayDeque;
-import java.util.Deque;
-import java.util.concurrent.atomic.AtomicBoolean;
-
 /**
- * A thread-safe pool class that knows how to create instances as needed.  When clients are done
+ * A thread-safe pool that knows how to create instances as needed.  When clients are done
- * with the pooled item they then call {@link  #release(Object)}.
+ * with the pooled item they then call {@link  #release(Object)}, thus enabling them to be
+ * re-used in the future.
+ *
+ * <p>Calling {@link #setCleanupTimeout(long)} with a non-negative value will start a timer when
+ * {@link #release(Object)} is called to {@link BasicFactory#dispose(Object)} any objects in the
+ * pool.   By default, the cleanup timer does not run.
+ *
+ * <p>Once {@link #dispose()} has been called on this class, items created or released will no
+ * longer be pooled.
  *
  * @param <T> the type of object to pool
  */
 public class CachingPool<T> {
 
-	private static final long TIMEOUT = 0;
+	// Use -1 to signal the cleanup timer should not be used
+	private static final long TIMEOUT = -1;
 
-	private AtomicBoolean isDisposed = new AtomicBoolean(false);
+	private boolean isDisposed;
 	private BasicFactory<T> factory;
 	private Deque<T> cache = new ArrayDeque<T>();
 
 	private long disposeTimeout = TIMEOUT;
 	private GTimerMonitor timerMonitor;
-	private Runnable cleanupRunnable = new Runnable() {
-		@Override
-		public void run() {
-			synchronized (CachingPool.this) {
-				for (T t : cache) {
-					factory.dispose(t);
-				}
-			}
-		}
-	};
 
+	/**
+	 * Creates a new pool that uses the given factory to create new items as needed
+	 *
+	 * @param factory the factory used to create new items
+	 */
 	public CachingPool(BasicFactory<T> factory) {
-		if (factory == null) {
+		this.factory = Objects.requireNonNull(factory);
-			throw new IllegalArgumentException("factory cannot be null");
-		}
-		this.factory = factory;
 	}
 
 	/**
-	 * Sets the time to wait for released items to be automatically disposed.  The 
+	 * Sets the time to wait for released items to be disposed by this pool by calling
-	 * default is {@link #TIMEOUT}.
+	 * {@link BasicFactory#dispose(Object)}.  A negative timeout value signals to disable
-	 * 
+	 * the cleanup task.
+	 *
+	 * <p>When clients call {@link #get()}, the timer will not be running.  It will be restarted
+	 * again once {@link #release(Object)} has been called.
+	 *
 	 * @param timeout the new timeout.
 	 */
 	public void setCleanupTimeout(long timeout) {
@@ -68,47 +70,60 @@ public class CachingPool<T> {
 	}
 
 	/**
-	 * Returns a cached or new {@link T}
+	 * Returns a cached or new {@code T}
-	 * 
+	 *
-	 * @return a cached or new {@link T}
+	 * @return a cached or new {@code T}
 	 * @throws Exception if there is a problem instantiating a new instance
 	 */
 	public synchronized T get() throws Exception {
-		cancel();
+		stopCleanupTimer();
-		if (cache.isEmpty()) {
+		if (cache.isEmpty() || isDisposed) {
 			return factory.create();
 		}
 		return cache.pop();
 	}
 
+	/**
+	 * Signals that the given object is no longer being used.  The object will be placed back into
+	 * the pool until it is disposed via the cleanup timer, if it is running.
+	 * @param t the item to release
+	 */
 	public synchronized void release(T t) {
-		restart();
+		restartCleanupTimer();
-		if (isDisposed.get()) {
+		if (isDisposed) {
 			factory.dispose(t);
 			return;
 		}
 		cache.push(t);
 	}
 
+	/**
+	 * Triggers all pooled object to be disposed via this pool's factory.   Future calls to
+	 * {@link #get()} will still create new objects, but the internal cache will no longer be used.
+	 */
 	public synchronized void dispose() {
-		cancel();
+		stopCleanupTimer();
-		isDisposed.set(true);
+		isDisposed = true;
 
+		disposeCachedItems();
+	}
+
+	private synchronized void disposeCachedItems() {
 		for (T t : cache) {
 			factory.dispose(t);
 		}
 	}
 
-	private void cancel() {
+	private void stopCleanupTimer() {
 		if (timerMonitor != null) {
 			timerMonitor.cancel();
 		}
 	}
 
-	private void restart() {
+	private void restartCleanupTimer() {
 		if (timerMonitor != null) {
 			timerMonitor.cancel();
 		}
-		timerMonitor = GTimer.scheduleRunnable(disposeTimeout, cleanupRunnable);
+		timerMonitor = GTimer.scheduleRunnable(disposeTimeout, this::disposeCachedItems);
 	}
 }

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,23 +15,32 @@
  */
 package ghidra.util.timer;
 
-import ghidra.util.Msg;
-
 import java.util.Timer;
 import java.util.TimerTask;
 
+import ghidra.util.Msg;
+
+/**
+ * A class to schedule {@link Runnable}s to run after some delay, optionally repeating.  This class
+ * uses a {@link Timer} internally to schedule work.   Clients of this class are given a monitor
+ * that allows them to check on the state of the runnable, as well as to cancel the runnable.
+ */
 public class GTimer {
 	private static Timer timer;
 	private static GTimerMonitor DO_NOTHING_MONITOR = new DoNothingMonitor();
 
 	/**
-	 * Schedules a runnable for execution after the specified delay.
+	 * Schedules a runnable for execution after the specified delay.   A delay value less than 0
-	 * @param delay the time (in milliseconds) to wait before executing the runnable.
+	 * will cause this timer to schedule nothing.  This allows clients to use this timer class
+	 * with no added logic for managing timer enablement.
+	 *
+	 * @param delay the time (in milliseconds) to wait before executing the runnable.  A negative
+	 *        value signals not to run the timer--the callback will not be executed
 	 * @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 scheduleRunnable(long delay, Runnable callback) {
-		if (delay <= 0) {
+		if (delay < 0) {
 			return DO_NOTHING_MONITOR;
 		}
 		GTimerTask gTimerTask = new GTimerTask(callback);
@@ -41,14 +49,22 @@ public class GTimer {
 	}
 
 	/**
-	 * Schedules a runnable for <b>repeated</b> execution after the specified delay.  
+	 * Schedules a runnable for <b>repeated</b> execution after the specified delay. A delay value
-	 * 
+	 * less than 0 will cause this timer to schedule nothing.  This allows clients to use this
-	 * @param delay the time (in milliseconds) to wait before executing the runnable.
+	 * timer class with no added logic for managing timer enablement.
-	 * @param period time in milliseconds between successive runnable executions.
+	 *
-	 * @param callback the runnable to be executed.
+	 * @param delay the time (in milliseconds) to wait before executing the runnable.   A negative
-	 * @return a GTimerMonitor which allows the caller to cancel the timer and check its status.
+	 *        value signals not to run the timer--the callback will not be executed
+	 * @param period time in milliseconds between successive runnable executions
+	 * @param callback the runnable to be executed
+	 * @return a GTimerMonitor which allows the caller to cancel the timer and check its status
+	 * @throws IllegalArgumentException if {@code period <= 0}
 	 */
-	public static GTimerMonitor scheduleRepeatingRunnable(long delay, long period, Runnable callback) {
+	public static GTimerMonitor scheduleRepeatingRunnable(long delay, long period,
+			Runnable callback) {
+		if (delay < 0) {
+			return DO_NOTHING_MONITOR;
+		}
 		GTimerTask gTimerTask = new GTimerTask(callback);
 		getTimer().schedule(gTimerTask, delay, period);
 		return gTimerTask;

Ghidra/Framework/Project/src/main/java/ghidra/framework/data/LockingTaskMonitor.java

@@ -34,12 +34,13 @@ class LockingTaskMonitor implements TaskMonitor {
 	private MyTaskDialog taskDialog;
 
 	/**
-	 * Constructs a locking task handler for a locked dobj.  The setCompleted() method must be
+	 * Constructs a locking task handler for a locked domain object.  The
-	 * invoked to dispose this object and release the lock.  This should
+	 * {@link #releaseLock()} method must be invoked to dispose this object and release the
-	 * be done in a try/finally block to avoid accidentally locking the
+	 * lock.  This should be done in a try/finally block to avoid accidentally locking the
 	 * domain object indefinitely.
+	 *
 	 * @param dobj domain object
-	 * @param hasProgress true if this monitorhas progress
+	 * @param hasProgress true if this monitor has progress
 	 * @param title task title
 	 */
 	LockingTaskMonitor(DomainObjectAdapterDB dobj, boolean hasProgress, String title) {
@@ -84,9 +85,6 @@ class LockingTaskMonitor implements TaskMonitor {
 		}
 	}
 
-	/*
-	 * @see ghidra.util.task.TaskMonitor#isCancelled()
-	 */
 	@Override
 	public synchronized boolean isCancelled() {
 		return taskDialog != null ? taskDialog.isCancelled() : isCanceled;
@@ -108,9 +106,6 @@ class LockingTaskMonitor implements TaskMonitor {
 		notifyAll();
 	}
 
-	/*
-	 * @see ghidra.util.task.TaskMonitor#setMessage(java.lang.String)
-	 */
 	@Override
 	public synchronized void setMessage(String msg) {
 		this.msg = msg;
@@ -124,9 +119,6 @@ class LockingTaskMonitor implements TaskMonitor {
 		return msg;
 	}
 
-	/*
-	 * @see ghidra.util.task.TaskMonitor#setProgress(int)
-	 */
 	@Override
 	public synchronized void setProgress(long value) {
 		this.curProgress = value;
@@ -175,9 +167,6 @@ class LockingTaskMonitor implements TaskMonitor {
 		return indeterminate;
 	}
 
-	/*
-	 * @see ghidra.util.task.TaskMonitor#setCancelEnabled(boolean)
-	 */
 	@Override
 	public synchronized void setCancelEnabled(boolean enable) {
 		this.cancelEnabled = enable;
@@ -186,17 +175,11 @@ class LockingTaskMonitor implements TaskMonitor {
 		}
 	}
 
-	/**
-	 * @see ghidra.util.task.TaskMonitor#isCancelEnabled()
-	 */
 	@Override
 	public synchronized boolean isCancelEnabled() {
 		return taskDialog != null ? taskDialog.isCancelEnabled() : cancelEnabled;
 	}
 
-	/*
-	 * @see ghidra.util.task.TaskMonitor#cancel()
-	 */
 	@Override
 	public synchronized void cancel() {
 		this.isCanceled = true;
@@ -205,9 +188,6 @@ class LockingTaskMonitor implements TaskMonitor {
 		}
 	}
 
-	/*
-	 * @see ghidra.util.task.TaskMonitor#clearCanceled()
-	 */
 	@Override
 	public void clearCanceled() {
 		this.isCanceled = false;