Fix channelInactive event handling in the netty pipeline for netconf.

Reoreding events from AsyncSshHandler.
+ Moving ClosedChannelHandler from Reconnect promise at the end of the pipeline.
+ Add channel id to toString() of abstract netconf session

Change-Id: I9884c2b8d8b2d89878e2fe5cb43fda7a98be5b23
Signed-off-by: Maros Marsalek <mmarsale@cisco.com>

diff --git a/src/main/java/org/opendaylight/protocol/framework/AbstractProtocolSession.java b/src/main/java/org/opendaylight/protocol/framework/AbstractProtocolSession.java
index 47e96d1ff490df392accfb84dc10d55eb84a4583..af196a941a777df18eacc9d2929a3048e57677ab 100644 (file)
--- a/src/main/java/org/opendaylight/protocol/framework/AbstractProtocolSession.java
+++ b/src/main/java/org/opendaylight/protocol/framework/AbstractProtocolSession.java
@@ -37,6 +37,12 @@ public abstract class AbstractProtocolSession<M> extends SimpleChannelInboundHan
     public final void channelInactive(final ChannelHandlerContext ctx) {
         LOG.debug("Channel {} inactive.", ctx.channel());
         endOfInput();
+        try {
+            // Forward channel inactive event, all handlers in pipeline might be interested in the event e.g. close channel handler of reconnect promise
+            super.channelInactive(ctx);
+        } catch (final Exception e) {
+            throw new RuntimeException("Failed to delegate channel inactive event on channel " + ctx.channel(), e);
+        }
     }
 
     @Override

diff --git a/src/main/java/org/opendaylight/protocol/framework/ReconnectPromise.java b/src/main/java/org/opendaylight/protocol/framework/ReconnectPromise.java
index b2ab27a82671cdc0c2380ac8f1084e540ee61691..aaec95a74b5886150c8d0bc4bb7d11a1a1570dbf 100644 (file)
--- a/src/main/java/org/opendaylight/protocol/framework/ReconnectPromise.java
+++ b/src/main/java/org/opendaylight/protocol/framework/ReconnectPromise.java
@@ -47,13 +47,12 @@ final class ReconnectPromise<S extends ProtocolSession<?>, L extends SessionList
         pending = this.dispatcher.createClient(this.address, cs, b, new AbstractDispatcher.PipelineInitializer<S>() {
             @Override
             public void initializeChannel(final SocketChannel channel, final Promise<S> promise) {
-                // add closed channel handler
-                // This handler has to be added before initializer.initializeChannel is called
-                // Initializer might add some handlers using addFirst e.g. AsyncSshHandler and in that case
-                // closed channel handler is before the handler that invokes channel inactive event
-                channel.pipeline().addFirst(new ClosedChannelHandler(ReconnectPromise.this));
-
                 initializer.initializeChannel(channel, promise);
+                // add closed channel handler
+                // This handler has to be added as last channel handler and the channel inactive event has to be caught by it
+                // Handlers in front of it can react to channelInactive event, but have to forward the event or the reconnect will not work
+                // This handler is last so all handlers in front of it can handle channel inactive (to e.g. resource cleanup) before a new connection is started
+                channel.pipeline().addLast(new ClosedChannelHandler(ReconnectPromise.this));
             }
         });
     }
@@ -91,9 +90,7 @@ final class ReconnectPromise<S extends ProtocolSession<?>, L extends SessionList
 
         @Override
         public void channelInactive(final ChannelHandlerContext ctx) throws Exception {
-            // Pass info about disconnect further and then reconnect
-            super.channelInactive(ctx);
-
+            // This is the ultimate channel inactive handler, not forwarding
             if (promise.isCancelled()) {
                 return;
             }