BUG-509: Fix thread safety of listener registration

[controller.git] / opendaylight / md-sal / sal-dom-broker / src / main / java / org / opendaylight / controller / md / sal / dom / store / impl / tree / ListenerRegistrationNode.java

diff --git a/opendaylight/md-sal/sal-dom-broker/src/main/java/org/opendaylight/controller/md/sal/dom/store/impl/tree/ListenerRegistrationNode.java b/opendaylight/md-sal/sal-dom-broker/src/main/java/org/opendaylight/controller/md/sal/dom/store/impl/tree/ListenerRegistrationNode.java
index 2528d383b95a0c9f6534ad0f01627f56dbc26c5c..854c125af17a732f1d1c40365cb355ddae25a317 100644 (file)
--- a/opendaylight/md-sal/sal-dom-broker/src/main/java/org/opendaylight/controller/md/sal/dom/store/impl/tree/ListenerRegistrationNode.java
+++ b/opendaylight/md-sal/sal-dom-broker/src/main/java/org/opendaylight/controller/md/sal/dom/store/impl/tree/ListenerRegistrationNode.java
@@ -46,9 +46,16 @@ public class ListenerRegistrationNode implements StoreTreeNode<ListenerRegistrat
         return identifier;
     }
 
         return identifier;
     }
 

+    /**
+     * Return the list of current listeners. Any caller wishing to use this method
+     * has to make sure the collection remains unchanged while it's executing. This
+     * means the caller has to synchronize externally both the registration and
+     * unregistration process.
+     *
+     * @return the list of current listeners
+     */

     @SuppressWarnings({ "rawtypes", "unchecked" })
     public Collection<org.opendaylight.controller.md.sal.dom.store.impl.DataChangeListenerRegistration<?>> getListeners() {
     @SuppressWarnings({ "rawtypes", "unchecked" })
     public Collection<org.opendaylight.controller.md.sal.dom.store.impl.DataChangeListenerRegistration<?>> getListeners() {

-        // FIXME: this is not thread-safe and races with listener (un)registration!

         return (Collection) listeners;
     }
 
         return (Collection) listeners;
     }
 


@@ -67,7 +74,6 @@ public class ListenerRegistrationNode implements StoreTreeNode<ListenerRegistrat
     }
 
     /**
     }
 
     /**

-     *

      * Registers listener on this node.
      *
      * @param path Full path on which listener is registered.
      * Registers listener on this node.
      *
      * @param path Full path on which listener is registered.


@@ -75,16 +81,18 @@ public class ListenerRegistrationNode implements StoreTreeNode<ListenerRegistrat
      * @param scope Scope of triggering event.
      * @return
      */
      * @param scope Scope of triggering event.
      * @return
      */

-    public <L extends AsyncDataChangeListener<InstanceIdentifier, NormalizedNode<?, ?>>> DataChangeListenerRegistration<L> registerDataChangeListener(final InstanceIdentifier path,
+    public synchronized <L extends AsyncDataChangeListener<InstanceIdentifier, NormalizedNode<?, ?>>> DataChangeListenerRegistration<L> registerDataChangeListener(final InstanceIdentifier path,

             final L listener, final DataChangeScope scope) {
 
         DataChangeListenerRegistration<L> listenerReg = new DataChangeListenerRegistration<L>(path,listener, scope, this);
         listeners.add(listenerReg);
             final L listener, final DataChangeScope scope) {
 
         DataChangeListenerRegistration<L> listenerReg = new DataChangeListenerRegistration<L>(path,listener, scope, this);
         listeners.add(listenerReg);

+        LOG.debug("Listener {} registered", listener);

         return listenerReg;
     }
 
         return listenerReg;
     }
 

-    private void removeListener(final DataChangeListenerRegistration<?> listener) {
+    private synchronized void removeListener(final DataChangeListenerRegistration<?> listener) {

         listeners.remove(listener);
         listeners.remove(listener);

+        LOG.debug("Listener {} unregistered", listener);

         removeThisIfUnused();
     }
 
         removeThisIfUnused();
     }