8047769: SecureRandom should be more frugal with file descriptors
Summary: Introduce FileInputStreamPool to cache open FileInputStreams
Reviewed-by: wetmore, alanb, chegar
--- /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);
+ }
+ }
+ }
+}