Replace replaceHelloMessageOutboundHandler()

[netconf.git] / netconf / netconf-netty-util / src / main / java / org / opendaylight / netconf / nettyutil / AbstractNetconfSessionNegotiator.java

diff --git a/netconf/netconf-netty-util/src/main/java/org/opendaylight/netconf/nettyutil/AbstractNetconfSessionNegotiator.java b/netconf/netconf-netty-util/src/main/java/org/opendaylight/netconf/nettyutil/AbstractNetconfSessionNegotiator.java
index c72312fa79558d16110085f7e438aece60a46885..082d4247cc4dc2fdd6d6fef5e4da26a2e7001eb8 100644 (file)
--- a/netconf/netconf-netty-util/src/main/java/org/opendaylight/netconf/nettyutil/AbstractNetconfSessionNegotiator.java
+++ b/netconf/netconf-netty-util/src/main/java/org/opendaylight/netconf/nettyutil/AbstractNetconfSessionNegotiator.java
@@ -134,9 +134,13 @@ public abstract class AbstractNetconfSessionNegotiator<S extends AbstractNetconf
         }
     }
 
-    protected final synchronized boolean ifNegotiatedAlready() {
+    protected final boolean ifNegotiatedAlready() {
         // Indicates whether negotiation already started
-        return this.state != State.IDLE;
+        return state() != State.IDLE;
+    }
+
+    private synchronized State state() {
+        return state;
     }
 
     private static @Nullable SslHandler getSslHandler(final Channel channel) {
@@ -157,9 +161,24 @@ public abstract class AbstractNetconfSessionNegotiator<S extends AbstractNetconf
             return;
         }
 
+        // Catch any exceptions from this point on. Use a named class to ease debugging.
+        final class ExceptionHandlingInboundChannelHandler extends ChannelInboundHandlerAdapter {
+            @Override
+            public void exceptionCaught(final ChannelHandlerContext ctx, final Throwable cause) {
+                LOG.warn("An exception occurred during negotiation with {} on channel {}",
+                        channel.remoteAddress(), channel, cause);
+                // FIXME: this is quite suspect as it is competing with timeoutExpired() without synchronization
+                cancelTimeout();
+                negotiationFailed(cause);
+                changeState(State.FAILED);
+            }
+        }
+
         channel.pipeline().addLast(NAME_OF_EXCEPTION_HANDLER, new ExceptionHandlingInboundChannelHandler());
 
-        replaceHelloMessageOutboundHandler();
+        // Remove special outbound handler for hello message. Insert regular netconf xml message (en|de)coders.
+        replaceChannelHandler(channel, AbstractChannelInitializer.NETCONF_MESSAGE_ENCODER,
+            new NetconfMessageToXMLEncoder());
 
         synchronized (this) {
             lockedChangeState(State.OPEN_WAIT);
@@ -283,14 +302,6 @@ public abstract class AbstractNetconfSessionNegotiator<S extends AbstractNetconf
         }
     }
 
-    /**
-     * Remove special outbound handler for hello message. Insert regular netconf xml message (en|de)coders.
-     */
-    private void replaceHelloMessageOutboundHandler() {
-        replaceChannelHandler(channel, AbstractChannelInitializer.NETCONF_MESSAGE_ENCODER,
-                new NetconfMessageToXMLEncoder());
-    }
-
     private static ChannelHandler replaceChannelHandler(final Channel channel, final String handlerKey,
                                                         final ChannelHandler decoder) {
         return channel.pipeline().replace(handlerKey, handlerKey, decoder);
@@ -331,21 +342,6 @@ public abstract class AbstractNetconfSessionNegotiator<S extends AbstractNetconf
         return false;
     }
 
-    /**
-     * Handler to catch exceptions in pipeline during negotiation.
-     */
-    private final class ExceptionHandlingInboundChannelHandler extends ChannelInboundHandlerAdapter {
-        @Override
-        public void exceptionCaught(final ChannelHandlerContext ctx, final Throwable cause) {
-            LOG.warn("An exception occurred during negotiation with {} on channel {}",
-                    channel.remoteAddress(), channel, cause);
-            // FIXME: this is quite suspect as it is competing with timeoutExpired() without synchronization
-            cancelTimeout();
-            negotiationFailed(cause);
-            changeState(State.FAILED);
-        }
-    }
-
     protected final void negotiationSuccessful(final S session) {
         LOG.debug("Negotiation on channel {} successful with session {}", channel, session);
         channel.pipeline().replace(this, "session", session);
@@ -373,6 +369,11 @@ public abstract class AbstractNetconfSessionNegotiator<S extends AbstractNetconf
     @Override
     @SuppressWarnings("checkstyle:illegalCatch")
     public final void channelRead(final ChannelHandlerContext ctx, final Object msg) {
+        if (state() == State.FAILED) {
+            // We have already failed -- do not process any more messages
+            return;
+        }
+
         LOG.debug("Negotiation read invoked on channel {}", channel);
         try {
             handleMessage((NetconfHelloMessage) msg);