BUG-7937: Fix BGPTerminationReason

When closing the session because of some error
2 Notify messages are sent. One containing the
reason and other with CEASE.
Fix by dont send CEASE Termination Notification
if already other termination notification has
been sent.

Change-Id: Ibfabd7cfb038bef7a452a7458cd5cc1e6fa4b9a7
Signed-off-by: Claudio D. Gasparini <claudio.gasparini@pantheon.tech>
(cherry picked from commit d9a7ab7217f7d14c96356db48e7d6dcdd00601ba)

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 e77c61eebaa2d23b3f48462202c9715fb4f2570e..8575561401e469d84e92c044214aba76721b9087 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
@@ -130,6 +130,7 @@ public class BGPSessionImpl extends SimpleChannelInboundHandler<Notification> im
     private final ChannelOutputLimiter limiter;
 
     private BGPSessionStatsImpl sessionStats;
+    private boolean terminationReasonNotified;
 
     public BGPSessionImpl(final BGPSessionListener listener, final Channel channel, final Open remoteOpen, final BGPSessionPreferences localPreferences,
             final BGPPeerRegistry peerRegistry) {
@@ -185,19 +186,8 @@ public class BGPSessionImpl extends SimpleChannelInboundHandler<Notification> im
         }
 
         if (this.holdTimerValue != 0) {
-            channel.eventLoop().schedule(new Runnable() {
-                @Override
-                public void run() {
-                    handleHoldTimer();
-                }
-            }, this.holdTimerValue, TimeUnit.SECONDS);
-
-            channel.eventLoop().schedule(new Runnable() {
-                @Override
-                public void run() {
-                    handleKeepaliveTimer();
-                }
-            }, this.keepAlive, TimeUnit.SECONDS);
+            channel.eventLoop().schedule(this::handleHoldTimer, this.holdTimerValue, TimeUnit.SECONDS);
+            channel.eventLoop().schedule(this::handleKeepaliveTimer, this.keepAlive, TimeUnit.SECONDS);
         }
         this.bgpId = remoteOpen.getBgpIdentifier();
         this.sessionStats = new BGPSessionStatsImpl(this, remoteOpen, this.holdTimerValue, this.keepAlive, channel, Optional.<BGPSessionPreferences>absent(),
@@ -224,7 +214,7 @@ public class BGPSessionImpl extends SimpleChannelInboundHandler<Notification> im
 
     @Override
     public synchronized void close() {
-        if (this.state != State.IDLE) {
+        if (this.state != State.IDLE && !this.terminationReasonNotified) {
             this.writeAndFlush(new NotifyBuilder().setErrorCode(BGPError.CEASE.getCode()).setErrorSubcode(BGPError.CEASE.getSubcode()).build());
             this.closeWithoutMessage();
         }
@@ -251,9 +241,7 @@ public class BGPSessionImpl extends SimpleChannelInboundHandler<Notification> im
                 // Notifications are handled internally
                 LOG.info("Session closed because Notification message received: {} / {}, data={}", notify.getErrorCode(),
                     notify.getErrorSubcode(), notify.getData() != null ? ByteBufUtil.hexDump(notify.getData()) : null);
-                this.closeWithoutMessage();
-                this.listener.onSessionTerminated(this, new BGPTerminationReason(
-                    BGPError.forValue(notify.getErrorCode(), notify.getErrorSubcode())));
+                notifyTerminationReasonAndCloseWithoutMessage(notify.getErrorCode(), notify.getErrorSubcode());
             } else if (msg instanceof Keepalive) {
                 // Keepalives are handled internally
                 LOG.trace("Received KeepAlive message.");
@@ -277,6 +265,13 @@ public class BGPSessionImpl extends SimpleChannelInboundHandler<Notification> im
         }
     }
 
+    private synchronized void notifyTerminationReasonAndCloseWithoutMessage(final Short errorCode, final Short errorSubcode) {
+        this.terminationReasonNotified = true;
+        this.listener.onSessionTerminated(this, new BGPTerminationReason(
+            BGPError.forValue(errorCode, errorSubcode)));
+        this.closeWithoutMessage();
+    }
+
     synchronized void endOfInput() {
         if (this.state == State.UP) {
             LOG.info(END_OF_INPUT);
@@ -287,14 +282,11 @@ public class BGPSessionImpl extends SimpleChannelInboundHandler<Notification> im
     @GuardedBy("this")
     private ChannelFuture writeEpilogue(final ChannelFuture future, final Notification msg) {
         future.addListener(
-            new ChannelFutureListener() {
-                @Override
-                public void operationComplete(final ChannelFuture f) {
-                    if (!f.isSuccess()) {
-                        LOG.warn("Failed to send message {} to socket {}", msg, BGPSessionImpl.this.channel, f.cause());
-                    } else {
-                        LOG.trace("Message {} sent to socket {}", msg, BGPSessionImpl.this.channel);
-                    }
+            (ChannelFutureListener) f -> {
+                if (!f.isSuccess()) {
+                    LOG.warn("Failed to send message {} to socket {}", msg, BGPSessionImpl.this.channel, f.cause());
+                } else {
+                    LOG.trace("Message {} sent to socket {}", msg, BGPSessionImpl.this.channel);
                 }
             });
         this.lastMessageSentAt = System.nanoTime();
@@ -326,12 +318,7 @@ public class BGPSessionImpl extends SimpleChannelInboundHandler<Notification> im
             return;
         }
         LOG.info("Closing session: {}", this);
-        this.channel.close().addListener(new ChannelFutureListener() {
-            @Override
-            public void operationComplete(final ChannelFuture future) throws Exception {
-                Preconditions.checkArgument(future.isSuccess(), "Channel failed to close: %s", future.cause());
-            }
-        });
+        this.channel.close().addListener((ChannelFutureListener) future -> Preconditions.checkArgument(future.isSuccess(), "Channel failed to close: %s", future.cause()));
         this.state = State.IDLE;
         removePeerSession();
     }
@@ -350,8 +337,7 @@ public class BGPSessionImpl extends SimpleChannelInboundHandler<Notification> im
             builder.setData(data);
         }
         this.writeAndFlush(builder.build());
-        this.listener.onSessionTerminated(this, new BGPTerminationReason(error));
-        this.closeWithoutMessage();
+        notifyTerminationReasonAndCloseWithoutMessage(error.getCode(), error.getSubcode());
     }
 
     private void removePeerSession() {
@@ -378,12 +364,7 @@ public class BGPSessionImpl extends SimpleChannelInboundHandler<Notification> im
             LOG.debug("HoldTimer expired. {}", new Date());
             this.terminate(new BGPDocumentedException(BGPError.HOLD_TIMER_EXPIRED));
         } else {
-            this.channel.eventLoop().schedule(new Runnable() {
-                @Override
-                public void run() {
-                    handleHoldTimer();
-                }
-            }, nextHold - ct, TimeUnit.NANOSECONDS);
+            this.channel.eventLoop().schedule(this::handleHoldTimer, nextHold - ct, TimeUnit.NANOSECONDS);
         }
     }
 
@@ -406,12 +387,7 @@ public class BGPSessionImpl extends SimpleChannelInboundHandler<Notification> im
             nextKeepalive = this.lastMessageSentAt + TimeUnit.SECONDS.toNanos(this.keepAlive);
             this.sessionStats.updateSentMsgKA();
         }
-        this.channel.eventLoop().schedule(new Runnable() {
-            @Override
-            public void run() {
-                handleKeepaliveTimer();
-            }
-        }, nextKeepalive - ct, TimeUnit.NANOSECONDS);
+        this.channel.eventLoop().schedule(this::handleKeepaliveTimer, nextKeepalive - ct, TimeUnit.NANOSECONDS);
     }
 
     @Override
@@ -497,7 +473,7 @@ public class BGPSessionImpl extends SimpleChannelInboundHandler<Notification> im
     }
 
     @Override
-    public void exceptionCaught(final ChannelHandlerContext ctx, final Throwable cause) {
+    public synchronized void exceptionCaught(final ChannelHandlerContext ctx, final Throwable cause) {
         LOG.warn("BGP session encountered error", cause);
         if (cause.getCause() instanceof BGPDocumentedException) {
             this.terminate((BGPDocumentedException) cause.getCause());