Fixed deadlock in AsyncSshHandlerWriter

[netconf.git] / netconf / netconf-netty-util / src / main / java / org / opendaylight / netconf / nettyutil / handler / ssh / client / AsyncSshHandler.java

diff --git a/netconf/netconf-netty-util/src/main/java/org/opendaylight/netconf/nettyutil/handler/ssh/client/AsyncSshHandler.java b/netconf/netconf-netty-util/src/main/java/org/opendaylight/netconf/nettyutil/handler/ssh/client/AsyncSshHandler.java
index c3ff3496625e0f84f154692f49d71b924d67ca15..149ccf16f9910dfbf38a866f5c0d89fbda50c7b6 100644 (file)
--- a/netconf/netconf-netty-util/src/main/java/org/opendaylight/netconf/nettyutil/handler/ssh/client/AsyncSshHandler.java
+++ b/netconf/netconf-netty-util/src/main/java/org/opendaylight/netconf/nettyutil/handler/ssh/client/AsyncSshHandler.java
@@ -63,7 +63,7 @@ public class AsyncSshHandler extends ChannelOutboundHandlerAdapter {
     private GenericFutureListener negotiationFutureListener;
 
     public AsyncSshHandler(final AuthenticationHandler authenticationHandler, final SshClient sshClient,
-            final Future<?> negotiationFuture) throws IOException {
+            final Future<?> negotiationFuture) {
         this(authenticationHandler, sshClient);
         this.negotiationFuture = negotiationFuture;
     }
@@ -73,16 +73,14 @@ public class AsyncSshHandler extends ChannelOutboundHandlerAdapter {
      *
      * @param authenticationHandler authentication handler
      * @param sshClient             started SshClient
-     * @throws IOException          if the I/O operation fails
      */
     public AsyncSshHandler(final AuthenticationHandler authenticationHandler,
-                           final SshClient sshClient) throws IOException {
+                           final SshClient sshClient) {
         this.authenticationHandler = Preconditions.checkNotNull(authenticationHandler);
         this.sshClient = Preconditions.checkNotNull(sshClient);
     }
 
-    public static AsyncSshHandler createForNetconfSubsystem(final AuthenticationHandler authenticationHandler)
-            throws IOException {
+    public static AsyncSshHandler createForNetconfSubsystem(final AuthenticationHandler authenticationHandler) {
         return new AsyncSshHandler(authenticationHandler, DEFAULT_CLIENT);
     }
 
@@ -93,10 +91,9 @@ public class AsyncSshHandler extends ChannelOutboundHandlerAdapter {
      * @param authenticationHandler authentication handler
      * @param negotiationFuture     negotiation future
      * @return                      {@code AsyncSshHandler}
-     * @throws IOException          if the I/O operation fails
      */
     public static AsyncSshHandler createForNetconfSubsystem(final AuthenticationHandler authenticationHandler,
-            final Future<?> negotiationFuture) throws IOException {
+            final Future<?> negotiationFuture) {
         return new AsyncSshHandler(authenticationHandler, DEFAULT_CLIENT, negotiationFuture);
     }
 
@@ -119,9 +116,10 @@ public class AsyncSshHandler extends ChannelOutboundHandlerAdapter {
 
             session = future.getSession();
             final AuthFuture authenticateFuture = authenticationHandler.authenticate(session);
+            final ClientSession localSession = session;
             authenticateFuture.addListener(future1 -> {
                 if (future1.isSuccess()) {
-                    handleSshAuthenticated(session, ctx);
+                    handleSshAuthenticated(localSession, ctx);
                 } else {
                     // Exception does not have to be set in the future, add simple exception in such case
                     final Throwable exception = future1.getException() == null
@@ -164,8 +162,9 @@ public class AsyncSshHandler extends ChannelOutboundHandlerAdapter {
 
         // TODO we should also read from error stream and at least log from that
 
+        ClientChannel localChannel = channel;
         sshReadAsyncListener = new AsyncSshHandlerReader(() -> AsyncSshHandler.this.disconnect(ctx, ctx.newPromise()),
-            msg -> ctx.fireChannelRead(msg), channel.toString(), channel.getAsyncOut());
+            ctx::fireChannelRead, localChannel.toString(), localChannel.getAsyncOut());
 
         // if readAsyncListener receives immediate close,
         // it will close this handler and closing this handler sets channel variable to null
@@ -198,10 +197,9 @@ public class AsyncSshHandler extends ChannelOutboundHandlerAdapter {
         this.connectPromise = promise;
 
         if (negotiationFuture != null) {
-
             negotiationFutureListener = future -> {
                 if (future.isSuccess()) {
-                    connectPromise.setSuccess();
+                    promise.setSuccess();
                 }
             };
             //complete connection promise with netconf negotiation future
@@ -211,7 +209,7 @@ public class AsyncSshHandler extends ChannelOutboundHandlerAdapter {
     }
 
     @Override
-    public void close(final ChannelHandlerContext ctx, final ChannelPromise promise) throws Exception {
+    public void close(final ChannelHandlerContext ctx, final ChannelPromise promise) {
         disconnect(ctx, promise);
     }
 
@@ -249,10 +247,12 @@ public class AsyncSshHandler extends ChannelOutboundHandlerAdapter {
 
         if (session != null && !session.isClosed() && !session.isClosing()) {
             session.close(false).addListener(future -> {
-                if (!future.isClosed()) {
-                    session.close(true);
+                synchronized (this) {
+                    if (!future.isClosed()) {
+                        session.close(true);
+                    }
+                    session = null;
                 }
-                session = null;
             });
         }