8047769: SecureRandom should be more frugal with file descriptors
authorplevart
Wed, 21 Jan 2015 12:49:53 +0100
changeset 28542 d50a7783fe02
parent 28541 f6b5d78556d6
child 28543 31afdc0e77af
8047769: SecureRandom should be more frugal with file descriptors Summary: Introduce FileInputStreamPool to cache open FileInputStreams Reviewed-by: wetmore, alanb, chegar
jdk/src/java.base/share/classes/sun/security/provider/FileInputStreamPool.java
jdk/src/java.base/share/classes/sun/security/provider/SeedGenerator.java
jdk/src/java.base/unix/classes/sun/security/provider/NativePRNG.java
jdk/test/sun/security/provider/FileInputStreamPool/FileInputStreamPoolTest.java
--- /dev/null	Thu Jan 01 00:00:00 1970 +0000
+++ b/jdk/src/java.base/share/classes/sun/security/provider/FileInputStreamPool.java	Wed Jan 21 12:49:53 2015 +0100
@@ -0,0 +1,159 @@
+/*
+ * Copyright (c) 2014, 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.  Oracle designates this
+ * particular file as subject to the "Classpath" exception as provided
+ * by Oracle in the LICENSE file that accompanied this code.
+ *
+ * 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.
+ */
+
+package sun.security.provider;
+
+import java.io.*;
+import java.lang.ref.ReferenceQueue;
+import java.lang.ref.WeakReference;
+import java.util.concurrent.ConcurrentHashMap;
+import java.util.concurrent.ConcurrentMap;
+
+/**
+ * A pool of {@code InputStream}s opened from distinct files. Only a single
+ * instance is ever opened from the same file. This is used to read special
+ * infinite files like {@code /dev/random} where the current file pointer is not
+ * relevant, so multiple readers can share the same file descriptor and
+ * consequently the same {@code InputStream}.
+ */
+class FileInputStreamPool {
+
+    /**
+     * a pool of: StreamRef -> UnclosableInputStream -> FileInputStream(s)
+     */
+    private static final ConcurrentMap<File, StreamRef> pool =
+        new ConcurrentHashMap<>();
+
+    /**
+     * a reference queue of cleared StreamRef(s)
+     */
+    private static final ReferenceQueue<UnclosableInputStream> refQueue =
+        new ReferenceQueue<>();
+
+    /**
+     * This method opens an underlying {@link java.io.FileInputStream} for a
+     * given {@code file} and returns a wrapper over it. The wrapper is shared
+     * among multiple readers of the same {@code file} and ignores
+     * {@link java.io.InputStream#close()} requests. The underlying stream is
+     * closed when all references to the wrapper are relinquished.
+     *
+     * @param file the file to be opened for reading.
+     * @return a shared {@link java.io.InputStream} instance opened from given
+     * file.
+     * @throws FileNotFoundException if the file does not exist, is a directory
+     *                               rather than a regular file, or for some
+     *                               other reason cannot be opened for  reading.
+     * @throws SecurityException     if a security manager exists and its
+     *                               <code>checkRead</code> method denies read
+     *                               access to the file.
+     */
+    static InputStream getInputStream(File file) throws IOException {
+
+        // expunge any cleared references
+        StreamRef oldRref;
+        while ((oldRref = (StreamRef) refQueue.poll()) != null) {
+            pool.remove(oldRref.file, oldRref);
+        }
+
+        // canonicalize the path
+        // (this also checks the read permission on the file if SecurityManager
+        // is present, so no checking is needed later when we just return the
+        // already opened stream)
+        File cfile = file.getCanonicalFile();
+
+        // check if it exists in pool
+        oldRref = pool.get(cfile);
+        UnclosableInputStream oldStream = (oldRref == null)
+            ? null
+            : oldRref.get();
+        StreamRef newRef = null;
+        UnclosableInputStream newStream = null;
+
+        // retry loop
+        while (true) {
+            if (oldStream != null) {
+                // close our optimistically opened stream 1st (if we opened it)
+                if (newStream != null) {
+                    try {
+                        newStream.getWrappedStream().close();
+                    } catch (IOException ignore) {
+                        // can't do anything here
+                    }
+                }
+                // return it
+                return oldStream;
+            } else {
+                // we need to open new stream optimistically (if not already)
+                if (newStream == null) {
+                    newStream = new UnclosableInputStream(
+                        new FileInputStream(cfile));
+                    newRef = new StreamRef(cfile, newStream, refQueue);
+                }
+                // either try to install newRef or replace oldRef with newRef
+                if (oldRref == null) {
+                    oldRref = pool.putIfAbsent(cfile, newRef);
+                } else {
+                    oldRref = pool.replace(cfile, oldRref, newRef)
+                        ? null
+                        : pool.get(cfile);
+                }
+                if (oldRref == null) {
+                    // success
+                    return newStream;
+                } else {
+                    // lost race
+                    oldStream = oldRref.get();
+                    // another loop
+                }
+            }
+        }
+    }
+
+    private static class StreamRef extends WeakReference<UnclosableInputStream> {
+        final File file;
+
+        StreamRef(File file,
+                  UnclosableInputStream stream,
+                  ReferenceQueue<UnclosableInputStream> refQueue) {
+            super(stream, refQueue);
+            this.file = file;
+        }
+    }
+
+    private static final class UnclosableInputStream extends FilterInputStream {
+        UnclosableInputStream(InputStream in) {
+            super(in);
+        }
+
+        @Override
+        public void close() throws IOException {
+            // Ignore close attempts since underlying InputStream is shared.
+        }
+
+        InputStream getWrappedStream() {
+            return in;
+        }
+    }
+}
--- a/jdk/src/java.base/share/classes/sun/security/provider/SeedGenerator.java	Wed Jan 21 09:46:24 2015 +0000
+++ b/jdk/src/java.base/share/classes/sun/security/provider/SeedGenerator.java	Wed Jan 21 12:49:53 2015 +0100
@@ -504,9 +504,10 @@
                         @Override
                         public InputStream run() throws IOException {
                             /*
-                             * return a FileInputStream for file URLs and
-                             * avoid buffering. The openStream() call wraps
-                             * InputStream in a BufferedInputStream which
+                             * return a shared InputStream for file URLs and
+                             * avoid buffering.
+                             * The URL.openStream() call wraps InputStream in a
+                             * BufferedInputStream which
                              * can buffer up to 8K bytes. This read is a
                              * performance issue for entropy sources which
                              * can be slow to replenish.
@@ -514,7 +515,8 @@
                             if (device.getProtocol().equalsIgnoreCase("file")) {
                                 File deviceFile =
                                     SunEntries.getDeviceFile(device);
-                                return new FileInputStream(deviceFile);
+                                return FileInputStreamPool
+                                    .getInputStream(deviceFile);
                             } else {
                                 return device.openStream();
                             }
--- a/jdk/src/java.base/unix/classes/sun/security/provider/NativePRNG.java	Wed Jan 21 09:46:24 2015 +0000
+++ b/jdk/src/java.base/unix/classes/sun/security/provider/NativePRNG.java	Wed Jan 21 12:49:53 2015 +0100
@@ -371,8 +371,8 @@
         // constructor, called only once from initIO()
         private RandomIO(File seedFile, File nextFile) throws IOException {
             this.seedFile = seedFile;
-            seedIn = new FileInputStream(seedFile);
-            nextIn = new FileInputStream(nextFile);
+            seedIn = FileInputStreamPool.getInputStream(seedFile);
+            nextIn = FileInputStreamPool.getInputStream(nextFile);
             nextBuffer = new byte[BUFFER_SIZE];
         }
 
--- /dev/null	Thu Jan 01 00:00:00 1970 +0000
+++ b/jdk/test/sun/security/provider/FileInputStreamPool/FileInputStreamPoolTest.java	Wed Jan 21 12:49:53 2015 +0100
@@ -0,0 +1,166 @@
+/*
+ * Copyright (c) 2014, 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.
+ */
+
+/**
+ * @test
+ * @bug 8047769
+ * @summary SecureRandom should be more frugal with file descriptors
+ */
+
+import java.io.File;
+import java.io.FileOutputStream;
+import java.io.IOException;
+import java.io.InputStream;
+import java.lang.ref.Reference;
+import java.lang.reflect.InvocationTargetException;
+import java.lang.reflect.Method;
+import java.lang.reflect.UndeclaredThrowableException;
+import java.util.Arrays;
+
+public class FileInputStreamPoolTest {
+
+    static final byte[] bytes = new byte[]{1, 2, 3, 4, 5, 6, 7, 8};
+
+    static void testCaching(File file) throws IOException {
+        InputStream in1 = TestProxy.FileInputStreamPool_getInputStream(file);
+        InputStream in2 = TestProxy.FileInputStreamPool_getInputStream(file);
+        assertTrue(in1 == in2,
+            "1st InputStream: " + in1 +
+                " is not same as 2nd: " + in2);
+
+        byte[] readBytes = new byte[bytes.length];
+        int nread = in1.read(readBytes);
+        assertTrue(bytes.length == nread,
+            "short read: " + nread +
+                " bytes of expected: " + bytes.length);
+        assertTrue(Arrays.equals(readBytes, bytes),
+            "readBytes: " + Arrays.toString(readBytes) +
+                " not equal to expected: " + Arrays.toString(bytes));
+    }
+
+    static void assertTrue(boolean test, String message) {
+        if (!test) {
+            throw new AssertionError(message);
+        }
+    }
+
+    static void processReferences() {
+        // make JVM process References
+        System.gc();
+        // help ReferenceHandler thread enqueue References
+        while (TestProxy.Reference_tryHandlePending(false)) {}
+        // help run Finalizers
+        System.runFinalization();
+    }
+
+    public static void main(String[] args) throws Exception {
+        // 1st create temporary file
+        File file = File.createTempFile("test", ".dat");
+        try (AutoCloseable acf = () -> {
+            // On Windows, failure to delete file is probably a consequence
+            // of the file still being opened - so the test should fail.
+            assertTrue(file.delete(),
+                "Can't delete: " + file + " (is it still open?)");
+        }) {
+            try (FileOutputStream out = new FileOutputStream(file)) {
+                out.write(bytes);
+            }
+
+            // test caching 1t time
+            testCaching(file);
+
+            processReferences();
+
+            // test caching 2nd time - this should only succeed if the stream
+            // is re-opened as a consequence of cleared WeakReference
+            testCaching(file);
+
+            processReferences();
+        }
+    }
+
+    /**
+     * A proxy for (package)private static methods:
+     *   sun.security.provider.FileInputStreamPool.getInputStream
+     *   java.lang.ref.Reference.tryHandlePending
+     */
+    static class TestProxy {
+        private static final Method getInputStreamMethod;
+        private static final Method tryHandlePendingMethod;
+
+        static {
+            try {
+                Class<?> fileInputStreamPoolClass =
+                    Class.forName("sun.security.provider.FileInputStreamPool");
+                getInputStreamMethod =
+                    fileInputStreamPoolClass.getDeclaredMethod(
+                        "getInputStream", File.class);
+                getInputStreamMethod.setAccessible(true);
+
+                tryHandlePendingMethod = Reference.class.getDeclaredMethod(
+                    "tryHandlePending", boolean.class);
+                tryHandlePendingMethod.setAccessible(true);
+            } catch (Exception e) {
+                throw new Error(e);
+            }
+        }
+
+        static InputStream FileInputStreamPool_getInputStream(File file)
+            throws IOException {
+            try {
+                return (InputStream) getInputStreamMethod.invoke(null, file);
+            } catch (InvocationTargetException e) {
+                Throwable te = e.getTargetException();
+                if (te instanceof IOException) {
+                    throw (IOException) te;
+                } else if (te instanceof RuntimeException) {
+                    throw (RuntimeException) te;
+                } else if (te instanceof Error) {
+                    throw (Error) te;
+                } else {
+                    throw new UndeclaredThrowableException(te);
+                }
+            } catch (IllegalAccessException e) {
+                throw new RuntimeException(e);
+            }
+        }
+
+        static boolean Reference_tryHandlePending(boolean waitForNotify) {
+            try {
+                return (boolean) tryHandlePendingMethod
+                    .invoke(null, waitForNotify);
+            } catch (InvocationTargetException e) {
+                Throwable te = e.getTargetException();
+                if (te instanceof RuntimeException) {
+                    throw (RuntimeException) te;
+                } else if (te instanceof Error) {
+                    throw (Error) te;
+                } else {
+                    throw new UndeclaredThrowableException(te);
+                }
+            } catch (IllegalAccessException e) {
+                throw new RuntimeException(e);
+            }
+        }
+    }
+}