BUG-2109 : cleaned BGP sessions on session closed.

[bgpcep.git] / bgp / rib-impl / src / main / java / org / opendaylight / protocol / bgp / rib / impl / AbstractBGPSessionNegotiator.java

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 0192bb9daae7d3fb9800779df2d5b47160a31f4a..e865690bbec291ca4da963b577d12fa9492f6b79 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
@@ -10,14 +10,10 @@ package org.opendaylight.protocol.bgp.rib.impl;
 
 import com.google.common.annotations.VisibleForTesting;
 import com.google.common.base.Preconditions;
-
 import io.netty.channel.Channel;
 import io.netty.util.concurrent.Promise;
-
 import java.util.concurrent.TimeUnit;
-
 import javax.annotation.concurrent.GuardedBy;
-
 import org.opendaylight.protocol.bgp.parser.BGPDocumentedException;
 import org.opendaylight.protocol.bgp.parser.BGPError;
 import org.opendaylight.protocol.bgp.rib.impl.spi.BGPPeerRegistry;
@@ -94,11 +90,11 @@ public abstract class AbstractBGPSessionNegotiator extends AbstractSessionNegoti
         Preconditions.checkState(this.state == State.Idle);
 
         // Check if peer is configured in registry before retrieving preferences
-        if (!registry.isPeerConfigured(getRemoteIp())) {
+        if (!this.registry.isPeerConfigured(getRemoteIp())) {
             final BGPDocumentedException cause = new BGPDocumentedException(
                     "BGP peer with ip: " + getRemoteIp()
                     + " not configured, check configured peers in : "
-                    + registry, BGPError.CEASE);
+                    + this.registry, BGPError.CEASE);
             negotiationFailed(cause);
             return;
         }
@@ -112,26 +108,28 @@ public abstract class AbstractBGPSessionNegotiator extends AbstractSessionNegoti
         }
         this.sendMessage(new OpenBuilder().setMyAsNumber(as).setHoldTimer(preferences.getHoldTime()).setBgpIdentifier(
                 preferences.getBgpId()).setBgpParameters(preferences.getParams()).build());
-        this.state = State.OpenSent;
-
-        this.channel.eventLoop().schedule(new Runnable() {
-            @Override
-            public void run() {
-                if (AbstractBGPSessionNegotiator.this.state != State.Finished) {
-                    AbstractBGPSessionNegotiator.this.sendMessage(buildErrorNotify(BGPError.HOLD_TIMER_EXPIRED));
-                    negotiationFailed(new BGPDocumentedException("HoldTimer expired", BGPError.FSM_ERROR));
-                    AbstractBGPSessionNegotiator.this.state = State.Finished;
+        if (this.state != State.Finished) {
+            this.state = State.OpenSent;
+
+            this.channel.eventLoop().schedule(new Runnable() {
+                @Override
+                public void run() {
+                    if (AbstractBGPSessionNegotiator.this.state != State.Finished) {
+                        AbstractBGPSessionNegotiator.this.sendMessage(buildErrorNotify(BGPError.HOLD_TIMER_EXPIRED));
+                        negotiationFailed(new BGPDocumentedException("HoldTimer expired", BGPError.FSM_ERROR));
+                        AbstractBGPSessionNegotiator.this.state = State.Finished;
+                    }
                 }
-            }
-        }, INITIAL_HOLDTIMER, TimeUnit.MINUTES);
+            }, INITIAL_HOLDTIMER, TimeUnit.MINUTES);
+        }
     }
 
     private BGPSessionPreferences getPreferences() {
-        return registry.getPeerPreferences(getRemoteIp());
+        return this.registry.getPeerPreferences(getRemoteIp());
     }
 
     private IpAddress getRemoteIp() {
-        return StrictBGPPeerRegistry.getIpAddress(channel.remoteAddress());
+        return StrictBGPPeerRegistry.getIpAddress(this.channel.remoteAddress());
     }
 
     @Override
@@ -175,14 +173,14 @@ public abstract class AbstractBGPSessionNegotiator extends AbstractSessionNegoti
 
     private void handleOpen(final Open openObj) {
         try {
-            sessionValidator.validate(openObj, getPreferences());
+            this.sessionValidator.validate(openObj, getPreferences());
         } catch (final BGPDocumentedException e) {
             negotiationFailed(e);
             return;
         }
 
         try {
-            final BGPSessionListener peer = registry.getPeer(getRemoteIp(), getSourceId(openObj, getPreferences()), getDestinationId(openObj, getPreferences()));
+            final BGPSessionListener peer = this.registry.getPeer(getRemoteIp(), getSourceId(openObj, getPreferences()), getDestinationId(openObj, getPreferences()));
             this.sendMessage(new KeepaliveBuilder().build());
             this.session = new BGPSessionImpl(peer, this.channel, openObj, getPreferences().getHoldTime());
             this.state = State.OpenConfirm;
@@ -193,9 +191,14 @@ public abstract class AbstractBGPSessionNegotiator extends AbstractSessionNegoti
         }
     }
 
-    private void negotiationFailed(final BGPDocumentedException e) {
+    @Override
+    protected void negotiationFailed(final Throwable e) {
         LOG.warn("Channel {} negotiation failed: {}", this.channel, e.getMessage());
-        this.sendMessage(buildErrorNotify(e.getError()));
+        if (e instanceof BGPDocumentedException) {
+            // although sendMessage() can also result in calling this method, it won't create a cycle. In case sendMessage() fails to
+            // deliver the message, this method gets called with different exception (definitely not with BGPDocumentedException).
+            this.sendMessage(buildErrorNotify(((BGPDocumentedException)e).getError()));
+        }
         super.negotiationFailed(e);
         this.state = State.Finished;
     }