8256818: SSLSocket that is never bound or connected leaks socket resources

Reviewed-by: xuelei

Author: RealCLanger

src/java.base/share/classes/sun/security/ssl/SSLSocketImpl.java

@@ -553,7 +553,7 @@ public boolean isClosed() {
     // locks may be deadlocked.
     @Override
     public void close() throws IOException {
-        if (tlsIsClosed) {
+        if (isClosed()) {
             return;
         }
 
@@ -562,27 +562,36 @@ public void close() throws IOException {
         }
 
         try {
-            // shutdown output bound, which may have been closed previously.
-            if (!isOutputShutdown()) {
-                duplexCloseOutput();
-            }
-
-            // shutdown input bound, which may have been closed previously.
-            if (!isInputShutdown()) {
-                duplexCloseInput();
-            }
+            if (isConnected()) {
+                // shutdown output bound, which may have been closed previously.
+                if (!isOutputShutdown()) {
+                    duplexCloseOutput();
+                }
 
-            if (!isClosed()) {
-                // close the connection directly
-                closeSocket(false);
+                // shutdown input bound, which may have been closed previously.
+                if (!isInputShutdown()) {
+                    duplexCloseInput();
+                }
             }
         } catch (IOException ioe) {
             // ignore the exception
             if (SSLLogger.isOn && SSLLogger.isOn("ssl")) {
                 SSLLogger.warning("SSLSocket duplex close failed", ioe);
             }
         } finally {
-            tlsIsClosed = true;
+            if (!isClosed()) {
+                // close the connection directly
+                try {
+                    closeSocket(false);
+                } catch (IOException ioe) {
+                    // ignore the exception
+                    if (SSLLogger.isOn && SSLLogger.isOn("ssl")) {
+                        SSLLogger.warning("SSLSocket close failed", ioe);
+                    }
+                } finally {
+                    tlsIsClosed = true;
+                }
+            }
         }
     }
 

test/jdk/java/lang/ProcessBuilder/checkHandles/CheckHandles.java

@@ -27,32 +27,30 @@
 import java.io.InputStreamReader;
 import java.lang.ProcessHandle;
 
+import jdk.test.lib.util.FileUtils;
+
 /*
  * @test
  * @bug 8239893
  * @summary Verify that handles for processes that terminate do not accumulate
  * @requires ((os.family == "windows") & (vm.compMode != "Xcomp"))
+ * @library /test/lib
  * @run main/othervm/native -Xint CheckHandles
  */
 public class CheckHandles {
 
-    // Return the current process handle count
-    private static native long getProcessHandleCount();
-
     public static void main(String[] args) throws Exception {
-        System.loadLibrary("CheckHandles");
-
         System.out.println("mypid: " + ProcessHandle.current().pid());
 
         // Warmup the process launch mechanism and vm to stabilize the number of handles in use
         int MAX_WARMUP = 20;
-        long prevCount = getProcessHandleCount();
+        long prevCount = FileUtils.getProcessHandleCount();
         for (int i = 0; i < MAX_WARMUP; i++) {
             oneProcess();
             System.gc();        // an opportunity to close unreferenced handles
             sleep(10);
 
-            long count = getProcessHandleCount();
+            long count = FileUtils.getProcessHandleCount();
             if (count < 0)
                 throw new AssertionError("getProcessHandleCount failed");
             System.out.println("warmup handle delta: " + (count - prevCount));
@@ -61,7 +59,7 @@ public static void main(String[] args) throws Exception {
         System.out.println("Warmup done");
         System.out.println();
 
-        prevCount = getProcessHandleCount();
+        prevCount = FileUtils.getProcessHandleCount();
         long startHandles = prevCount;
         long maxHandles = startHandles;
         int MAX_SPAWN = 50;
@@ -70,7 +68,7 @@ public static void main(String[] args) throws Exception {
             System.gc();        // an opportunity to close unreferenced handles
             sleep(10);
 
-            long count = getProcessHandleCount();
+            long count = FileUtils.getProcessHandleCount();
             if (count < 0)
                 throw new AssertionError("getProcessHandleCount failed");
             System.out.println("handle delta: " + (count - prevCount));

test/jdk/sun/security/ssl/SSLSocketImpl/SSLSocketLeak.java

@@ -0,0 +1,59 @@
+/*
+ * Copyright (c) 2020 SAP SE. All rights reserved.
+ * DO NOT ALTER OR REMOVE COPYRIGHT NOTICES OR THIS FILE HEADER.
+ *
+ * This code is free software; you can redistribute it and/or modify it
+ * under the terms of the GNU General Public License version 2 only, as
+ * published by the Free Software Foundation.
+ *
+ * This code is distributed in the hope that it will be useful, but WITHOUT
+ * ANY WARRANTY; without even the implied warranty of MERCHANTABILITY or
+ * FITNESS FOR A PARTICULAR PURPOSE.  See the GNU General Public License
+ * version 2 for more details (a copy is included in the LICENSE file that
+ * accompanied this code).
+ *
+ * You should have received a copy of the GNU General Public License version
+ * 2 along with this work; if not, write to the Free Software Foundation,
+ * Inc., 51 Franklin St, Fifth Floor, Boston, MA 02110-1301 USA.
+ *
+ * Please contact Oracle, 500 Oracle Parkway, Redwood Shores, CA 94065 USA
+ * or visit www.oracle.com if you need additional information or have any
+ * questions.
+ */
+
+import java.io.IOException;
+
+import javax.net.SocketFactory;
+import javax.net.ssl.SSLSocketFactory;
+
+import jdk.test.lib.util.FileUtils;
+
+/*
+ * @test
+ * @bug 8256818
+ * @summary Test that creating and closing SSL Sockets without bind/connect
+ *          will not leave leaking socket file descriptors
+ * @library /test/lib
+ * @run main/othervm SSLSocketLeak
+ */
+public class SSLSocketLeak {
+
+    private static final int NUM_TEST_SOCK = 500;
+
+    public static void main(String[] args) throws IOException {
+        long fds_start = FileUtils.getProcessHandleCount();
+        System.out.println("FDs at the beginning: " + fds_start);
+
+        SocketFactory f = SSLSocketFactory.getDefault();
+        for (int i = 0; i < NUM_TEST_SOCK; i++) {
+            f.createSocket().close();
+        }
+
+        long fds_end = FileUtils.getProcessHandleCount();
+        System.out.println("FDs in the end: " + fds_end);
+
+        if ((fds_end - fds_start) > (NUM_TEST_SOCK / 10)) {
+            throw new RuntimeException("Too many open file descriptors. Looks leaky.");
+        }
+    }
+}

test/lib/jdk/test/lib/util/FileUtils.java

@@ -23,34 +23,34 @@
 
 package jdk.test.lib.util;
 
-import jdk.test.lib.Platform;
-
 import java.io.BufferedReader;
-import java.io.InputStreamReader;
 import java.io.IOException;
+import java.io.InputStreamReader;
 import java.io.PrintStream;
 import java.io.UncheckedIOException;
 import java.lang.ProcessBuilder.Redirect;
+import java.lang.management.ManagementFactory;
 import java.nio.file.DirectoryNotEmptyException;
 import java.nio.file.FileVisitResult;
 import java.nio.file.Files;
 import java.nio.file.NoSuchFileException;
 import java.nio.file.Path;
-import java.nio.file.Paths;
 import java.nio.file.SimpleFileVisitor;
 import java.nio.file.attribute.BasicFileAttributes;
 import java.time.Instant;
-import java.time.Duration;
-import java.util.Arrays;
 import java.util.ArrayList;
-import java.util.ArrayDeque;
+import java.util.Arrays;
 import java.util.HashSet;
 import java.util.List;
 import java.util.Optional;
 import java.util.Set;
+import java.util.concurrent.TimeUnit;
 import java.util.concurrent.atomic.AtomicBoolean;
 import java.util.concurrent.atomic.AtomicReference;
-import java.util.concurrent.TimeUnit;
+
+import jdk.test.lib.Platform;
+
+import com.sun.management.UnixOperatingSystemMXBean;
 
 /**
  * Common library for various test file utility functions.
@@ -59,6 +59,7 @@ public final class FileUtils {
     private static final boolean IS_WINDOWS = Platform.isWindows();
     private static final int RETRY_DELETE_MILLIS = IS_WINDOWS ? 500 : 0;
     private static final int MAX_RETRY_DELETE_TIMES = IS_WINDOWS ? 15 : 0;
+    private static volatile boolean nativeLibLoaded;
 
     /**
      * Deletes a file, retrying if necessary.
@@ -363,6 +364,21 @@ public static void listFileDescriptors(PrintStream ps) {
         });
     }
 
+    // Return the current process handle count
+    public static long getProcessHandleCount() {
+        if (IS_WINDOWS) {
+            if (!nativeLibLoaded) {
+                System.loadLibrary("FileUtils");
+                nativeLibLoaded = true;
+            }
+            return getWinProcessHandleCount();
+        } else {
+            return ((UnixOperatingSystemMXBean)ManagementFactory.getOperatingSystemMXBean()).getOpenFileDescriptorCount();
+        }
+    }
+
+    private static native long getWinProcessHandleCount();
+
     // Possible command locations and arguments
     static String[][] lsCommands = new String[][] {
             {"/usr/bin/lsof", "-p"},

test/jdk/java/lang/ProcessBuilder/checkHandles/libCheckHandles.c test/lib/jdk/test/lib/util/libFileUtils.c

@@ -29,7 +29,7 @@
 #include "jni.h"
 #include <windows.h>
 
-JNIEXPORT jlong JNICALL Java_CheckHandles_getProcessHandleCount(JNIEnv *env)
+JNIEXPORT jlong JNICALL Java_jdk_test_lib_util_FileUtils_getWinProcessHandleCount(JNIEnv *env)
 {
     DWORD handleCount;
     HANDLE handle = GetCurrentProcess();