Merge "BUG-2510: Remove all registrations when a routed rpc is closed"

diff --git a/opendaylight/md-sal/sal-binding-broker/src/main/java/org/opendaylight/controller/sal/binding/codegen/impl/RpcRouterCodegenInstance.java b/opendaylight/md-sal/sal-binding-broker/src/main/java/org/opendaylight/controller/sal/binding/codegen/impl/RpcRouterCodegenInstance.java
index 783e5c0cd4f459e94a2009c5a6783c6e8a54cb42..d69aeed3523a660177774826a25be1db2f2974c7 100644 (file)
--- a/opendaylight/md-sal/sal-binding-broker/src/main/java/org/opendaylight/controller/sal/binding/codegen/impl/RpcRouterCodegenInstance.java
+++ b/opendaylight/md-sal/sal-binding-broker/src/main/java/org/opendaylight/controller/sal/binding/codegen/impl/RpcRouterCodegenInstance.java
@@ -8,11 +8,14 @@
 package org.opendaylight.controller.sal.binding.codegen.impl;
 
 import static org.opendaylight.controller.sal.binding.codegen.RuntimeCodeHelper.setRoutingTable;
-
+import com.google.common.collect.ImmutableMap;
+import com.google.common.collect.ImmutableSet;
+import java.util.ArrayList;
+import java.util.Collection;
 import java.util.HashMap;
 import java.util.Map;
 import java.util.Set;
-
+import javax.annotation.concurrent.GuardedBy;
 import org.opendaylight.controller.md.sal.common.api.routing.RouteChange;
 import org.opendaylight.controller.md.sal.common.api.routing.RouteChangeListener;
 import org.opendaylight.controller.sal.binding.api.BindingAwareBroker.RoutedRpcRegistration;
@@ -29,9 +32,6 @@ import org.opendaylight.yangtools.yang.binding.RpcService;
 import org.slf4j.Logger;
 import org.slf4j.LoggerFactory;
 
-import com.google.common.collect.ImmutableMap;
-import com.google.common.collect.ImmutableSet;
-
 public class RpcRouterCodegenInstance<T extends RpcService> implements //
 RpcRouter<T>, RouteChangeListener<Class<? extends BaseIdentity>, InstanceIdentifier<?>> {
 
@@ -133,7 +133,14 @@ RpcRouter<T>, RouteChangeListener<Class<? extends BaseIdentity>, InstanceIdentif
         return new DefaultRpcImplementationRegistration(service);
     }
 
-    private class RoutedRpcRegistrationImpl extends AbstractObjectRegistration<T> implements RoutedRpcRegistration<T> {
+    private final class RoutedRpcRegistrationImpl extends AbstractObjectRegistration<T> implements RoutedRpcRegistration<T> {
+        /*
+         * FIXME: retaining this collection is not completely efficient. We really should be storing
+         *        a reference to this registration, as a particular listener may be registered multiple
+         *        times -- and then this goes kaboom in various aspects.
+         */
+        @GuardedBy("this")
+        private final Collection<Class<? extends BaseIdentity>> contexts = new ArrayList<>(1);
 
         public RoutedRpcRegistrationImpl(final T instance) {
             super(instance);
@@ -145,32 +152,49 @@ RpcRouter<T>, RouteChangeListener<Class<? extends BaseIdentity>, InstanceIdentif
         }
 
         @Override
-        public void registerPath(final Class<? extends BaseIdentity> context, final InstanceIdentifier<?> path) {
+        public synchronized void registerPath(final Class<? extends BaseIdentity> context, final InstanceIdentifier<?> path) {
+            if (isClosed()) {
+                LOG.debug("Closed registration of {} ignoring new path {}", getInstance(), path);
+                return;
+            }
+
             routingTables.get(context).updateRoute(path, getInstance());
+            contexts.add(context);
         }
 
         @Override
-        public void unregisterPath(final Class<? extends BaseIdentity> context, final InstanceIdentifier<?> path) {
+        public synchronized void unregisterPath(final Class<? extends BaseIdentity> context, final InstanceIdentifier<?> path) {
+            if (isClosed()) {
+                LOG.debug("Closed unregistration of {} ignoring new path {}", getInstance(), path);
+                return;
+            }
+
             routingTables.get(context).removeRoute(path, getInstance());
+            contexts.remove(context);
         }
 
+        @Deprecated
         @Override
         public void registerInstance(final Class<? extends BaseIdentity> context, final InstanceIdentifier<?> instance) {
             registerPath(context, instance);
         }
 
+        @Deprecated
         @Override
         public void unregisterInstance(final Class<? extends BaseIdentity> context, final InstanceIdentifier<?> instance) {
             unregisterPath(context, instance);
         }
 
         @Override
-        protected void removeRegistration() {
-
+        protected synchronized void removeRegistration() {
+            for (Class<? extends BaseIdentity> ctx : contexts) {
+                routingTables.get(ctx).removeAllReferences(getInstance());
+            }
+            contexts.clear();
         }
     }
 
-    private class DefaultRpcImplementationRegistration extends AbstractObjectRegistration<T> implements RpcRegistration<T> {
+    private final class DefaultRpcImplementationRegistration extends AbstractObjectRegistration<T> implements RpcRegistration<T> {
 
 
         protected DefaultRpcImplementationRegistration(final T instance) {

diff --git a/opendaylight/md-sal/sal-binding-broker/src/main/java/org/opendaylight/controller/sal/binding/codegen/impl/RpcRoutingTableImpl.java b/opendaylight/md-sal/sal-binding-broker/src/main/java/org/opendaylight/controller/sal/binding/codegen/impl/RpcRoutingTableImpl.java
index ce159b8f3ed0974b9e47300e1b0aa8a935596e3a..78fa88bb1b24d0c55e966afaaab085b04560ba10 100644 (file)
--- a/opendaylight/md-sal/sal-binding-broker/src/main/java/org/opendaylight/controller/sal/binding/codegen/impl/RpcRoutingTableImpl.java
+++ b/opendaylight/md-sal/sal-binding-broker/src/main/java/org/opendaylight/controller/sal/binding/codegen/impl/RpcRoutingTableImpl.java
@@ -8,10 +8,10 @@
 package org.opendaylight.controller.sal.binding.codegen.impl;
 
 import java.util.Collections;
+import java.util.Iterator;
 import java.util.Map;
 import java.util.concurrent.ConcurrentHashMap;
 import java.util.concurrent.ConcurrentMap;
-
 import org.opendaylight.controller.md.sal.common.api.routing.RouteChangeListener;
 import org.opendaylight.controller.md.sal.common.api.routing.RouteChangePublisher;
 import org.opendaylight.controller.md.sal.common.impl.routing.RoutingUtils;
@@ -25,8 +25,7 @@ import org.opendaylight.yangtools.yang.binding.RpcService;
 import org.slf4j.Logger;
 import org.slf4j.LoggerFactory;
 
-class RpcRoutingTableImpl<C extends BaseIdentity, S extends RpcService> //
-implements //
+final class RpcRoutingTableImpl<C extends BaseIdentity, S extends RpcService> implements
         Mutable, //
         RpcRoutingTable<C, S>, //
         RouteChangePublisher<Class<? extends BaseIdentity>, InstanceIdentifier<?>> {
@@ -42,7 +41,7 @@ implements //
     private RouteChangeListener<Class<? extends BaseIdentity>, InstanceIdentifier<?>> listener;
     private S defaultRoute;
 
-    public RpcRoutingTableImpl(String routerName,Class<C> contextType, Class<S> serviceType) {
+    public RpcRoutingTableImpl(final String routerName,final Class<C> contextType, final Class<S> serviceType) {
         super();
         this.routerName = routerName;
         this.serviceType = serviceType;
@@ -52,7 +51,7 @@ implements //
     }
 
     @Override
-    public void setDefaultRoute(S target) {
+    public void setDefaultRoute(final S target) {
         defaultRoute = target;
     }
 
@@ -63,7 +62,7 @@ implements //
 
     @Override
     public <L extends RouteChangeListener<Class<? extends BaseIdentity>, InstanceIdentifier<?>>> ListenerRegistration<L> registerRouteChangeListener(
-            L listener) {
+            final L listener) {
         return new SingletonListenerRegistration<L>(listener);
     }
 
@@ -74,7 +73,7 @@ implements //
 
     @Override
     @SuppressWarnings("unchecked")
-    public void updateRoute(InstanceIdentifier<?> path, S service) {
+    public void updateRoute(final InstanceIdentifier<?> path, final S service) {
         S previous = this.routes.put(path, service);
 
         LOGGER.debug("Route {} updated to {} in routing table {}",path,service,this);
@@ -88,7 +87,7 @@ implements //
 
     @Override
     @SuppressWarnings("unchecked")
-    public void removeRoute(InstanceIdentifier<?> path) {
+    public void removeRoute(final InstanceIdentifier<?> path) {
         S previous = this.routes.remove(path);
         LOGGER.debug("Route {} to {} removed in routing table {}",path,previous,this);
         @SuppressWarnings("rawtypes")
@@ -98,7 +97,7 @@ implements //
         }
     }
 
-    public void removeRoute(InstanceIdentifier<?> path, S service) {
+    void removeRoute(final InstanceIdentifier<?> path, final S service) {
         @SuppressWarnings("rawtypes")
         RouteChangeListener listenerCapture = listener;
         if (routes.remove(path, service) && listenerCapture != null) {
@@ -108,7 +107,7 @@ implements //
     }
 
     @Override
-    public S getRoute(InstanceIdentifier<?> nodeInstance) {
+    public S getRoute(final InstanceIdentifier<?> nodeInstance) {
         S route = routes.get(nodeInstance);
         if (route != null) {
             return route;
@@ -121,25 +120,28 @@ implements //
         return unmodifiableRoutes;
     }
 
-    protected void removeAllReferences(S service) {
-
+    void removeAllReferences(final S service) {
+        // FIXME: replace this via properly-synchronized BiMap (or something)
+        final Iterator<S> it = routes.values().iterator();
+        while (it.hasNext()) {
+            final S s = it.next();
+            if (service.equals(s)) {
+                it.remove();
+            }
+        }
     }
 
-
-
     @Override
     public String toString() {
         return "RpcRoutingTableImpl [router=" + routerName + ", service=" + serviceType.getSimpleName() + ", context="
                 + contextType.getSimpleName() + "]";
     }
 
-
-
     private class SingletonListenerRegistration<L extends RouteChangeListener<Class<? extends BaseIdentity>, InstanceIdentifier<?>>> extends
             AbstractObjectRegistration<L>
             implements ListenerRegistration<L> {
 
-        public SingletonListenerRegistration(L instance) {
+        public SingletonListenerRegistration(final L instance) {
             super(instance);
             listener = instance;
         }