Threaded Tables - fixed timing bug that caused a 'pending job' to never

get run if the thread finished with a new pending job being scheduled
before the 'notify finished' happened

Ghidra/Framework/Docking/src/main/java/docking/widgets/table/threaded/TableUpdateJob.java

@@ -204,7 +204,8 @@ public class TableUpdateJob<T> {
 	 * @return true if the sort can be processed by this job, false if this job is essentially already
 	 * completed and therefor cannot perform the sort job.
 	 */
-	public synchronized boolean requestSort(TableSortingContext<T> newSortingContext, boolean forceSort) {
+	public synchronized boolean requestSort(TableSortingContext<T> newSortingContext,
+			boolean forceSort) {
 		if (currentState == DONE) {
 			return false;
 		}
@@ -688,7 +689,7 @@ public class TableUpdateJob<T> {
 
 	@Override
 	public String toString() {
-		return "Job - [state history=\n" + getStateHistoryString() + "]";
+		return getClass().getSimpleName() + " - [state history=\n" + getStateHistoryString() + "]";
 	}
 
 	private String getStateHistoryString() {

Ghidra/Framework/Docking/src/main/java/docking/widgets/table/threaded/ThreadedTableModelUpdateMgr.java

@@ -31,6 +31,8 @@ import ghidra.util.task.*;
  * in a ThreadedTableModel occur, this class schedules a TableUpdateJob to do the work.  It uses
  * a SwingUpdateManager to coalesce add/remove so that the table does not constantly update when
  * are large number of table changing events are incoming.
+ * 
+ * @param <T> the row type 
  */
 class ThreadedTableModelUpdateMgr<T> {
 	static final int TOO_MANY_ADD_REMOVES = 3000;
@@ -38,7 +40,7 @@ class ThreadedTableModelUpdateMgr<T> {
 	public final static int MAX_DELAY = 1000 * 60 * 20;
 
 	private ThreadedTableModel<T, ?> model;
-	private SwingUpdateManager updateManager;
+	private SwingUpdateManager addRemoveUpdater;
 	private TaskMonitor monitor;
 
 	// weakly consistent iterator so that clients can remove listeners on notification
@@ -70,7 +72,7 @@ class ThreadedTableModelUpdateMgr<T> {
 				"the given task monitor must be cancel enabled (e.g., you cannot use " +
 				"the TaskMonitorAdapter.DUMMY_MONITOR, as that is not cancelleable)");
 
-		updateManager = new SwingUpdateManager(DELAY, MAX_DELAY, () -> processAddRemoveItems());
+		addRemoveUpdater = new SwingUpdateManager(DELAY, MAX_DELAY, () -> processAddRemoveItems());
 
 		notifyPending = () -> {
 			for (ThreadedTableModelListener listener : listeners) {
@@ -110,7 +112,7 @@ class ThreadedTableModelUpdateMgr<T> {
 	}
 
 	Object getSynchronizingLock() {
-		return updateManager;
+		return addRemoveUpdater;
 	}
 
 	/**
@@ -118,15 +120,17 @@ class ThreadedTableModelUpdateMgr<T> {
 	 * Called when the swing update manager decides its time to run.
 	 */
 	private void processAddRemoveItems() {
-		synchronized (updateManager) {
+		synchronized (addRemoveUpdater) {
 			if (addRemoveWaitList.isEmpty()) {
 				return;
 			}
+
 			// too many requests for add/remove triggers a full reload for efficiency
 			if (addRemoveWaitList.size() > maxAddRemoveCount) {
 				reload();
 				return;
 			}
+
 			pendingJob = new AddRemoveJob<>(model, addRemoveWaitList, monitor);
 			addRemoveWaitList = new ArrayList<>();
 			runJob();
@@ -138,13 +142,15 @@ class ThreadedTableModelUpdateMgr<T> {
 	 * work that is built into the pending job.
 	 */
 	private void runJob() {
-		synchronized (updateManager) {
+		synchronized (addRemoveUpdater) {
 			if (thread != null) {
-				return; // if thread exists, it will handle any pending job.
+				return; // if thread exists, it will handle any pending job
 			}
+
 			if (pendingJob == null) {
 				return; // nothing to do!
 			}
+
 			// Ok, we have to create a thread to process the pending job
 			thread = new Thread(threadRunnable,
 				"Threaded Table Model Update Manager: " + model.getName());
@@ -161,8 +167,8 @@ class ThreadedTableModelUpdateMgr<T> {
 	 *  things.
 	 */
 	public void cancelAllJobs() {
-		synchronized (updateManager) {
+		synchronized (addRemoveUpdater) {
-			updateManager.stop();
+			addRemoveUpdater.stop();
 			if (currentJob != null) {
 				currentJob.cancel();
 			}
@@ -176,7 +182,7 @@ class ThreadedTableModelUpdateMgr<T> {
 	 * is performed.  It also is called if too many add/removes have been accumulated.
 	 */
 	void reload() {
-		synchronized (updateManager) {
+		synchronized (addRemoveUpdater) {
 			cancelAllJobs();
 			pendingJob = new LoadJob<>(model, monitor);
 			runJob();
@@ -184,7 +190,7 @@ class ThreadedTableModelUpdateMgr<T> {
 	}
 
 	void reloadSpecificData(List<T> data) {
-		synchronized (updateManager) {
+		synchronized (addRemoveUpdater) {
 			cancelAllJobs();
 			TableData<T> tableData = TableData.createFullDataset(data);
 			pendingJob = new LoadSpecificDataJob<>(model, monitor, tableData);
@@ -209,19 +215,18 @@ class ThreadedTableModelUpdateMgr<T> {
 	 *                  to be re-sorted.
 	 */
 	void sort(TableSortingContext<T> sortingContext, boolean forceSort) {
-		synchronized (updateManager) {
+		synchronized (addRemoveUpdater) {
 			if (currentJob != null && pendingJob == null &&
 				currentJob.requestSort(sortingContext, forceSort)) {
 				return;
 			}
 
-			if (pendingJob != null) {
+			if (pendingJob != null && pendingJob.requestSort(sortingContext, forceSort)) {
-				pendingJob.requestSort(sortingContext, forceSort);
+				return;
-			}
-			else {
-				pendingJob = new SortJob<>(model, monitor, sortingContext, forceSort);
-				runJob();
 			}
+
+			pendingJob = new SortJob<>(model, monitor, sortingContext, forceSort);
+			runJob();
 		}
 	}
 
@@ -239,17 +244,17 @@ class ThreadedTableModelUpdateMgr<T> {
 	 * start a thread to do the work.
 	 */
 	void filter() {
-		synchronized (updateManager) {
+		synchronized (addRemoveUpdater) {
 			if (currentJob != null && pendingJob == null && currentJob.requestFilter()) {
 				return;
 			}
-			if (pendingJob != null) {
+
-				pendingJob.requestFilter();
+			if (pendingJob != null && pendingJob.requestFilter()) {
-			}
+				return;
-			else {
-				pendingJob = new FilterJob<>(model, monitor);
-				runJob();
 			}
+
+			pendingJob = new FilterJob<>(model, monitor);
+			runJob();
 		}
 	}
 
@@ -263,7 +268,7 @@ class ThreadedTableModelUpdateMgr<T> {
 	 * @param item the add/remove item to process.
 	 */
 	void addRemove(AddRemoveListItem<T> item) {
-		synchronized (updateManager) {
+		synchronized (addRemoveUpdater) {
 			if (pendingJob != null) {
 				pendingJob.addRemove(item, maxAddRemoveCount);
 				return;
@@ -279,7 +284,7 @@ class ThreadedTableModelUpdateMgr<T> {
 			}
 
 			addRemoveWaitList.add(item);
-			updateManager.update();
+			addRemoveUpdater.update();
 		}
 	}
 
@@ -289,20 +294,20 @@ class ThreadedTableModelUpdateMgr<T> {
 	 * @return true if there is work to be done.
 	 */
 	boolean isBusy() {
-		synchronized (updateManager) {
+		synchronized (addRemoveUpdater) {
-			return thread != null || pendingJob != null || updateManager.isBusy() ||
+			return thread != null || pendingJob != null || addRemoveUpdater.isBusy() ||
 				!addRemoveWaitList.isEmpty();
 		}
 	}
 
 	/**
-	 * Sets the delay for the swing update manager.
+	 * Sets the delay for the swing update managers
 	 * @param updateDelayMillis the new delay for the swing update manager
 	 * @param maxUpdateDelayMillis the new max update delay; updates will not wait past this time
 	 */
 	void setUpdateDelay(int updateDelayMillis, int maxUpdateDelayMillis) {
-		updateManager.dispose();
+		addRemoveUpdater.dispose();
-		updateManager = new SwingUpdateManager(updateDelayMillis, maxUpdateDelayMillis,
+		addRemoveUpdater = new SwingUpdateManager(updateDelayMillis, maxUpdateDelayMillis,
 			() -> processAddRemoveItems());
 	}
 
@@ -335,12 +340,12 @@ class ThreadedTableModelUpdateMgr<T> {
 	 * Disposes the updateManager resource.
 	 */
 	void dispose() {
-		synchronized (updateManager) {
+		synchronized (addRemoveUpdater) {
 			listeners.clear();
 			monitor.cancel();
 			monitor = new PermantentlyCancelledMonitor();
 			cancelAllJobs();
-			updateManager.dispose();
+			addRemoveUpdater.dispose();
 		}
 	}
 
@@ -348,7 +353,7 @@ class ThreadedTableModelUpdateMgr<T> {
 	 * Kicks the swing update manager to immediately process any accumulated add/removes.
 	 */
 	public void updateNow() {
-		updateManager.updateNow();
+		addRemoveUpdater.updateNow();
 	}
 
 	/**
@@ -356,9 +361,14 @@ class ThreadedTableModelUpdateMgr<T> {
 	 * @return the new current job.
 	 */
 	private TableUpdateJob<T> getNextJob() {
-		synchronized (updateManager) {
+		synchronized (addRemoveUpdater) {
 			currentJob = pendingJob;
 			pendingJob = null;
+
+			if (currentJob == null) {
+				jobDone();
+			}
+
 			return currentJob;
 		}
 	}
@@ -369,7 +379,11 @@ class ThreadedTableModelUpdateMgr<T> {
 	 * to work pending.
 	 */
 	private void jobDone() {
-		synchronized (updateManager) {
+
+		// This synchronized is not need, as this method is only called from within a 
+		// synchronized block.  But, it feels better to be explicit here to signal that 
+		// synchronization is needed.  
+		synchronized (addRemoveUpdater) {
 
 			boolean isCancelled = monitor.isCancelled();
 			if (isCancelled) {
@@ -393,7 +407,7 @@ class ThreadedTableModelUpdateMgr<T> {
 	/**
 	 * Runnable used be new threads to run scheduled jobs.
 	 */
-	class ThreadRunnable implements Runnable {
+	private class ThreadRunnable implements Runnable {
 		@Override
 		public void run() {
 			TableUpdateJob<T> job = getNextJob();
@@ -402,11 +416,9 @@ class ThreadedTableModelUpdateMgr<T> {
 				job.run();
 
 				// useful for debug				
-//				Msg.debug(this, "ran job: " + job);
+				// Msg.debug(this, "ran job: " + job);
 				job = getNextJob();
-
 			}
-			jobDone();
 		}
 	}