Simplify AbstractPreferences.
There's no need for a complicated re-implementation of copy
on write lists. Simple ArrayLists will suffice; all operations
are guarded by |lock| and we can return a copy of the list to
the Event processor thread to avoid concurrent modification of
the list while that thread is dispatching events.
Fixes various preference tests related to adding / removing
duplicate listeners.
A notable difference in behaviour here is that listeners are
now compared using their equals() method and not using reference
equality.
bug: 26023763
bug: 25996530
Change-Id: I57ee723cf926786e1f119ec1e7740c66162ab749
diff --git a/ojluni/src/main/java/java/util/prefs/AbstractPreferences.java b/ojluni/src/main/java/java/util/prefs/AbstractPreferences.java
index f4a7e5e..0abb8b0 100755
--- a/ojluni/src/main/java/java/util/prefs/AbstractPreferences.java
+++ b/ojluni/src/main/java/java/util/prefs/AbstractPreferences.java
@@ -166,13 +166,12 @@
/**
* Registered preference change listeners.
*/
- private PreferenceChangeListener[] prefListeners =
- new PreferenceChangeListener[0];
+ private final ArrayList<PreferenceChangeListener> prefListeners = new ArrayList<>();
/**
* Registered node change listeners.
*/
- private NodeChangeListener[] nodeListeners = new NodeChangeListener[0];
+ private final ArrayList<NodeChangeListener> nodeListeners = new ArrayList<>();
/**
* An object whose monitor is used to lock this node. This object
@@ -1034,11 +1033,7 @@
if (removed)
throw new IllegalStateException("Node has been removed.");
- // Copy-on-write
- PreferenceChangeListener[] old = prefListeners;
- prefListeners = new PreferenceChangeListener[old.length + 1];
- System.arraycopy(old, 0, prefListeners, 0, old.length);
- prefListeners[old.length] = pcl;
+ prefListeners.add(pcl);
}
startEventDispatchThreadIfNecessary();
}
@@ -1047,21 +1042,12 @@
synchronized(lock) {
if (removed)
throw new IllegalStateException("Node has been removed.");
- if ((prefListeners == null) || (prefListeners.length == 0))
- throw new IllegalArgumentException("Listener not registered.");
- // Copy-on-write
- PreferenceChangeListener[] newPl =
- new PreferenceChangeListener[prefListeners.length - 1];
- int i = 0;
- while (i < newPl.length && prefListeners[i] != pcl)
- newPl[i] = prefListeners[i++];
-
- if (i == newPl.length && prefListeners[i] != pcl)
+ if (!prefListeners.contains(pcl)) {
throw new IllegalArgumentException("Listener not registered.");
- while (i < newPl.length)
- newPl[i] = prefListeners[++i];
- prefListeners = newPl;
+ }
+
+ prefListeners.remove(pcl);
}
}
@@ -1072,16 +1058,7 @@
if (removed)
throw new IllegalStateException("Node has been removed.");
- // Copy-on-write
- if (nodeListeners == null) {
- nodeListeners = new NodeChangeListener[1];
- nodeListeners[0] = ncl;
- } else {
- NodeChangeListener[] old = nodeListeners;
- nodeListeners = new NodeChangeListener[old.length + 1];
- System.arraycopy(old, 0, nodeListeners, 0, old.length);
- nodeListeners[old.length] = ncl;
- }
+ nodeListeners.add(ncl);
}
startEventDispatchThreadIfNecessary();
}
@@ -1090,23 +1067,12 @@
synchronized(lock) {
if (removed)
throw new IllegalStateException("Node has been removed.");
- if ((nodeListeners == null) || (nodeListeners.length == 0))
- throw new IllegalArgumentException("Listener not registered.");
- // Copy-on-write
- int i = 0;
- while (i < nodeListeners.length && nodeListeners[i] != ncl)
- i++;
- if (i == nodeListeners.length)
+ if (!nodeListeners.contains(ncl)) {
throw new IllegalArgumentException("Listener not registered.");
- NodeChangeListener[] newNl =
- new NodeChangeListener[nodeListeners.length - 1];
- if (i != 0)
- System.arraycopy(nodeListeners, 0, newNl, 0, i);
- if (i != newNl.length)
- System.arraycopy(nodeListeners, i + 1,
- newNl, i, newNl.length - i);
- nodeListeners = newNl;
+ }
+
+ nodeListeners.remove(ncl);
}
}
@@ -1521,19 +1487,19 @@
}
/**
- * Return this node's preference/node change listeners. Even though
- * we're using a copy-on-write lists, we use synchronized accessors to
- * ensure information transmission from the writing thread to the
- * reading thread.
+ * Return this node's preference/node change listeners. All accesses to prefListeners
+ * and nodeListeners are guarded by |lock|. We return a copy of the list so that the
+ * EventQueue thread will iterate over a fixed snapshot of the listeners at the time of
+ * this call.
*/
PreferenceChangeListener[] prefListeners() {
synchronized(lock) {
- return prefListeners;
+ return prefListeners.toArray(new PreferenceChangeListener[prefListeners.size()]);
}
}
NodeChangeListener[] nodeListeners() {
synchronized(lock) {
- return nodeListeners;
+ return nodeListeners.toArray(new NodeChangeListener[nodeListeners.size()]);
}
}
@@ -1543,7 +1509,7 @@
* listeners. Invoked with this.lock held.
*/
private void enqueuePreferenceChangeEvent(String key, String newValue) {
- if (prefListeners.length != 0) {
+ if (!prefListeners.isEmpty()) {
synchronized(eventQueue) {
eventQueue.add(new PreferenceChangeEvent(this, key, newValue));
eventQueue.notify();
@@ -1557,7 +1523,7 @@
* this.lock held.
*/
private void enqueueNodeAddedEvent(Preferences child) {
- if (nodeListeners.length != 0) {
+ if (!nodeListeners.isEmpty()) {
synchronized(eventQueue) {
eventQueue.add(new NodeAddedEvent(this, child));
eventQueue.notify();
@@ -1571,7 +1537,7 @@
* this.lock held.
*/
private void enqueueNodeRemovedEvent(Preferences child) {
- if (nodeListeners.length != 0) {
+ if (!nodeListeners.isEmpty()) {
synchronized(eventQueue) {
eventQueue.add(new NodeRemovedEvent(this, child));
eventQueue.notify();
