From 4029366de98e5bc583ecd0f0b093c1c49a9dae9d Mon Sep 17 00:00:00 2001 From: Peter Levart Date: Wed, 21 Jan 2015 12:49:53 +0100 Subject: [PATCH] 8047769: SecureRandom should be more frugal with file descriptors Introduce FileInputStreamPool to cache open FileInputStreams Reviewed-by: wetmore, alanb, chegar --- .../provider/FileInputStreamPool.java | 159 +++++++++++++++++ .../sun/security/provider/SeedGenerator.java | 10 +- .../sun/security/provider/NativePRNG.java | 4 +- .../FileInputStreamPoolTest.java | 166 ++++++++++++++++++ 4 files changed, 333 insertions(+), 6 deletions(-) create mode 100644 jdk/src/java.base/share/classes/sun/security/provider/FileInputStreamPool.java create mode 100644 jdk/test/sun/security/provider/FileInputStreamPool/FileInputStreamPoolTest.java diff --git a/jdk/src/java.base/share/classes/sun/security/provider/FileInputStreamPool.java b/jdk/src/java.base/share/classes/sun/security/provider/FileInputStreamPool.java new file mode 100644 index 00000000000..67ad6946806 --- /dev/null +++ b/jdk/src/java.base/share/classes/sun/security/provider/FileInputStreamPool.java @@ -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 pool = + new ConcurrentHashMap<>(); + + /** + * a reference queue of cleared StreamRef(s) + */ + private static final ReferenceQueue 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 + * checkRead 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 { + final File file; + + StreamRef(File file, + UnclosableInputStream stream, + ReferenceQueue 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; + } + } +} diff --git a/jdk/src/java.base/share/classes/sun/security/provider/SeedGenerator.java b/jdk/src/java.base/share/classes/sun/security/provider/SeedGenerator.java index b4c2e77c2e0..474e20ed433 100644 --- a/jdk/src/java.base/share/classes/sun/security/provider/SeedGenerator.java +++ b/jdk/src/java.base/share/classes/sun/security/provider/SeedGenerator.java @@ -504,9 +504,10 @@ abstract class SeedGenerator { @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 @@ abstract class SeedGenerator { if (device.getProtocol().equalsIgnoreCase("file")) { File deviceFile = SunEntries.getDeviceFile(device); - return new FileInputStream(deviceFile); + return FileInputStreamPool + .getInputStream(deviceFile); } else { return device.openStream(); } diff --git a/jdk/src/java.base/unix/classes/sun/security/provider/NativePRNG.java b/jdk/src/java.base/unix/classes/sun/security/provider/NativePRNG.java index 1d275f086cb..ef728aa78eb 100644 --- a/jdk/src/java.base/unix/classes/sun/security/provider/NativePRNG.java +++ b/jdk/src/java.base/unix/classes/sun/security/provider/NativePRNG.java @@ -371,8 +371,8 @@ public final class NativePRNG extends SecureRandomSpi { // 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]; } diff --git a/jdk/test/sun/security/provider/FileInputStreamPool/FileInputStreamPoolTest.java b/jdk/test/sun/security/provider/FileInputStreamPool/FileInputStreamPoolTest.java new file mode 100644 index 00000000000..762515637f3 --- /dev/null +++ b/jdk/test/sun/security/provider/FileInputStreamPool/FileInputStreamPoolTest.java @@ -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); + } + } + } +}