Make sure to notify handshake success even if SSLEngine is closed

Related:

e9685ea45a

Motivation:

SslHandler.unwrap() does not evaluate the handshake status of
SSLEngine.unwrap() when the status of SSLEngine.unwrap() is CLOSED.

It is not correct because the status does not reflect the state of the
handshake currently in progress, accoding to the API documentation of
SSLEngineResult.Status.

Also, sslCloseFuture can be notified earlier than handshake notification
because we call sslCloseFuture.trySuccess() before evaluating handshake
status.

Modifications:

- Notify sslCloseFuture after the unwrap loop is finished
- Add more assertions to SocketSslEchoTest

Result:

Potentially fix the regression caused by:
- e9685ea45a

handler/src/main/java/io/netty/handler/ssl/SslHandler.java

@@ -887,6 +887,7 @@ public class SslHandler extends ByteToMessageDecoder {
         }
 
         boolean wrapLater = false;
+        boolean notifyClosure = false;
         ByteBuf decodeOut = allocate(ctx, initialOutAppBufCapacity);
         try {
             for (;;) {
@@ -898,8 +899,7 @@ public class SslHandler extends ByteToMessageDecoder {
 
                 if (status == Status.CLOSED) {
                     // notify about the CLOSED state of the SSLEngine. See #137
-                    sslCloseFuture.trySuccess(ctx.channel());
+                    notifyClosure = true;
-                    break;
                 }
 
                 switch (handshakeStatus) {
@@ -941,6 +941,10 @@ public class SslHandler extends ByteToMessageDecoder {
             if (wrapLater) {
                 wrap(ctx, true);
             }
+
+            if (notifyClosure) {
+                sslCloseFuture.trySuccess(ctx.channel());
+            }
         } catch (SSLException e) {
             setHandshakeFailure(e);
             throw e;

testsuite/src/test/java/io/netty/testsuite/transport/socket/SocketSslEchoTest.java

@@ -290,8 +290,7 @@ public class SocketSslEchoTest extends AbstractSocketTest {
         }
 
         @Override
-        public void channelActive(ChannelHandlerContext ctx)
+        public void channelActive(ChannelHandlerContext ctx) throws Exception {
-                throws Exception {
             channel = ctx.channel();
         }
 
@@ -320,7 +319,10 @@ public class SocketSslEchoTest extends AbstractSocketTest {
                 counter > data.length / 2 && renegoFuture == null) {
 
                 SslHandler sslHandler = ctx.pipeline().get(SslHandler.class);
+
                 Future<Channel> hf = sslHandler.handshakeFuture();
+                assertThat(hf.isDone(), is(true));
+
                 renegoFuture = sslHandler.renegotiate();
                 assertThat(renegoFuture, is(not(sameInstance(hf))));
                 assertThat(renegoFuture, is(sameInstance(sslHandler.handshakeFuture())));
@@ -341,13 +343,13 @@ public class SocketSslEchoTest extends AbstractSocketTest {
         @Override
         public void userEventTriggered(ChannelHandlerContext ctx, Object evt) throws Exception {
             if (evt instanceof SslHandshakeCompletionEvent) {
+                assertSame(SslHandshakeCompletionEvent.SUCCESS, evt);
                 negoCounter ++;
             }
         }
 
         @Override
-        public void exceptionCaught(ChannelHandlerContext ctx,
+        public void exceptionCaught(ChannelHandlerContext ctx, Throwable cause) throws Exception {
-                Throwable cause) throws Exception {
             if (logger.isWarnEnabled()) {
                 logger.warn(
                         "Unexpected exception from the " +