BugFix: 1st listener could potentially break Event value.
- Original code was not copying the value for the 1st loop.
If the Event value was mutable and 1st listener modifies it,
rest of the event listeners will receive the modified Event value.
Change-Id: Ia853210f24beccc3b1256de0b0bfc4a10e6f438f
diff --git a/src/main/java/net/onrc/onos/core/datagrid/HazelcastEventChannel.java b/src/main/java/net/onrc/onos/core/datagrid/HazelcastEventChannel.java
index c3a35cf..fbc67a4 100644
--- a/src/main/java/net/onrc/onos/core/datagrid/HazelcastEventChannel.java
+++ b/src/main/java/net/onrc/onos/core/datagrid/HazelcastEventChannel.java
@@ -1,6 +1,7 @@
package net.onrc.onos.core.datagrid;
import java.util.Collection;
+import java.util.Iterator;
import java.util.LinkedList;
import java.util.Set;
import java.util.concurrent.CopyOnWriteArrayList;
@@ -343,14 +344,19 @@
//
// Deliver the notification
//
- int index = 0;
- for (IEventChannelListener<K, V> listener : listeners) {
- V copyValue = value;
- if (index++ > 0) {
+ final Iterator<IEventChannelListener<K, V>> it = listeners.iterator();
+ while (it.hasNext()) {
+ final IEventChannelListener<K, V> listener = it.next();
+ if (it.hasNext()) {
// Each listener should get a deep copy of the value
- copyValue = kryo.copy(value);
+ // TODO: compare which is faster
+ // - kryo.copy(value)
+ // - deserializeValue(kryo, valueBytes)
+ listener.entryAdded(kryo.copy(value));
+ } else {
+ // Last listener can use the value
+ listener.entryAdded(value);
}
- listener.entryAdded(copyValue);
}
} finally {
kryoFactory.deleteKryo(kryo);
@@ -375,14 +381,16 @@
//
// Deliver the notification
//
- int index = 0;
- for (IEventChannelListener<K, V> listener : listeners) {
- V copyValue = value;
- if (index++ > 0) {
+ final Iterator<IEventChannelListener<K, V>> it = listeners.iterator();
+ while (it.hasNext()) {
+ final IEventChannelListener<K, V> listener = it.next();
+ if (it.hasNext()) {
// Each listener should get a deep copy of the value
- copyValue = kryo.copy(value);
+ listener.entryRemoved(kryo.copy(value));
+ } else {
+ // Last listener can use the value
+ listener.entryRemoved(value);
}
- listener.entryRemoved(copyValue);
}
} finally {
kryoFactory.deleteKryo(kryo);
@@ -407,14 +415,16 @@
//
// Deliver the notification
//
- int index = 0;
- for (IEventChannelListener<K, V> listener : listeners) {
- V copyValue = value;
- if (index++ > 0) {
+ final Iterator<IEventChannelListener<K, V>> it = listeners.iterator();
+ while (it.hasNext()) {
+ final IEventChannelListener<K, V> listener = it.next();
+ if (it.hasNext()) {
// Each listener should get a deep copy of the value
- copyValue = kryo.copy(value);
+ listener.entryUpdated(kryo.copy(value));
+ } else {
+ // Last listener can use the value
+ listener.entryUpdated(value);
}
- listener.entryUpdated(copyValue);
}
} finally {
kryoFactory.deleteKryo(kryo);