BUG-4867: AdjRibOutListener still listening changes after peer Session closed

Once Peer session is closed. Listener needs to be removed.

Change-Id: Ie3575bdd42e9b0924e48f110652d0c595b42804c
Signed-off-by: Claudio D. Gasparini <cgaspari@cisco.com>

diff --git a/bgp/rib-impl/src/main/java/org/opendaylight/protocol/bgp/rib/impl/AbstractBGPSessionNegotiator.java b/bgp/rib-impl/src/main/java/org/opendaylight/protocol/bgp/rib/impl/AbstractBGPSessionNegotiator.java
index 013f02d5846dc7aa43d5915cbacb55c9aa340d98..f7f10bb63080274c1f3f86c26a50cc02697e1101 100644 (file)
--- a/bgp/rib-impl/src/main/java/org/opendaylight/protocol/bgp/rib/impl/AbstractBGPSessionNegotiator.java
+++ b/bgp/rib-impl/src/main/java/org/opendaylight/protocol/bgp/rib/impl/AbstractBGPSessionNegotiator.java
@@ -246,7 +246,7 @@ public abstract class AbstractBGPSessionNegotiator extends ChannelInboundHandler
             @Override
             public void operationComplete(final ChannelFuture f) {
                 if (!f.isSuccess()) {
-                    LOG.info("Failed to send message {}", msg, f.cause());
+                    LOG.warn("Failed to send message {}", msg, f.cause());
                     negotiationFailedCloseChannel(f.cause());
                 } else {
                     LOG.trace("Message {} sent to socket", msg);

diff --git a/bgp/rib-impl/src/main/java/org/opendaylight/protocol/bgp/rib/impl/AdjRibOutListener.java b/bgp/rib-impl/src/main/java/org/opendaylight/protocol/bgp/rib/impl/AdjRibOutListener.java
index 19625a8845ff7beefb6132afd9b906c82e165c7d..2d566f4b8bfc8fd11744c3e96e3d9b4327ec4788 100644 (file)
--- a/bgp/rib-impl/src/main/java/org/opendaylight/protocol/bgp/rib/impl/AdjRibOutListener.java
+++ b/bgp/rib-impl/src/main/java/org/opendaylight/protocol/bgp/rib/impl/AdjRibOutListener.java
@@ -34,6 +34,7 @@ import org.opendaylight.yang.gen.v1.urn.opendaylight.params.xml.ns.yang.bgp.rib.
 import org.opendaylight.yang.gen.v1.urn.opendaylight.params.xml.ns.yang.bgp.rib.rev130925.bgp.rib.rib.peer.AdjRibOut;
 import org.opendaylight.yang.gen.v1.urn.opendaylight.params.xml.ns.yang.bgp.rib.rev130925.rib.Tables;
 import org.opendaylight.yang.gen.v1.urn.opendaylight.params.xml.ns.yang.bgp.rib.rev130925.rib.TablesKey;
+import org.opendaylight.yangtools.concepts.ListenerRegistration;
 import org.opendaylight.yangtools.yang.common.QName;
 import org.opendaylight.yangtools.yang.data.api.YangInstanceIdentifier;
 import org.opendaylight.yangtools.yang.data.api.schema.ContainerNode;
@@ -60,6 +61,7 @@ final class AdjRibOutListener implements DOMDataTreeChangeListener {
     private final Codecs codecs;
     private final RIBSupport support;
     private final boolean mpSupport;
+    private final ListenerRegistration<AdjRibOutListener> registerDataTreeChangeListener;
 
     private AdjRibOutListener(final PeerId peerId, final TablesKey tablesKey, final YangInstanceIdentifier ribId,
         final CodecsRegistry registry, final RIBSupport support, final DOMDataTreeChangeService service,
@@ -69,7 +71,7 @@ final class AdjRibOutListener implements DOMDataTreeChangeListener {
         this.codecs = registry.getCodecs(this.support);
         this.mpSupport = mpSupport;
         final YangInstanceIdentifier adjRibOutId =  ribId.node(Peer.QNAME).node(IdentifierUtils.domPeerId(peerId)).node(AdjRibOut.QNAME).node(Tables.QNAME).node(RibSupportUtils.toYangTablesKey(tablesKey));
-        service.registerDataTreeChangeListener(new DOMDataTreeIdentifier(LogicalDatastoreType.OPERATIONAL, adjRibOutId), this);
+        this.registerDataTreeChangeListener = service.registerDataTreeChangeListener(new DOMDataTreeIdentifier(LogicalDatastoreType.OPERATIONAL, adjRibOutId), this);
     }
 
     static AdjRibOutListener create(@Nonnull final PeerId peerId, @Nonnull final TablesKey tablesKey, @Nonnull final YangInstanceIdentifier ribId, @Nonnull final CodecsRegistry registry, @Nonnull final RIBSupport support, @Nonnull final DOMDataTreeChangeService service, @Nonnull final ChannelOutputLimiter session, @Nonnull final boolean mpSupport) {
@@ -150,4 +152,8 @@ final class AdjRibOutListener implements DOMDataTreeChangeListener {
         }
         return prefs;
     }
+
+    public void close() {
+        this.registerDataTreeChangeListener.close();
+    }
 }

diff --git a/bgp/rib-impl/src/main/java/org/opendaylight/protocol/bgp/rib/impl/BGPPeer.java b/bgp/rib-impl/src/main/java/org/opendaylight/protocol/bgp/rib/impl/BGPPeer.java
index 973312e02432f69061604787f5d1e154b93f5f62..e7212e3062b68ffd94fdb18361f71a87758195b9 100644 (file)
--- a/bgp/rib-impl/src/main/java/org/opendaylight/protocol/bgp/rib/impl/BGPPeer.java
+++ b/bgp/rib-impl/src/main/java/org/opendaylight/protocol/bgp/rib/impl/BGPPeer.java
@@ -85,6 +85,7 @@ public class BGPPeer implements BGPSessionListener, Peer, AutoCloseable, BGPPeer
     private BGPPeerRuntimeRegistrator registrator;
     private BGPPeerRuntimeRegistration runtimeReg;
     private long sessionEstablishedCounter = 0L;
+    private AdjRibOutListener adjRibOutListener;
 
     public BGPPeer(final String name, final RIB rib) {
         this(name, rib, PeerRole.Ibgp);
@@ -227,13 +228,14 @@ public class BGPPeer implements BGPSessionListener, Peer, AutoCloseable, BGPPeer
 
         // not particularly nice
         if (context != null && this.session instanceof BGPSessionImpl) {
-            AdjRibOutListener.create(peerId, key, this.rib.getYangRibId(), this.rib.getCodecsRegistry(), context.getRibSupport(), ((RIBImpl) this.rib).getService(),
-                ((BGPSessionImpl) this.session).getLimiter(), mpSupport);
+            this.adjRibOutListener = AdjRibOutListener.create(peerId, key, this.rib.getYangRibId(), this.rib.getCodecsRegistry(), context.getRibSupport(),
+                ((RIBImpl) this.rib).getService(), ((BGPSessionImpl) this.session).getLimiter(), mpSupport);
         }
     }
 
     private synchronized void cleanup() {
-        // FIXME: BUG-196: support graceful restart
+        // FIXME: BUG-196: support graceful
+        this.adjRibOutListener.close();
         this.ribWriter.cleanTables(this.tables);
         this.tables.clear();
     }

diff --git a/bgp/rib-impl/src/main/java/org/opendaylight/protocol/bgp/rib/impl/BGPSessionImpl.java b/bgp/rib-impl/src/main/java/org/opendaylight/protocol/bgp/rib/impl/BGPSessionImpl.java
index f9a525a246c3fdeddb6b9b40f220d896374d3da5..dc5aa448e2592544bc1537c66dd313b88875b72c 100644 (file)
--- a/bgp/rib-impl/src/main/java/org/opendaylight/protocol/bgp/rib/impl/BGPSessionImpl.java
+++ b/bgp/rib-impl/src/main/java/org/opendaylight/protocol/bgp/rib/impl/BGPSessionImpl.java
@@ -236,7 +236,7 @@ public class BGPSessionImpl extends SimpleChannelInboundHandler<Notification> im
                 @Override
                 public void operationComplete(final ChannelFuture f) {
                     if (!f.isSuccess()) {
-                        LOG.info("Failed to send message {} to socket {}", msg, f.cause(), BGPSessionImpl.this.channel);
+                        LOG.warn("Failed to send message {} to socket {}", msg, f.cause(), BGPSessionImpl.this.channel);
                     } else {
                         LOG.trace("Message {} sent to socket {}", msg, BGPSessionImpl.this.channel);
                     }

diff --git a/bgp/rib-impl/src/test/java/org/opendaylight/protocol/bgp/rib/impl/AbstractRIBTestSetup.java b/bgp/rib-impl/src/test/java/org/opendaylight/protocol/bgp/rib/impl/AbstractRIBTestSetup.java
index 5d3e605db7adc5ffe3abb3d5d044eb8631f7cb66..dd96631af7b8afc17265ff1bfab675522c61946f 100644 (file)
--- a/bgp/rib-impl/src/test/java/org/opendaylight/protocol/bgp/rib/impl/AbstractRIBTestSetup.java
+++ b/bgp/rib-impl/src/test/java/org/opendaylight/protocol/bgp/rib/impl/AbstractRIBTestSetup.java
@@ -59,6 +59,7 @@ import org.opendaylight.yangtools.binding.data.codec.api.BindingCodecTreeFactory
 import org.opendaylight.yangtools.binding.data.codec.gen.impl.DataObjectSerializerGenerator;
 import org.opendaylight.yangtools.binding.data.codec.gen.impl.StreamWriterGenerator;
 import org.opendaylight.yangtools.binding.data.codec.impl.BindingNormalizedNodeCodecRegistry;
+import org.opendaylight.yangtools.concepts.ListenerRegistration;
 import org.opendaylight.yangtools.sal.binding.generator.api.ClassLoadingStrategy;
 import org.opendaylight.yangtools.sal.binding.generator.impl.GeneratedClassLoadingStrategy;
 import org.opendaylight.yangtools.sal.binding.generator.impl.ModuleInfoBackedContext;
@@ -168,7 +169,7 @@ public class AbstractRIBTestSetup {
     private void mockedMethods() throws Exception {
         MockitoAnnotations.initMocks(this);
         final ReadOnlyTransaction readTx = Mockito.mock(ReadOnlyTransaction.class);
-        Mockito.doReturn(null).when(this.service).registerDataTreeChangeListener(Mockito.any(DOMDataTreeIdentifier.class), Mockito.any(DOMDataTreeChangeListener.class));
+        Mockito.doReturn(new listenerRegistration()).when(this.service).registerDataTreeChangeListener(Mockito.any(DOMDataTreeIdentifier.class), Mockito.any(DOMDataTreeChangeListener.class));
         final Map<Class<? extends DOMDataBrokerExtension>, DOMDataBrokerExtension> map = new HashMap<>();
         map.put(DOMDataTreeChangeService.class, this.service);
         Mockito.doNothing().when(readTx).close();
@@ -243,4 +244,14 @@ public class AbstractRIBTestSetup {
     public void tearDown() {
         this.a1.close();
     }
+
+    private class listenerRegistration implements ListenerRegistration {
+        @Override
+        public Object getInstance() {
+            return null;
+        }
+
+        public void close() {
+        }
+    }
 }