From: Maros Marsalek Date: Wed, 20 Nov 2013 09:46:49 +0000 (+0100) Subject: Dispose HashedWheelTimer instances properly in netconf client and server X-Git-Tag: jenkins-controller-bulk-release-prepare-only-2-1~366^2 X-Git-Url: https://git.opendaylight.org/gerrit/gitweb?p=controller.git;a=commitdiff_plain;h=ea7f0e0e25c7c41c314f10d9587b050c24d27344 Dispose HashedWheelTimer instances properly in netconf client and server Prevents resource leak exception from netty Change-Id: I1c12806382ee5d5edf394c2ad5f81c5b0f282fdd Signed-off-by: Maros Marsalek --- diff --git a/opendaylight/netconf/config-netconf-connector/src/main/java/org/opendaylight/controller/netconf/confignetconfconnector/transactions/TransactionProvider.java b/opendaylight/netconf/config-netconf-connector/src/main/java/org/opendaylight/controller/netconf/confignetconfconnector/transactions/TransactionProvider.java index b3483a737a..b8113a0903 100644 --- a/opendaylight/netconf/config-netconf-connector/src/main/java/org/opendaylight/controller/netconf/confignetconfconnector/transactions/TransactionProvider.java +++ b/opendaylight/netconf/config-netconf-connector/src/main/java/org/opendaylight/controller/netconf/confignetconfconnector/transactions/TransactionProvider.java @@ -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(); diff --git a/opendaylight/netconf/config-persister-impl/src/main/java/org/opendaylight/controller/netconf/persist/impl/ConfigPersisterNotificationHandler.java b/opendaylight/netconf/config-persister-impl/src/main/java/org/opendaylight/controller/netconf/persist/impl/ConfigPersisterNotificationHandler.java index a20e00bcff..0d68e25f67 100644 --- a/opendaylight/netconf/config-persister-impl/src/main/java/org/opendaylight/controller/netconf/persist/impl/ConfigPersisterNotificationHandler.java +++ b/opendaylight/netconf/config-persister-impl/src/main/java/org/opendaylight/controller/netconf/persist/impl/ConfigPersisterNotificationHandler.java @@ -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.absent(), nettyThreadgroup, nettyThreadgroup); } public void init() throws InterruptedException { @@ -125,11 +124,12 @@ public class ConfigPersisterNotificationHandler implements NotificationListener, while (true) { attempt++; + netconfClientDispatcher = new NetconfClientDispatcher(Optional.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 currentCapabilities, Set 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 diff --git a/opendaylight/netconf/netconf-client/src/main/java/org/opendaylight/controller/netconf/client/NetconfClientDispatcher.java b/opendaylight/netconf/netconf-client/src/main/java/org/opendaylight/controller/netconf/client/NetconfClientDispatcher.java index 6fc4da026f..62c2113056 100644 --- a/opendaylight/netconf/netconf-client/src/main/java/org/opendaylight/controller/netconf/client/NetconfClientDispatcher.java +++ b/opendaylight/netconf/netconf-client/src/main/java/org/opendaylight/controller/netconf/client/NetconfClientDispatcher.java @@ -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 { +public class NetconfClientDispatcher extends AbstractDispatcher implements Closeable { + + private static final Logger logger = LoggerFactory.getLogger(NetconfClient.class); private final Optional maybeContext; private final NetconfClientSessionNegotiatorFactory negotatorFactory; + private final HashedWheelTimer timer; public NetconfClientDispatcher(final Optional 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 createClient(InetSocketAddress address, @@ -83,4 +90,12 @@ public class NetconfClientDispatcher extends AbstractDispatcher