From: Tony Tkacik Date: Mon, 15 Dec 2014 07:56:57 +0000 (+0000) Subject: Merge "BUG-2510: Remove all registrations when a routed rpc is closed" X-Git-Tag: release/lithium~769 X-Git-Url: https://git.opendaylight.org/gerrit/gitweb?p=controller.git;a=commitdiff_plain;h=27a4009ad3e151118c6d76c5c4c0f56def4118d8;hp=da7b79cbf4b48a928910796150da4cb128848783 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 783e5c0cd4..d69aeed352 100644 --- 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 implements // RpcRouter, RouteChangeListener, InstanceIdentifier> { @@ -133,7 +133,14 @@ RpcRouter, RouteChangeListener, InstanceIdentif return new DefaultRpcImplementationRegistration(service); } - private class RoutedRpcRegistrationImpl extends AbstractObjectRegistration implements RoutedRpcRegistration { + private final class RoutedRpcRegistrationImpl extends AbstractObjectRegistration implements RoutedRpcRegistration { + /* + * 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> contexts = new ArrayList<>(1); public RoutedRpcRegistrationImpl(final T instance) { super(instance); @@ -145,32 +152,49 @@ RpcRouter, RouteChangeListener, InstanceIdentif } @Override - public void registerPath(final Class context, final InstanceIdentifier path) { + public synchronized void registerPath(final Class 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 context, final InstanceIdentifier path) { + public synchronized void unregisterPath(final Class 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 context, final InstanceIdentifier instance) { registerPath(context, instance); } + @Deprecated @Override public void unregisterInstance(final Class context, final InstanceIdentifier instance) { unregisterPath(context, instance); } @Override - protected void removeRegistration() { - + protected synchronized void removeRegistration() { + for (Class ctx : contexts) { + routingTables.get(ctx).removeAllReferences(getInstance()); + } + contexts.clear(); } } - private class DefaultRpcImplementationRegistration extends AbstractObjectRegistration implements RpcRegistration { + private final class DefaultRpcImplementationRegistration extends AbstractObjectRegistration implements RpcRegistration { 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 ce159b8f3e..78fa88bb1b 100644 --- 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 // -implements // +final class RpcRoutingTableImpl implements Mutable, // RpcRoutingTable, // RouteChangePublisher, InstanceIdentifier> { @@ -42,7 +41,7 @@ implements // private RouteChangeListener, InstanceIdentifier> listener; private S defaultRoute; - public RpcRoutingTableImpl(String routerName,Class contextType, Class serviceType) { + public RpcRoutingTableImpl(final String routerName,final Class contextType, final Class 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 , InstanceIdentifier>> ListenerRegistration registerRouteChangeListener( - L listener) { + final L listener) { return new SingletonListenerRegistration(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 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, InstanceIdentifier>> extends AbstractObjectRegistration implements ListenerRegistration { - public SingletonListenerRegistration(L instance) { + public SingletonListenerRegistration(final L instance) { super(instance); listener = instance; }