Dispose HashedWheelTimer instances properly in netconf client and server 98/2898/2
authorMaros Marsalek <mmarsale@cisco.com>
Wed, 20 Nov 2013 09:46:49 +0000 (10:46 +0100)
committerMaros Marsalek <mmarsale@cisco.com>
Wed, 20 Nov 2013 14:50:53 +0000 (15:50 +0100)
Prevents resource leak exception from netty

Change-Id: I1c12806382ee5d5edf394c2ad5f81c5b0f282fdd
Signed-off-by: Maros Marsalek <mmarsale@cisco.com>
opendaylight/netconf/config-netconf-connector/src/main/java/org/opendaylight/controller/netconf/confignetconfconnector/transactions/TransactionProvider.java
opendaylight/netconf/config-persister-impl/src/main/java/org/opendaylight/controller/netconf/persist/impl/ConfigPersisterNotificationHandler.java
opendaylight/netconf/netconf-client/src/main/java/org/opendaylight/controller/netconf/client/NetconfClientDispatcher.java
opendaylight/netconf/netconf-impl/src/main/java/org/opendaylight/controller/netconf/impl/osgi/NetconfImplActivator.java
opendaylight/netconf/netconf-it/src/test/java/org/opendaylight/controller/netconf/it/NetconfITTest.java

index b3483a7..b8113a0 100644 (file)
@@ -41,12 +41,12 @@ public class TransactionProvider implements AutoCloseable {
     @Override
     public synchronized void close() {
         for (ObjectName tx : allOpenedTransactions) {
-            if (isStillOpenTransaction(tx)) {
-                try {
+            try {
+                if (isStillOpenTransaction(tx)) {
                     configRegistryClient.getConfigTransactionClient(tx).abortConfig();
-                } catch (Exception e) {
-                    logger.debug("Ignoring {} while closing transaction {}", e.toString(), tx, e);
                 }
+            } catch (Exception e) {
+                logger.debug("Ignoring exception while closing transaction {}", tx, e);
             }
         }
         allOpenedTransactions.clear();
index a20e00b..0d68e25 100644 (file)
@@ -54,9 +54,9 @@ public class ConfigPersisterNotificationHandler implements NotificationListener,
     private static final Logger logger = LoggerFactory.getLogger(ConfigPersisterNotificationHandler.class);
 
     private final InetSocketAddress address;
-    private final NetconfClientDispatcher dispatcher;
     private final EventLoopGroup nettyThreadgroup;
 
+    private NetconfClientDispatcher netconfClientDispatcher;
     private NetconfClient netconfClient;
 
     private final Persister persister;
@@ -81,7 +81,6 @@ public class ConfigPersisterNotificationHandler implements NotificationListener,
         this.timeout = timeout;
 
         this.nettyThreadgroup = new NioEventLoopGroup();
-        this.dispatcher = new NetconfClientDispatcher(Optional.<SSLContext>absent(), nettyThreadgroup, nettyThreadgroup);
     }
 
     public void init() throws InterruptedException {
@@ -125,11 +124,12 @@ public class ConfigPersisterNotificationHandler implements NotificationListener,
         while (true) {
             attempt++;
 
+            netconfClientDispatcher = new NetconfClientDispatcher(Optional.<SSLContext>absent(), nettyThreadgroup, nettyThreadgroup);
             try {
-                netconfClient = new NetconfClient(this.toString(), address, delay, dispatcher);
-                // TODO is this correct ex to catch ?
+                netconfClient = new NetconfClient(this.toString(), address, delay, netconfClientDispatcher);
             } catch (IllegalStateException e) {
                 logger.debug("Netconf {} was not initialized or is not stable, attempt {}", address, attempt, e);
+                netconfClientDispatcher.close();
                 Thread.sleep(delay);
                 continue;
             }
@@ -148,11 +148,7 @@ public class ConfigPersisterNotificationHandler implements NotificationListener,
 
             logger.debug("Polling hello from netconf, attempt {}, capabilities {}", attempt, currentCapabilities);
 
-            try {
-                netconfClient.close();
-            } catch (IOException e) {
-                throw new RuntimeException("Error closing temporary client " + netconfClient);
-            }
+            closeClientAndDispatcher(netconfClient, netconfClientDispatcher);
 
             Thread.sleep(delay);
         }
@@ -162,6 +158,25 @@ public class ConfigPersisterNotificationHandler implements NotificationListener,
 
     }
 
+    private static void closeClientAndDispatcher(Closeable client, Closeable dispatcher) {
+        Exception fromClient = null;
+        try {
+            client.close();
+        } catch (Exception e) {
+            fromClient = e;
+        } finally {
+            try {
+                dispatcher.close();
+            } catch (Exception e) {
+                if (fromClient != null) {
+                    e.addSuppressed(fromClient);
+                }
+
+                throw new RuntimeException("Error closing temporary client ", e);
+            }
+        }
+    }
+
     private boolean isSubset(Set<String> currentCapabilities, Set<String> expectedCaps) {
         for (String exCap : expectedCaps) {
             if (currentCapabilities.contains(exCap) == false)
@@ -318,10 +333,18 @@ public class ConfigPersisterNotificationHandler implements NotificationListener,
             }
         }
 
+        if (netconfClientDispatcher != null) {
+            try {
+                netconfClientDispatcher.close();
+            } catch (Exception e) {
+                logger.warn("Unable to close connection to netconf {}", netconfClientDispatcher, e);
+            }
+        }
+
         try {
             nettyThreadgroup.shutdownGracefully();
         } catch (Exception e) {
-            logger.warn("Unable to close netconf client thread group {}", dispatcher, e);
+            logger.warn("Unable to close netconf client thread group {}", netconfClientDispatcher, e);
         }
 
         // unregister from JMX
index 6fc4da0..62c2113 100644 (file)
@@ -23,20 +23,27 @@ import org.opendaylight.protocol.framework.AbstractDispatcher;
 import org.opendaylight.protocol.framework.ReconnectStrategy;
 import org.opendaylight.protocol.framework.SessionListener;
 import org.opendaylight.protocol.framework.SessionListenerFactory;
+import org.slf4j.Logger;
+import org.slf4j.LoggerFactory;
 
 import javax.net.ssl.SSLContext;
 import javax.net.ssl.SSLEngine;
+import java.io.Closeable;
 import java.net.InetSocketAddress;
 
-public class NetconfClientDispatcher extends AbstractDispatcher<NetconfClientSession, NetconfClientSessionListener> {
+public class NetconfClientDispatcher extends AbstractDispatcher<NetconfClientSession, NetconfClientSessionListener> implements Closeable {
+
+    private static final Logger logger = LoggerFactory.getLogger(NetconfClient.class);
 
     private final Optional<SSLContext> maybeContext;
     private final NetconfClientSessionNegotiatorFactory negotatorFactory;
+    private final HashedWheelTimer timer;
 
     public NetconfClientDispatcher(final Optional<SSLContext> maybeContext, EventLoopGroup bossGroup, EventLoopGroup workerGroup) {
         super(bossGroup, workerGroup);
         this.maybeContext = Preconditions.checkNotNull(maybeContext);
-        this.negotatorFactory = new NetconfClientSessionNegotiatorFactory(new HashedWheelTimer());
+        timer = new HashedWheelTimer();
+        this.negotatorFactory = new NetconfClientSessionNegotiatorFactory(timer);
     }
 
     public Future<NetconfClientSession> createClient(InetSocketAddress address,
@@ -83,4 +90,12 @@ public class NetconfClientDispatcher extends AbstractDispatcher<NetconfClientSes
         }
     }
 
+    @Override
+    public void close() {
+        try {
+            timer.stop();
+        } catch (Exception e) {
+            logger.debug("Ignoring exception while closing {}", timer, e);
+        }
+    }
 }
index 1a4888b..890bbe7 100644 (file)
@@ -37,6 +37,7 @@ public class NetconfImplActivator implements BundleActivator {
     private DefaultCommitNotificationProducer commitNot;
     private NetconfServerDispatcher dispatch;
     private NioEventLoopGroup eventLoopGroup;
+    private HashedWheelTimer timer;
 
     @Override
     public void start(final BundleContext context) throws Exception {
@@ -50,8 +51,9 @@ public class NetconfImplActivator implements BundleActivator {
         factoriesTracker.open();
 
         SessionIdProvider idProvider = new SessionIdProvider();
+        timer = new HashedWheelTimer();
         NetconfServerSessionNegotiatorFactory serverNegotiatorFactory = new NetconfServerSessionNegotiatorFactory(
-                new HashedWheelTimer(), factoriesListener, idProvider);
+                timer, factoriesListener, idProvider);
 
         commitNot = new DefaultCommitNotificationProducer(ManagementFactory.getPlatformMBeanServer());
 
@@ -88,5 +90,6 @@ public class NetconfImplActivator implements BundleActivator {
 
         commitNot.close();
         eventLoopGroup.shutdownGracefully();
+        timer.stop();
     }
 }
index 403ba3d..5f6e046 100644 (file)
@@ -134,6 +134,7 @@ public class NetconfITTest extends AbstractConfigTest {
     public void tearDown() throws Exception {
         commitNot.close();
         nettyThreadgroup.shutdownGracefully();
+        clientDispatcher.close();
     }
 
     private void loadMessages() throws IOException, SAXException, ParserConfigurationException {