GP-2527 - Fixed issue with dialog always appearing for short running tasks

Ghidra/Framework/Generic/src/main/java/ghidra/util/datastruct/CopyOnWriteWeakSet.java

@@ -27,7 +27,7 @@ import org.apache.commons.collections4.IteratorUtils;
  * <p>
  * An example use cases where using this class is a good fit would be a listener list where
  * listeners are added during initialization, but not after that.   Further, this hypothetical
- * list fires a large number of events.
+ * list is used to fire a large number of events.
  * <p>
  * A bad use of this class would be as a container to store widgets where the container the
  * contents are changed often, but iterated over very little.
@@ -51,6 +51,24 @@ class CopyOnWriteWeakSet<T> extends WeakSet<T> {
 		return IteratorUtils.unmodifiableIterator(weakHashStorage.keySet().iterator());
 	}
 
+	/**
+	 * Adds all items to this set.
+	 * <p>
+	 * Note: calling this method will only result in one copy operation.  If {@link #add(Object)}
+	 * were called instead for each item of the iterator, then each call would copy this set.
+	 *
+	 * @param it the items
+	 */
+	@Override
+	public void addAll(Iterable<T> it) {
+		// only make one copy for the entire set of changes instead of for each change, as calling
+		// add() would do
+		weakHashStorage = new WeakHashMap<>(weakHashStorage);
+		for (T t : it) {
+			weakHashStorage.put(t, null);
+		}
+	}
+
 	@Override
 	public synchronized void add(T t) {
 		maybeWarnAboutAnonymousValue(t);

Ghidra/Framework/Generic/src/main/java/ghidra/util/datastruct/WeakDataStructureFactory.java

@@ -19,8 +19,8 @@ import generic.concurrent.ConcurrentListenerSet;
 
 /**
  * Factory for creating containers to use in various threading environments
- * 
- * Other non-weak listeners: 
+ *
+ * Other non-weak listeners:
  * <ul>
  * 	<li>{@link ConcurrentListenerSet}</li>
  * </ul>
@@ -29,16 +29,26 @@ public class WeakDataStructureFactory {
 
 	/**
 	 * Use when all access are on a single thread, such as the Swing thread.
-	 * 
+	 *
 	 * @return a new WeakSet
 	 */
 	public static <T> WeakSet<T> createSingleThreadAccessWeakSet() {
 		return new ThreadUnsafeWeakSet<>();
 	}
 
+	/**
+	 * Use to signal that the returned weak set is not thread safe and must be protected accordingly
+	 * when used in a multi-threaded environment.
+	 *
+	 * @return a new WeakSet
+	 */
+	public static <T> WeakSet<T> createThreadUnsafeWeakSet() {
+		return new ThreadUnsafeWeakSet<>();
+	}
+
 	/**
 	 * Use when mutations outweigh iterations.
-	 * 
+	 *
 	 * @return a new WeakSet
 	 * @see CopyOnReadWeakSet
 	 */
@@ -48,7 +58,7 @@ public class WeakDataStructureFactory {
 
 	/**
 	 * Use when iterations outweigh mutations.
-	 * 
+	 *
 	 * @return a new WeakSet
 	 * @see CopyOnWriteWeakSet
 	 */

Ghidra/Framework/Generic/src/main/java/ghidra/util/datastruct/WeakSet.java

@@ -42,10 +42,10 @@ public abstract class WeakSet<T> implements Iterable<T> {
 
 	/**
 	 * Looks for situations where clients <b>may</b> lose the values added to this class.  This
-	 * most often happens when a client adds an anonymous, local listener to an object that is 
-	 * using a WeakSet to store its listeners.  Our policy is to implement listeners at the 
-	 * class field level so that they will not be flagged by this method. 
-	 * 
+	 * most often happens when a client adds an anonymous, local listener to an object that is
+	 * using a WeakSet to store its listeners.  Our policy is to implement listeners at the
+	 * class field level so that they will not be flagged by this method.
+	 *
 	 * @param t The object to check
 	 */
 	protected void maybeWarnAboutAnonymousValue(T t) {
@@ -76,7 +76,17 @@ public abstract class WeakSet<T> implements Iterable<T> {
 
 //==================================================================================================
 // Interface Methods
-//==================================================================================================	
+//==================================================================================================
+
+	/**
+	 * Adds all items to this set
+	 * @param it the items
+	 */
+	public void addAll(Iterable<T> it) {
+		for (T t : it) {
+			add(t);
+		}
+	}
 
 	/**
 	 * Add the given object to the set
@@ -87,12 +97,13 @@ public abstract class WeakSet<T> implements Iterable<T> {
 	/**
 	 * Remove the given object from the data structure
 	 * @param t the object to remove
-	 * 
+	 *
 	 */
 	public abstract void remove(T t);
 
 	/**
 	 * Returns true if the given object is in this data structure
+	 * @param t the object
 	 * @return true if the given object is in this data structure
 	 */
 	public abstract boolean contains(T t);
@@ -116,7 +127,7 @@ public abstract class WeakSet<T> implements Iterable<T> {
 
 	/**
 	 * Returns a Collection view of this set.  The returned Collection is backed by this set.
-	 * 
+	 *
 	 * @return a Collection view of this set.  The returned Collection is backed by this set.
 	 */
 	public abstract Collection<T> values();

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

@@ -37,9 +37,10 @@ import ghidra.util.datastruct.WeakSet;
 class DomainObjectChangeSupport {
 
 	private WeakSet<DomainObjectListener> listeners =
-		WeakDataStructureFactory.createSingleThreadAccessWeakSet();
+		WeakDataStructureFactory.createThreadUnsafeWeakSet();
 	private List<EventNotification> notificationQueue = new ArrayList<>();
 	private List<DomainObjectChangeRecord> recordsQueue = new ArrayList<>();
+
 	private GhidraTimer timer;
 
 	private DomainObject src;
@@ -68,22 +69,6 @@ class DomainObjectChangeSupport {
 		timer.setRepeats(true);
 	}
 
-	// Note: must be called on the Swing thread
-	private void sendEventNow() {
-		List<EventNotification> notifications = withLock(() -> {
-
-			DomainObjectChangedEvent e = createEventFromQueuedRecords();
-			notificationQueue.add(new EventNotification(e, new ArrayList<>(listeners.values())));
-			List<EventNotification> existingNotifications = new ArrayList<>(notificationQueue);
-			notificationQueue.clear();
-			return existingNotifications;
-		});
-
-		for (EventNotification notification : notifications) {
-			notification.doNotify();
-		}
-	}
-
 	// Note: must be called inside of withLock()
 	private DomainObjectChangedEvent createEventFromQueuedRecords() {
 
@@ -106,13 +91,19 @@ class DomainObjectChangeSupport {
 		withLock(() -> {
 
 			// Capture the pending event to send to the existing listeners.  This prevents the new
-			// listener from getting events registered before the listener was added.
-			DomainObjectChangedEvent pendingEvent = createEventFromQueuedRecords();
-			List<DomainObjectListener> previousListeners = new ArrayList<>(listeners.values());
+			// listener from getting events registered before the listener was added.  Also, create
+			// a new set of listeners so that any events already posted to the Swing thread do not
+			// see the newly added listener.
+			Collection<DomainObjectListener> previousListeners = listeners.values();
+			listeners = WeakDataStructureFactory.createThreadUnsafeWeakSet();
+			listeners.addAll(previousListeners);
 			listeners.add(listener);
 
-			notificationQueue.add(new EventNotification(pendingEvent, previousListeners));
-			timer.start();
+			DomainObjectChangedEvent pendingEvent = createEventFromQueuedRecords();
+			if (pendingEvent != null) {
+				notificationQueue.add(new EventNotification(pendingEvent, previousListeners));
+				timer.start();
+			}
 		});
 	}
 
@@ -138,7 +129,43 @@ class DomainObjectChangeSupport {
 			throw new IllegalStateException("Cannot call flush() with locks!");
 		}
 
-		Swing.runNow(this::sendEventNow);
+		sendEventNow();
+	}
+
+	private void sendEventNow() {
+		List<EventNotification> notifications = withLock(() -> {
+
+			DomainObjectChangedEvent e = createEventFromQueuedRecords();
+			if (e == null) {
+				return Collections.emptyList();
+			}
+
+			//
+			// Note: we do not copy the listeners that are used by the event notification.  This
+			// provides a couple benefits:
+			// -if a listener is removed, it will no longer get the event, without us having to
+			//  processes the already posted event notifications, and
+			// -we avoid excessive copying of relatively large listener sets for frequent event
+			//  notifications.
+			//
+			notificationQueue.add(new EventNotification(e, listeners.values()));
+			List<EventNotification> existingNotifications = new ArrayList<>(notificationQueue);
+			notificationQueue.clear();
+			return existingNotifications;
+		});
+
+		if (notifications.isEmpty()) {
+			return;
+		}
+
+		Swing.runNow(() -> doSendEventsNow(notifications));
+	}
+
+	// Note: must be called on the Swing thread
+	private void doSendEventsNow(List<EventNotification> notifications) {
+		for (EventNotification notification : notifications) {
+			notification.doNotify();
+		}
 	}
 
 	void fireEvent(DomainObjectChangeRecord docr) {
@@ -249,9 +276,10 @@ class DomainObjectChangeSupport {
 	private class EventNotification {
 
 		private DomainObjectChangedEvent event;
-		private List<DomainObjectListener> receivers;
+		private Collection<DomainObjectListener> receivers;
 
-		EventNotification(DomainObjectChangedEvent event, List<DomainObjectListener> recievers) {
+		EventNotification(DomainObjectChangedEvent event,
+				Collection<DomainObjectListener> recievers) {
 			this.event = event;
 			this.receivers = recievers;
 		}