Do not reuse inactive handler

We have a simple event dispatch handler here, which is stateless and can
be reused, except Netty's defences are rejecting such reuse. Since it is
a very simple object, just do not bother with its reuse.

JIRA: NETCONF-852
Change-Id: I40b48b0a8e14a0a271043bb9fb4b471cfd56a8e7
Signed-off-by: Robert Varga <robert.varga@pantheon.tech>
(cherry picked from commit 438ed86b4a3e80cd18f52b40e6faf4260e14d6ac)

diff --git a/netconf/netconf-netty-util/src/main/java/org/opendaylight/netconf/nettyutil/ReconnectPromise.java b/netconf/netconf-netty-util/src/main/java/org/opendaylight/netconf/nettyutil/ReconnectPromise.java
index b4799811a9c03e519c7e68270a4e3934f281d26f..00345e17c97890180f1fffa8064bc7c01f7d350f 100644 (file)
--- a/netconf/netconf-netty-util/src/main/java/org/opendaylight/netconf/nettyutil/ReconnectPromise.java
+++ b/netconf/netconf-netty-util/src/main/java/org/opendaylight/netconf/nettyutil/ReconnectPromise.java
@@ -9,6 +9,7 @@ package org.opendaylight.netconf.nettyutil;
 
 import static java.util.Objects.requireNonNull;
 
+import edu.umd.cs.findbugs.annotations.SuppressFBWarnings;
 import io.netty.bootstrap.Bootstrap;
 import io.netty.channel.ChannelHandlerContext;
 import io.netty.channel.ChannelInboundHandlerAdapter;
@@ -37,30 +38,6 @@ final class ReconnectPromise<S extends NetconfSession, L extends NetconfSessionL
     private final Bootstrap bootstrap;
     private final PipelineInitializer<S> initializer;
     private final Promise<Empty> firstSessionFuture;
-    /**
-     * Channel handler that responds to channelInactive event and reconnects the session unless the promise is
-     * cancelled.
-     */
-    private final ChannelInboundHandlerAdapter inboundHandler = new ChannelInboundHandlerAdapter() {
-        @Override
-        public void channelInactive(final ChannelHandlerContext ctx) {
-            // This is the ultimate channel inactive handler, not forwarding
-            if (isCancelled()) {
-                return;
-            }
-
-            synchronized (ReconnectPromise.this) {
-                final Future<?> attempt = pending;
-                if (!attempt.isDone() || !attempt.isSuccess()) {
-                    // Connection refused, negotiation failed, or similar
-                    LOG.debug("Connection to {} was dropped during negotiation, reattempting", address);
-                }
-
-                LOG.debug("Reconnecting after connection to {} was dropped", address);
-                lockedConnect();
-            }
-        }
-    };
 
     @GuardedBy("this")
     private Future<?> pending;
@@ -111,7 +88,12 @@ final class ReconnectPromise<S extends NetconfSession, L extends NetconfSessionL
             // 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(inboundHandler);
+            channel.pipeline().addLast(new ChannelInboundHandlerAdapter() {
+                @Override
+                public void channelInactive(final ChannelHandlerContext ctx) {
+                    onChannelInactive();
+                }
+            });
         });
 
         if (!firstSessionFuture.isDone()) {
@@ -122,4 +104,24 @@ final class ReconnectPromise<S extends NetconfSession, L extends NetconfSessionL
             });
         }
     }
+
+    @SuppressFBWarnings(value = "UPM_UNCALLED_PRIVATE_METHOD",
+        justification = "https://github.com/spotbugs/spotbugs/issues/811")
+    private void onChannelInactive() {
+        // This is the ultimate channel inactive handler, not forwarding
+        if (isCancelled()) {
+            return;
+        }
+
+        synchronized (this) {
+            final Future<?> attempt = pending;
+            if (!attempt.isDone() || !attempt.isSuccess()) {
+                // Connection refused, negotiation failed, or similar
+                LOG.debug("Connection to {} was dropped during negotiation, reattempting", address);
+            }
+
+            LOG.debug("Reconnecting after connection to {} was dropped", address);
+            lockedConnect();
+        }
+    }
 }