From 178b6ae3c5e28eb77c4389f8d61e532363875f17 Mon Sep 17 00:00:00 2001
From: Bernd Gutjahr <bernd.gutjahr@hpe.com>
Date: Fri, 22 Apr 2016 08:22:41 +0200
Subject: [PATCH] ARTEMIS-497 Prevent 10 second stalls when closing an SSL
 connection

When NettyConnection.classSSLAndChannel is called from the EventLoop,
waiting for the SSL handler to close will always take 10 seconds, because
the sslCloseFuture is from a task that is scheduled with the same
EventLoop. But since the EventLoop is a single threaded executor, it
will only be executed after the current task is completed.

Due to the single threaded nature of the EventLoop, all blocking calls
should be avoided. Therefore, I removed both awaitUninterruptibly calls
if the closing happens within an event loop tasks. As a side effect,
the annoying server log timeout warnings will go away.
---
 .../remoting/impl/netty/NettyConnection.java  | 26 ++++++++++++-------
 1 file changed, 17 insertions(+), 9 deletions(-)

diff --git a/artemis-core-client/src/main/java/org/apache/activemq/artemis/core/remoting/impl/netty/NettyConnection.java b/artemis-core-client/src/main/java/org/apache/activemq/artemis/core/remoting/impl/netty/NettyConnection.java
index 69478832a5a..a8c80c6b3ff 100644
--- a/artemis-core-client/src/main/java/org/apache/activemq/artemis/core/remoting/impl/netty/NettyConnection.java
+++ b/artemis-core-client/src/main/java/org/apache/activemq/artemis/core/remoting/impl/netty/NettyConnection.java
@@ -29,6 +29,8 @@
 import io.netty.channel.ChannelPromise;
 import io.netty.channel.EventLoop;
 import io.netty.handler.ssl.SslHandler;
+import io.netty.util.concurrent.GenericFutureListener;
+
 import org.apache.activemq.artemis.api.core.ActiveMQBuffer;
 import org.apache.activemq.artemis.api.core.ActiveMQBuffers;
 import org.apache.activemq.artemis.api.core.ActiveMQInterruptedException;
@@ -193,13 +195,13 @@ public void close() {
       boolean inEventLoop = eventLoop.inEventLoop();
       //if we are in an event loop we need to close the channel after the writes have finished
       if (!inEventLoop) {
-         closeSSLAndChannel(sslHandler, channel);
+         closeSSLAndChannel(sslHandler, channel, false);
       }
       else {
          eventLoop.execute(new Runnable() {
             @Override
             public void run() {
-               closeSSLAndChannel(sslHandler, channel);
+               closeSSLAndChannel(sslHandler, channel, true);
             }
          });
       }
@@ -412,12 +414,17 @@ public String toString() {
 
    // Private -------------------------------------------------------
 
-   private void closeSSLAndChannel(SslHandler sslHandler, Channel channel) {
+   private void closeSSLAndChannel(SslHandler sslHandler, final Channel channel, boolean inEventLoop) {
       if (sslHandler != null) {
          try {
             ChannelFuture sslCloseFuture = sslHandler.close();
-
-            if (!sslCloseFuture.awaitUninterruptibly(10000)) {
+            sslCloseFuture.addListener(new GenericFutureListener<ChannelFuture>() {
+               @Override
+               public void operationComplete(ChannelFuture future) throws Exception {
+                  channel.close();
+               }
+            });
+            if (!inEventLoop && !sslCloseFuture.awaitUninterruptibly(10000)) {
                ActiveMQClientLogger.LOGGER.timeoutClosingSSL();
             }
          }
@@ -425,10 +432,11 @@ private void closeSSLAndChannel(SslHandler sslHandler, Channel channel) {
             // ignore
          }
       }
-
-      ChannelFuture closeFuture = channel.close();
-      if (!closeFuture.awaitUninterruptibly(10000)) {
-         ActiveMQClientLogger.LOGGER.timeoutClosingNettyChannel();
+      else {
+         ChannelFuture closeFuture = channel.close();
+         if (!inEventLoop && !closeFuture.awaitUninterruptibly(10000)) {
+            ActiveMQClientLogger.LOGGER.timeoutClosingNettyChannel();
+         }
       }
    }
    // Inner classes -------------------------------------------------