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 3eebcb1fcfaac1a81d946dac1ee3f2078ee445ed..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
@@ -11,18 +11,15 @@ 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.Timeout;
-import io.netty.util.Timer;
-import io.netty.util.TimerTask;
 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.parser.BGPSessionListener;
 import org.opendaylight.protocol.bgp.rib.impl.spi.BGPPeerRegistry;
 import org.opendaylight.protocol.bgp.rib.impl.spi.BGPSessionPreferences;
 import org.opendaylight.protocol.bgp.rib.impl.spi.BGPSessionValidator;
+import org.opendaylight.protocol.bgp.rib.spi.BGPSessionListener;
 import org.opendaylight.protocol.framework.AbstractSessionNegotiator;
 import org.opendaylight.protocol.util.Values;
 import org.opendaylight.yang.gen.v1.urn.ietf.params.xml.ns.yang.ietf.inet.types.rev100924.IpAddress;
@@ -72,7 +69,6 @@ public abstract class AbstractBGPSessionNegotiator extends AbstractSessionNegoti
     }
 
     private static final Logger LOG = LoggerFactory.getLogger(AbstractBGPSessionNegotiator.class);
-    private final Timer timer;
     private final BGPPeerRegistry registry;
     private final BGPSessionValidator sessionValidator;
 
@@ -82,24 +78,23 @@ public abstract class AbstractBGPSessionNegotiator extends AbstractSessionNegoti
     @GuardedBy("this")
     private BGPSessionImpl session;
 
-    public AbstractBGPSessionNegotiator(final Timer timer, final Promise<BGPSessionImpl> promise, final Channel channel,
-                                        final BGPPeerRegistry registry, final BGPSessionValidator sessionValidator) {
+    public AbstractBGPSessionNegotiator(final Promise<BGPSessionImpl> promise, final Channel channel,
+            final BGPPeerRegistry registry, final BGPSessionValidator sessionValidator) {
         super(promise, channel);
         this.registry = registry;
         this.sessionValidator = sessionValidator;
-        this.timer = Preconditions.checkNotNull(timer);
     }
 
     @Override
-    protected void startNegotiation() {
+    protected synchronized void startNegotiation() {
         Preconditions.checkState(this.state == State.Idle);
 
         // Check if peer is configured in registry before retrieving preferences
-        if (registry.isPeerConfigured(getRemoteIp()) == false) {
+        if (!this.registry.isPeerConfigured(getRemoteIp())) {
             final BGPDocumentedException cause = new BGPDocumentedException(
                     "BGP peer with ip: " + getRemoteIp()
-                            + " not configured, check configured peers in : "
-                            + registry, BGPError.CEASE);
+                    + " not configured, check configured peers in : "
+                    + this.registry, BGPError.CEASE);
             negotiationFailed(cause);
             return;
         }
@@ -113,29 +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;
+        if (this.state != State.Finished) {
+            this.state = State.OpenSent;
 
-        final Object lock = this;
-        this.timer.newTimeout(new TimerTask() {
-            @Override
-            public void run(final Timeout timeout) {
-                synchronized (lock) {
+            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
@@ -179,16 +173,16 @@ 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(this.timer, peer, this.channel, openObj, getPreferences().getHoldTime());
+            this.session = new BGPSessionImpl(peer, this.channel, openObj, getPreferences().getHoldTime());
             this.state = State.OpenConfirm;
             LOG.debug("Channel {} moved to OpenConfirm state with remote proposal {}", this.channel, openObj);
         } catch (final BGPDocumentedException e) {
@@ -197,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;
     }