8239893: Windows handle Leak when starting processes using ProcessBui…

…lder Reviewed-by: bpb, naoto
openjdk · Mar 6, 2020 · 375d0c1 · 375d0c1
1 parent e2f4319
commit 375d0c1
Show file tree

Hide file tree

Showing 3 changed files with 153 additions and 1 deletion.
diff --git a/src/java.base/windows/classes/java/lang/ProcessImpl.java b/src/java.base/windows/classes/java/lang/ProcessImpl.java
@@ -1,5 +1,5 @@
 /*
- * Copyright (c) 1995, 2019, Oracle and/or its affiliates. All rights reserved.
+ * Copyright (c) 1995, 2020, Oracle and/or its affiliates. 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
@@ -496,6 +496,7 @@ public Void run() {
             else {
                 FileDescriptor stdin_fd = new FileDescriptor();
                 fdAccess.setHandle(stdin_fd, stdHandles[0]);
+                fdAccess.registerCleanup(stdin_fd);
                 stdin_stream = new BufferedOutputStream(
                     new FileOutputStream(stdin_fd));
             }
@@ -505,6 +506,7 @@ public Void run() {
             else {
                 FileDescriptor stdout_fd = new FileDescriptor();
                 fdAccess.setHandle(stdout_fd, stdHandles[1]);
+                fdAccess.registerCleanup(stdout_fd);
                 stdout_stream = new BufferedInputStream(
                     new PipeInputStream(stdout_fd));
             }
@@ -514,6 +516,7 @@ public Void run() {
             else {
                 FileDescriptor stderr_fd = new FileDescriptor();
                 fdAccess.setHandle(stderr_fd, stdHandles[2]);
+                fdAccess.registerCleanup(stderr_fd);
                 stderr_stream = new PipeInputStream(stderr_fd);
             }
 

diff --git a/test/jdk/java/lang/ProcessBuilder/checkHandles/CheckHandles.java b/test/jdk/java/lang/ProcessBuilder/checkHandles/CheckHandles.java
@@ -0,0 +1,105 @@
+/*
+ * Copyright (c) 2020, Oracle and/or its affiliates. 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.BufferedReader;
+import java.io.IOException;
+import java.io.InputStream;
+import java.io.InputStreamReader;
+import java.lang.ProcessHandle;
+
+/*
+ * @test
+ * @bug 8239893
+ * @summary Verify that handles for processes that terminate do not accumulate
+ * @requires (os.family == "windows")
+ * @run main/native 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());
+        long minHandles = Long.MAX_VALUE;
+        long maxHandles = 0L;
+        int MAX_SPAWN = 50;
+        for (int i = 0; i < MAX_SPAWN; i++) {
+            try {
+                Process testProcess = new ProcessBuilder("cmd", "/c", "dir").start();
+
+                Thread outputConsumer = new Thread(() -> consumeStream(testProcess.pid(), testProcess.getInputStream()));
+                outputConsumer.setDaemon(true);
+                outputConsumer.start();
+                Thread errorConsumer = new Thread(() -> consumeStream(testProcess.pid(), testProcess.getErrorStream()));
+                errorConsumer.setDaemon(true);
+                errorConsumer.start();
+
+                testProcess.waitFor();
+                System.gc();
+                outputConsumer.join();
+                errorConsumer.join();
+                long count = getProcessHandleCount();
+                if (count < 0)
+                    throw new AssertionError("getProcessHandleCount failed");
+                minHandles =  Math.min(minHandles, count);
+                maxHandles =  Math.max(maxHandles, count);
+            } catch (IOException | InterruptedException e) {
+                e.printStackTrace();
+                throw e;
+            }
+        }
+        final long ERROR_PERCENT = 10;
+        final long ERROR_THRESHOLD = // 10% increase over min to passing max
+                minHandles + ((minHandles + ERROR_PERCENT - 1) / ERROR_PERCENT);
+        if (maxHandles >= ERROR_THRESHOLD) {
+            System.out.println("Processes started: " + MAX_SPAWN);
+            System.out.println("minhandles: " + minHandles);
+            System.out.println("maxhandles: " + maxHandles);
+            throw new AssertionError("Handle use increased by more than " + ERROR_PERCENT + " percent.");
+        }
+    }
+
+    private static void consumeStream(long pid, InputStream inputStream) {
+        BufferedReader reader = null;
+        try {
+            int lines = 0;
+            reader = new BufferedReader(new InputStreamReader(inputStream));
+            while (reader.readLine() != null) {
+                lines++;
+            }
+        } catch (IOException e) {
+            e.printStackTrace();
+        } finally {
+            if (reader != null) {
+                try {
+                    reader.close();
+                } catch (IOException e) {
+                    e.printStackTrace();
+                }
+            }
+        }
+    }
+}
diff --git a/test/jdk/java/lang/ProcessBuilder/checkHandles/libCheckHandles.c b/test/jdk/java/lang/ProcessBuilder/checkHandles/libCheckHandles.c
@@ -0,0 +1,44 @@
+/*
+ * Copyright (c) 2020, Oracle and/or its affiliates. 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.
+ */
+
+#include <stdlib.h>
+#include <string.h>
+
+#ifdef _WIN32
+
+#include "jni.h"
+#include "jni_util.h"
+#include <windows.h>
+
+JNIEXPORT jlong JNICALL Java_CheckHandles_getProcessHandleCount(JNIEnv *env)
+{
+    DWORD handleCount;
+    HANDLE handle = GetCurrentProcess();
+    if (GetProcessHandleCount(handle, &handleCount)) {
+        return (jlong)handleCount;
+    } else {
+        return -1L;
+    }
+}
+
+#endif  /*  _WIN32 */