JSSE: avoid potential deadlock between SSLSocket.close() and Input/OutputStream.close()

src/java/com/wolfssl/provider/jsse/WolfSSLSocket.java

@@ -35,6 +35,7 @@ import java.util.ArrayList;
 import java.util.function.BiFunction;
 import java.util.List;
 import java.util.Arrays;
+import java.util.concurrent.atomic.AtomicBoolean;
 import java.nio.channels.SocketChannel;
 import java.security.cert.CertificateEncodingException;
 
@@ -96,6 +97,9 @@ public class WolfSSLSocket extends SSLSocket {
      * accessing WolfSSLSession object / WOLFSSL struct */
     private final Object ioLock = new Object();
 
+    /* lock for get/set of SO timeout */
+    private final Object timeoutLock = new Object();
+
     /** ALPN selector callback, if set */
     protected BiFunction<SSLSocket, List<String>, String> alpnSelector = null;
 
@@ -1733,13 +1737,16 @@ public class WolfSSLSocket extends SSLSocket {
      * @throws SocketException if there is an error setting the timeout value
      */
     @Override
-    public synchronized void setSoTimeout(int timeout) throws SocketException {
+    public void setSoTimeout(int timeout) throws SocketException {
+        /* timeoutLock synchronizes get/set of timeout */
+        synchronized (timeoutLock) {
             if (this.socket != null) {
                 this.socket.setSoTimeout(timeout);
             } else {
                 super.setSoTimeout(timeout);
             }
         }
+    }
 
     /**
      * Get the SO_TIMEOUT value, in milliseconds.
@@ -1749,13 +1756,16 @@ public class WolfSSLSocket extends SSLSocket {
      * @throws SocketException if there is an error getting timeout value
      */
     @Override
-    public synchronized int getSoTimeout() throws SocketException {
+    public int getSoTimeout() throws SocketException {
+        /* timeoutLock synchronizes get/set of timeout */
+        synchronized (timeoutLock) {
             if (this.socket != null) {
                 return this.socket.getSoTimeout();
             } else {
                 return super.getSoTimeout();
             }
         }
+    }
 
     /**
      * Set the SSLParameters for this SSLSocket.
@@ -1938,13 +1948,17 @@ public class WolfSSLSocket extends SSLSocket {
                             this.EngineHelper.clearObjectState();
                             this.EngineHelper = null;
 
-                            /* Release Input/OutputStream objects */
+                            /* Release Input/OutputStream objects. Do not
+                             * close WolfSSLSocket inside stream close,
+                             * since we handle that next below and do
+                             * differently depending on if autoClose has been
+                             * set or not. */
                             if (this.inStream != null) {
-                                this.inStream.close();
+                                this.inStream.close(false);
                                 this.inStream = null;
                             }
                             if (this.outStream != null) {
-                                this.outStream.close();
+                                this.outStream.close(false);
                                 this.outStream = null;
                             }
 
@@ -2376,14 +2390,30 @@ public class WolfSSLSocket extends SSLSocket {
         private WolfSSLSocket  socket;
         private boolean isClosed = true;
 
+        /* Atomic boolean to indicate if this InputStream has started to
+         * close. Protects against deadlock between two threads calling
+         * SSLSocket.close() and InputStream.close() simulatenously. */
+        private AtomicBoolean isClosing = new AtomicBoolean(false);
+
         public WolfSSLInputStream(WolfSSLSession ssl, WolfSSLSocket socket) {
             this.ssl = ssl;
             this.socket = socket; /* parent socket */
             this.isClosed = false;
         }
 
-        public synchronized void close() throws IOException {
+        /**
+         * Close InputStream, but gives caller option to close underlying
+         * Socket or not.
+         *
+         * @param closeSocket close underlying WolfSSLSocket if set to true,
+         *        otherwise if false leave WolfSSLSocket open.
+         */
+        protected void close(boolean closeSocket) throws IOException {
 
+            if (isClosing.compareAndSet(false, true)) {
+
+                synchronized (this) {
+                    if (closeSocket) {
                         if (this.socket == null || this.isClosed) {
                             return;
                         }
@@ -2397,14 +2427,28 @@ public class WolfSSLSocket extends SSLSocket {
                             WolfSSLDebug.log(getClass(), WolfSSLDebug.INFO,
                                 "socket (input) closed: " + this.socket);
                         }
+                    }
 
                     this.socket = null;
                     this.ssl = null;
                     this.isClosed = true;
+                }
+            }
+            else {
+                WolfSSLDebug.log(getClass(), WolfSSLDebug.INFO,
+                    "InputStream already in process of being closed");
+            }
 
             return;
         }
 
+        /**
+         * Close InputStream, also closes internal WolfSSLSocket.
+         */
+        public void close() throws IOException {
+            close(true);
+        }
+
         @Override
         public synchronized int read() throws IOException {
 
@@ -2487,11 +2531,12 @@ public class WolfSSLSocket extends SSLSocket {
 
             try {
                 int err;
+                int timeout = socket.getSoTimeout();
 
                 WolfSSLDebug.log(getClass(), WolfSSLDebug.INFO,
-                    "ssl.read() socket timeout = " + socket.getSoTimeout());
+                    "ssl.read() socket timeout = " + timeout);
 
-                ret = ssl.read(b, off, len, socket.getSoTimeout());
+                ret = ssl.read(b, off, len, timeout);
                 err = ssl.getError(ret);
 
                 WolfSSLDebug.log(getClass(), WolfSSLDebug.INFO,
@@ -2519,16 +2564,17 @@ public class WolfSSLSocket extends SSLSocket {
                          * end of stream */
                         WolfSSLDebug.log(getClass(), WolfSSLDebug.INFO,
                             "Native wolfSSL_read() error: " + errStr +
-                            " (error code: " + err + "), end of stream");
+                            " (error code: " + err + "ret: " + ret +
+                            "), end of stream");
                         return -1;
 
                     } else {
                         WolfSSLDebug.log(getClass(), WolfSSLDebug.INFO,
                             "Native wolfSSL_read() error: " + errStr +
-                            " (error code: " + err + ")");
+                            " (error code: " + err + ", ret: " + ret + ")");
                         throw new IOException("Native wolfSSL_read() " +
                             "error: " + errStr +
-                            " (error code: " + err + ")");
+                            " (error code: " + err + ", ret: " + ret + ")");
                     }
                 }
 
@@ -2557,14 +2603,30 @@ public class WolfSSLSocket extends SSLSocket {
         private WolfSSLSocket  socket;
         private boolean isClosed = true;
 
+        /* Atomic boolean to indicate if this InputStream has started to
+         * close. Protects against deadlock between two threads calling
+         * SSLSocket.close() and InputStream.close() simulatenously. */
+        private AtomicBoolean isClosing = new AtomicBoolean(false);
+
         public WolfSSLOutputStream(WolfSSLSession ssl, WolfSSLSocket socket) {
             this.ssl = ssl;
             this.socket = socket; /* parent socket */
             this.isClosed = false;
         }
 
-        public synchronized void close() throws IOException {
+        /**
+         * Close OutputStream, but gives caller option to close underlying
+         * Socket or not.
+         *
+         * @param closeSocket close underlying WolfSSLSocket if set to true,
+         *        otherwise if false leave WolfSSLSocket open.
+         */
+        protected void close(boolean closeSocket) throws IOException {
 
+            if (isClosing.compareAndSet(false, true)) {
+
+                synchronized (this) {
+                    if (closeSocket) {
                         if (this.socket == null || this.isClosed) {
                             return;
                         }
@@ -2578,14 +2640,28 @@ public class WolfSSLSocket extends SSLSocket {
                             WolfSSLDebug.log(getClass(), WolfSSLDebug.INFO,
                                 "socket (output) closed: " + this.socket);
                         }
+                    }
 
                     this.socket = null;
                     this.ssl = null;
                     this.isClosed = true;
+                }
+            }
+            else {
+                WolfSSLDebug.log(getClass(), WolfSSLDebug.INFO,
+                    "OutputStream already in process of being closed");
+            }
 
             return;
         }
 
+        /**
+         * Close OutputStream, also closes internal WolfSSLSocket.
+         */
+        public void close() throws IOException {
+            this.close(true);
+        }
+
         public synchronized void write(int b) throws IOException {
             byte[] data = new byte[1];
             data[0] = (byte)(b & 0xFF);
@@ -2606,7 +2682,8 @@ public class WolfSSLSocket extends SSLSocket {
                 throw new NullPointerException("Input array is null");
             }
 
-            if (this.socket == null || this.isClosed) {
+            /* check if socket is closed */
+            if (this.isClosed || socket == null || socket.isClosed()) {
                 throw new SocketException("Socket is closed");
             }
 
@@ -2642,12 +2719,12 @@ public class WolfSSLSocket extends SSLSocket {
 
             try {
                 int err;
+                int timeout = socket.getSoTimeout();
 
                 WolfSSLDebug.log(getClass(), WolfSSLDebug.INFO,
-                    "ssl.write() socket timeout = " +
+                    "ssl.write() socket timeout = " + timeout);
-                    socket.getSoTimeout());
 
-                ret = ssl.write(b, off, len, socket.getSoTimeout());
+                ret = ssl.write(b, off, len, timeout);
                 err = ssl.getError(ret);
 
                 WolfSSLDebug.log(getClass(), WolfSSLDebug.INFO,