RFR [8024521] (process) Async close issues with Process InputStream

Martin Buchholz martinrb at google.com
Fri Oct 25 15:59:25 UTC 2013


After rebasing, I propose this change which is shorter and is more
obviously correct.

diff --git a/src/solaris/classes/java/lang/UNIXProcess.java.linux
b/src/solaris/classes/java/lang/UNIXProcess.java.linux
--- a/src/solaris/classes/java/lang/UNIXProcess.java.linux
+++ b/src/solaris/classes/java/lang/UNIXProcess.java.linux
@@ -344,47 +344,39 @@
         ProcessPipeInputStream(int fd) {
             super(new FileInputStream(newFileDescriptor(fd)));
         }
-
-        private InputStream drainInputStream(InputStream in)
+        private static byte[] drainInputStream(InputStream in)
                 throws IOException {
             int n = 0;
             int j;
             byte[] a = null;
-            synchronized (closeLock) {
-                if (buf == null) // asynchronous close()?
-                    return null; // discard
-                j = in.available();
+            while ((j = in.available()) > 0) {
+                a = (a == null) ? new byte[j] : Arrays.copyOf(a, n + j);
+                n += in.read(a, n, j);
             }
-            while (j > 0) {
-                a = (a == null) ? new byte[j] : Arrays.copyOf(a, n + j);
-                synchronized (closeLock) {
-                    if (buf == null) // asynchronous close()?
-                        return null; // discard
-                    n += in.read(a, n, j);
-                    j = in.available();
-                }
-            }
-            return (a == null) ?
-                    ProcessBuilder.NullInputStream.INSTANCE :
-                    new ByteArrayInputStream(n == a.length ? a :
Arrays.copyOf(a, n));
+            return (a == null || n == a.length) ? a : Arrays.copyOf(a, n);
         }

         /** Called by the process reaper thread when the process exits. */
         synchronized void processExited() {
-            try {
-                InputStream in = this.in;
-                if (in != null) {
-                    InputStream stragglers = drainInputStream(in);
-                    in.close();
-                    this.in = stragglers;
-                }
-            } catch (IOException ignored) { }
+            synchronized (closeLock) {
+                try {
+                    InputStream in = this.in;
+                    // this stream is closed if and only if: in == null
+                    if (in != null) {
+                        byte[] stragglers = drainInputStream(in);
+                        in.close();
+                        this.in = (stragglers == null) ?
+                            ProcessBuilder.NullInputStream.INSTANCE :
+                            new ByteArrayInputStream(stragglers);
+                    }
+                } catch (IOException ignored) {}
+            }
         }

         @Override
         public void close() throws IOException {
             // BufferedInputStream#close() is not synchronized unlike most
other methods.
-            // Synchronizing helps avoid racing with drainInputStream().
+            // Synchronizing helps avoid race with processExited().
             synchronized (closeLock) {
                 super.close();
             }



More information about the core-libs-dev mailing list