8175010: ImageReader is not thread-safe
authorredestad
Wed, 15 Feb 2017 15:57:18 +0100
changeset 43804 96fd6bf9c69b
parent 43803 e1881d258206
child 43805 9051877afb06
8175010: ImageReader is not thread-safe Reviewed-by: alanb, jlaskey, chegar
jdk/src/java.base/share/classes/jdk/internal/jimage/ImageReader.java
jdk/src/java.base/share/classes/jdk/internal/jimage/ImageReaderFactory.java
--- a/jdk/src/java.base/share/classes/jdk/internal/jimage/ImageReader.java	Wed Feb 15 21:46:50 2017 +0800
+++ b/jdk/src/java.base/share/classes/jdk/internal/jimage/ImageReader.java	Wed Feb 15 15:57:18 2017 +0100
@@ -52,7 +52,9 @@
  * to the jimage file provided by the shipped JDK by tools running on JDK 8.
  */
 public final class ImageReader implements AutoCloseable {
-    private SharedImageReader reader;
+    private final SharedImageReader reader;
+
+    private volatile boolean closed;
 
     private ImageReader(SharedImageReader reader) {
         this.reader = reader;
@@ -71,45 +73,49 @@
 
     @Override
     public void close() throws IOException {
-        if (reader == null) {
+        if (closed) {
             throw new IOException("image file already closed");
         }
+        reader.close(this);
+        closed = true;
+    }
 
-        reader.close(this);
-        reader = null;
+    private void ensureOpen() throws IOException {
+        if (closed) {
+            throw new IOException("image file closed");
+        }
+    }
+
+    private void requireOpen() {
+        if (closed) {
+            throw new IllegalStateException("image file closed");
+        }
     }
 
     // directory management interface
     public Directory getRootDirectory() throws IOException {
-        if (reader == null) {
-            throw new IOException("image file closed");
-        }
+        ensureOpen();
         return reader.getRootDirectory();
     }
 
+
     public Node findNode(String name) throws IOException {
-        if (reader == null) {
-            throw new IOException("image file closed");
-        }
+        ensureOpen();
         return reader.findNode(name);
     }
 
     public byte[] getResource(Node node) throws IOException {
-        if (reader == null) {
-            throw new IOException("image file closed");
-        }
+        ensureOpen();
         return reader.getResource(node);
     }
 
     public byte[] getResource(Resource rs) throws IOException {
-        if (reader == null) {
-            throw new IOException("image file closed");
-        }
+        ensureOpen();
         return reader.getResource(rs);
     }
 
     public ImageHeader getHeader() {
-        Objects.requireNonNull(reader, "image file closed");
+        requireOpen();
         return reader.getHeader();
     }
 
@@ -118,42 +124,42 @@
     }
 
     public String getName() {
-        Objects.requireNonNull(reader, "image file closed");
-        return reader.getName() ;
+        requireOpen();
+        return reader.getName();
     }
 
     public ByteOrder getByteOrder() {
-        Objects.requireNonNull(reader, "image file closed");
+        requireOpen();
         return reader.getByteOrder();
     }
 
     public Path getImagePath() {
-        Objects.requireNonNull(reader, "image file closed");
+        requireOpen();
         return reader.getImagePath();
     }
 
     public ImageStringsReader getStrings() {
-        Objects.requireNonNull(reader, "image file closed");
+        requireOpen();
         return reader.getStrings();
     }
 
     public ImageLocation findLocation(String mn, String rn) {
-        Objects.requireNonNull(reader, "image file closed");
+        requireOpen();
         return reader.findLocation(mn, rn);
     }
 
     public ImageLocation findLocation(String name) {
-        Objects.requireNonNull(reader, "image file closed");
+        requireOpen();
         return reader.findLocation(name);
     }
 
     public String[] getEntryNames() {
-        Objects.requireNonNull(reader, "image file closed");
+        requireOpen();
         return reader.getEntryNames();
     }
 
     public String[] getModuleNames() {
-        Objects.requireNonNull(reader, "image file closed");
+        requireOpen();
         int off = "/modules/".length();
         return reader.findNode("/modules")
                      .getChildren()
@@ -164,32 +170,32 @@
     }
 
     public long[] getAttributes(int offset) {
-        Objects.requireNonNull(reader, "image file closed");
+        requireOpen();
         return reader.getAttributes(offset);
     }
 
     public String getString(int offset) {
-        Objects.requireNonNull(reader, "image file closed");
+        requireOpen();
         return reader.getString(offset);
     }
 
     public byte[] getResource(String name) {
-        Objects.requireNonNull(reader, "image file closed");
+        requireOpen();
         return reader.getResource(name);
     }
 
     public byte[] getResource(ImageLocation loc) {
-        Objects.requireNonNull(reader, "image file closed");
+        requireOpen();
         return reader.getResource(loc);
     }
 
     public ByteBuffer getResourceBuffer(ImageLocation loc) {
-        Objects.requireNonNull(reader, "image file closed");
+        requireOpen();
         return reader.getResourceBuffer(loc);
     }
 
     public InputStream getResourceStream(ImageLocation loc) {
-        Objects.requireNonNull(reader, "image file closed");
+        requireOpen();
         return reader.getResourceStream(loc);
     }
 
--- a/jdk/src/java.base/share/classes/jdk/internal/jimage/ImageReaderFactory.java	Wed Feb 15 21:46:50 2017 +0800
+++ b/jdk/src/java.base/share/classes/jdk/internal/jimage/ImageReaderFactory.java	Wed Feb 15 15:57:18 2017 +0100
@@ -32,6 +32,7 @@
 import java.util.concurrent.ConcurrentHashMap;
 import java.util.Map;
 import java.util.Objects;
+import java.util.function.Function;
 
 /**
  * Factory to get ImageReader
@@ -56,21 +57,23 @@
      */
     public static ImageReader get(Path jimage) throws IOException {
         Objects.requireNonNull(jimage);
-        ImageReader reader = readers.get(jimage);
-        if (reader != null) {
-            return reader;
-        }
-        reader = ImageReader.open(jimage);
-        // potential race with other threads opening the same URL
-        ImageReader r = readers.putIfAbsent(jimage, reader);
-        if (r == null) {
-            return reader;
-        } else {
-            reader.close();
-            return r;
+        try {
+            return readers.computeIfAbsent(jimage, OPENER);
+        } catch (UncheckedIOException io) {
+            throw io.getCause();
         }
     }
 
+    private static Function<Path, ImageReader> OPENER = new Function<Path, ImageReader>() {
+        public ImageReader apply(Path path) {
+            try {
+                return ImageReader.open(path);
+            } catch (IOException io) {
+                throw new UncheckedIOException(io);
+            }
+        }
+    };
+
     /**
      * Returns the {@code ImageReader} to read the image file in this
      * run-time image.