diff --git a/Ghidra/Framework/Generic/src/main/java/ghidra/util/datastruct/CopyOnReadWeakSet.java b/Ghidra/Framework/Generic/src/main/java/ghidra/util/datastruct/CopyOnReadWeakSet.java index ee740a6a61..f068abc377 100644 --- a/Ghidra/Framework/Generic/src/main/java/ghidra/util/datastruct/CopyOnReadWeakSet.java +++ b/Ghidra/Framework/Generic/src/main/java/ghidra/util/datastruct/CopyOnReadWeakSet.java @@ -16,53 +16,43 @@ package ghidra.util.datastruct; import java.util.*; +import java.util.stream.Stream; -public class CopyOnReadWeakSet extends WeakSet { +/** + * A copy on read set that will create a copy of its internal data for iteration operations. This + * allows clients to avoid concurrency issue by allowing mutates during reads. All operations + * of this class are synchronized to allow clients to use non-iterative methods without the need + * for a copy operation. + * + * @param the type + */ +class CopyOnReadWeakSet extends WeakSet { protected CopyOnReadWeakSet() { // restrict access; use factory method in WeakDataStructureFactory } - /** - * Add the given object to the set. - */ + private synchronized Collection createCopy() { + Set ks = weakHashStorage.keySet(); + return new ArrayList<>(ks); + } + @Override public synchronized void add(T t) { maybeWarnAboutAnonymousValue(t); weakHashStorage.put(t, null); } - /** - * Remove the given object from the data structure - */ @Override public synchronized void remove(T t) { weakHashStorage.remove(t); } - /** - * Remove all elements from this data structure - */ @Override public synchronized void clear() { weakHashStorage.clear(); } - /** - * Returns an iterator over the elements in this data structure. - */ - @Override - public synchronized Iterator iterator() { - Set ks = weakHashStorage.keySet(); - List list = new ArrayList<>(ks); - return list.iterator(); - } - - @Override - public synchronized Collection values() { - return weakHashStorage.keySet(); - } - @Override public synchronized boolean isEmpty() { return weakHashStorage.isEmpty(); @@ -77,4 +67,25 @@ public class CopyOnReadWeakSet extends WeakSet { public synchronized boolean contains(T t) { return weakHashStorage.containsKey(t); } + + @Override + public synchronized String toString() { + return weakHashStorage.keySet().toString(); + } + + @Override + public synchronized Iterator iterator() { + return createCopy().iterator(); + } + + @Override + public synchronized Collection values() { + return createCopy(); + } + + @Override + public synchronized Stream stream() { + return createCopy().stream(); + } + } diff --git a/Ghidra/Framework/Generic/src/main/java/ghidra/util/datastruct/CopyOnWriteWeakSet.java b/Ghidra/Framework/Generic/src/main/java/ghidra/util/datastruct/CopyOnWriteWeakSet.java index 83211ac68f..a86af1c6a9 100644 --- a/Ghidra/Framework/Generic/src/main/java/ghidra/util/datastruct/CopyOnWriteWeakSet.java +++ b/Ghidra/Framework/Generic/src/main/java/ghidra/util/datastruct/CopyOnWriteWeakSet.java @@ -16,6 +16,7 @@ package ghidra.util.datastruct; import java.util.*; +import java.util.stream.Stream; import org.apache.commons.collections4.IteratorUtils; @@ -25,12 +26,12 @@ import org.apache.commons.collections4.IteratorUtils; * number of event notification operations significantly out numbers mutations to this structure * (e.g., adding and removing items. *

- * An example use cases where using this class is a good fit would be a listener list where + * An example use case 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 is used to fire a large number of events. *

* 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. + * contents are changed often, but iterated very little. *

* Finally, if this structure is only ever used from a single thread, like the Swing thread, then * you do not need the overhead of this class, as the Swing thread synchronous access guarantees @@ -47,7 +48,7 @@ class CopyOnWriteWeakSet extends WeakSet { } @Override - public synchronized Iterator iterator() { + public Iterator iterator() { return IteratorUtils.unmodifiableIterator(weakHashStorage.keySet().iterator()); } @@ -60,26 +61,29 @@ class CopyOnWriteWeakSet extends WeakSet { * @param it the items */ @Override - public void addAll(Iterable it) { + public synchronized void addAll(Iterable 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); + WeakHashMap newStorage = new WeakHashMap<>(weakHashStorage); for (T t : it) { - weakHashStorage.put(t, null); + newStorage.put(t, null); } + weakHashStorage = newStorage; } @Override public synchronized void add(T t) { maybeWarnAboutAnonymousValue(t); - weakHashStorage = new WeakHashMap<>(weakHashStorage); - weakHashStorage.put(t, null); + WeakHashMap newStorage = new WeakHashMap<>(weakHashStorage); + newStorage.put(t, null); + weakHashStorage = newStorage; } @Override public synchronized void remove(T t) { - weakHashStorage = new WeakHashMap<>(weakHashStorage); - weakHashStorage.remove(t); + WeakHashMap newStorage = new WeakHashMap<>(weakHashStorage); + newStorage.remove(t); + weakHashStorage = newStorage; } @Override @@ -88,22 +92,32 @@ class CopyOnWriteWeakSet extends WeakSet { } @Override - public synchronized Collection values() { + public Collection values() { return weakHashStorage.keySet(); } @Override - public synchronized boolean isEmpty() { + public boolean isEmpty() { return weakHashStorage.isEmpty(); } @Override - public synchronized int size() { + public int size() { return weakHashStorage.size(); } @Override - public synchronized boolean contains(T t) { + public boolean contains(T t) { return weakHashStorage.containsKey(t); } + + @Override + public Stream stream() { + return values().stream(); + } + + @Override + public String toString() { + return values().toString(); + } } diff --git a/Ghidra/Framework/Generic/src/main/java/ghidra/util/datastruct/ThreadUnsafeWeakSet.java b/Ghidra/Framework/Generic/src/main/java/ghidra/util/datastruct/ThreadUnsafeWeakSet.java index b4569f0949..01f46c2896 100644 --- a/Ghidra/Framework/Generic/src/main/java/ghidra/util/datastruct/ThreadUnsafeWeakSet.java +++ b/Ghidra/Framework/Generic/src/main/java/ghidra/util/datastruct/ThreadUnsafeWeakSet.java @@ -17,6 +17,7 @@ package ghidra.util.datastruct; import java.util.Collection; import java.util.Iterator; +import java.util.stream.Stream; class ThreadUnsafeWeakSet extends WeakSet { @@ -24,34 +25,22 @@ class ThreadUnsafeWeakSet extends WeakSet { // restrict access; use factory method in base class } - /** - * Add the given object to the set. - */ @Override public void add(T t) { maybeWarnAboutAnonymousValue(t); weakHashStorage.put(t, null); } - /** - * Remove the given object from the data structure - */ @Override public void remove(T t) { weakHashStorage.remove(t); } - /** - * Remove all elements from this data structure - */ @Override public void clear() { weakHashStorage.clear(); } - /** - * Returns an iterator over the elements in this data structure. - */ @Override public Iterator iterator() { return weakHashStorage.keySet().iterator(); @@ -81,4 +70,10 @@ class ThreadUnsafeWeakSet extends WeakSet { public String toString() { return weakHashStorage.toString(); } + + @Override + public Stream stream() { + return values().stream(); + } + } diff --git a/Ghidra/Framework/Generic/src/main/java/ghidra/util/datastruct/WeakSet.java b/Ghidra/Framework/Generic/src/main/java/ghidra/util/datastruct/WeakSet.java index 2ba5ebb5ed..2e54d74cec 100644 --- a/Ghidra/Framework/Generic/src/main/java/ghidra/util/datastruct/WeakSet.java +++ b/Ghidra/Framework/Generic/src/main/java/ghidra/util/datastruct/WeakSet.java @@ -136,12 +136,5 @@ public abstract class WeakSet implements Iterable { * Returns a stream of the values of this collection. * @return a stream of the values of this collection. */ - public Stream stream() { - return values().stream(); - } - - @Override - public String toString() { - return values().toString(); - } + public abstract Stream stream(); }