#705 No longer closes streams we didn't open

imageio/imageio-core/src/main/java/com/twelvemonkeys/imageio/stream/BufferedChannelImageInputStream.java

@@ -31,6 +31,7 @@
 package com.twelvemonkeys.imageio.stream;
 
 import javax.imageio.stream.ImageInputStreamImpl;
+import java.io.Closeable;
 import java.io.File;
 import java.io.FileInputStream;
 import java.io.IOException;
@@ -52,6 +53,10 @@ import static java.lang.Math.max;
  * for shorter reads, like single byte or bit reads.
  */
 final class BufferedChannelImageInputStream extends ImageInputStreamImpl {
+    private static final Closeable CLOSEABLE_STUB = new Closeable() {
+        @Override public void close() {}
+    };
+
     static final int DEFAULT_BUFFER_SIZE = 8192;
 
     private ByteBuffer byteBuffer = ByteBuffer.allocate(DEFAULT_BUFFER_SIZE);
@@ -63,6 +68,7 @@ final class BufferedChannelImageInputStream extends ImageInputStreamImpl {
     private final byte[] integralCacheArray = integralCache.array();
 
     private SeekableByteChannel channel;
+    private Closeable closeable;
 
     /**
      * Constructs a {@code BufferedChannelImageInputStream} that will read from a given {@code File}.
@@ -86,49 +92,62 @@ final class BufferedChannelImageInputStream extends ImageInputStreamImpl {
      * @throws SecurityException             if a security manager is installed, and it denies read access to the file.
      */
     public BufferedChannelImageInputStream(final Path file) throws IOException {
-        this(FileChannel.open(notNull(file, "file"), StandardOpenOption.READ));
+        this(FileChannel.open(notNull(file, "file"), StandardOpenOption.READ), true);
     }
 
     /**
      * Constructs a {@code BufferedChannelImageInputStream} that will read from a given {@code RandomAccessFile}.
-     * <p>
-     * Closing this stream will close the {@code RandomAccessFile}.
-     * </p>
      *
      * @param file a {@code RandomAccessFile} to read from.
      * @throws IllegalArgumentException if {@code file} is {@code null}.
      */
     public BufferedChannelImageInputStream(final RandomAccessFile file) {
-        // Assumption: Closing the FileChannel will also close its backing RandomAccessFile
-        // (it does in the OpenJDK implementation, and it makes sense, although I can't see this is documented behaviour).
-        this(notNull(file, "file").getChannel());
+        // Closing the RAF is inconsistent, but emulates the behavior of javax.imageio.stream.FileImageInputStream
+        this(notNull(file, "file").getChannel(), true);
     }
 
     /**
      * Constructs a {@code BufferedChannelImageInputStream} that will read from a given {@code FileInputStream}.
      * <p>
-     * Closing this stream will close the {@code FileInputStream}.
+     * Closing this stream will <em>not</em> close the {@code FileInputStream}.
      * </p>
      *
      * @param inputStream a {@code FileInputStream} to read from.
      * @throws IllegalArgumentException if {@code inputStream} is {@code null}.
      */
     public BufferedChannelImageInputStream(final FileInputStream inputStream) {
-        // Assumption: Closing the FileChannel will also close its backing FileInputStream (it does in the OpenJDK implementation, although I can't see this is documented).
-        this(notNull(inputStream, "inputStream").getChannel());
+        this(notNull(inputStream, "inputStream").getChannel(), false);
     }
 
     /**
      * Constructs a {@code BufferedChannelImageInputStream} that will read from a given {@code SeekableByteChannel}.
      * <p>
-     * Closing this stream will close the {@code SeekableByteChannel}.
+     * Closing this stream will <em>not</em> close the {@code SeekableByteChannel}.
      * </p>
      *
      * @param channel a {@code SeekableByteChannel} to read from.
      * @throws IllegalArgumentException if {@code channel} is {@code null}.
      */
     public BufferedChannelImageInputStream(final SeekableByteChannel channel) {
+        this(notNull(channel, "channel"), false);
+    }
+
+    /**
+     * Constructs a {@code BufferedChannelImageInputStream} that will read from a given {@code Cache}.
+     * <p>
+     * Closing this stream will close the {@code Cache}.
+     * </p>
+     *
+     * @param cache a {@code SeekableByteChannel} to read from.
+     * @throws IllegalArgumentException if {@code channel} is {@code null}.
+     */
+    BufferedChannelImageInputStream(final Cache cache) {
+        this(notNull(cache, "cache"), true);
+    }
+
+    private BufferedChannelImageInputStream(final SeekableByteChannel channel, boolean closeChannelOnClose) {
         this.channel = notNull(channel, "channel");
+        this.closeable = closeChannelOnClose ? this.channel : CLOSEABLE_STUB;
     }
 
     @SuppressWarnings("BooleanMethodIsAlwaysInverted")
@@ -246,8 +265,14 @@ final class BufferedChannelImageInputStream extends ImageInputStreamImpl {
         buffer = null;
         byteBuffer = null;
 
-        channel.close();
         channel = null;
+
+        try {
+            closeable.close();
+        }
+        finally {
+            closeable = null;
+        }
     }
 
     // Need to override the readShort(), readInt() and readLong() methods,
@@ -315,9 +340,9 @@ final class BufferedChannelImageInputStream extends ImageInputStreamImpl {
     public void flushBefore(final long pos) throws IOException {
         super.flushBefore(pos);
 
-        if (channel instanceof MemoryCache) {
+        if (channel instanceof Cache) {
             // In case of memory cache, free up memory
-            ((MemoryCache) channel).flushBefore(pos);
+            ((Cache) channel).flushBefore(pos);
         }
     }
 }

imageio/imageio-core/src/main/java/com/twelvemonkeys/imageio/stream/BufferedInputStreamImageInputStreamSpi.java

@@ -53,7 +53,7 @@ public final class BufferedInputStreamImageInputStreamSpi extends ImageInputStre
             }
 
             // Otherwise, create a cache for backwards seeking
-            return new BufferedChannelImageInputStream(useCacheFile ? new DiskCache(channel, cacheDir): new MemoryCache(channel));
+            return new BufferedChannelImageInputStream(useCacheFile ? new FileCache(channel, cacheDir) : new MemoryCache(channel));
         }
 
         throw new IllegalArgumentException("Expected input of type InputStream: " + input);

imageio/imageio-core/src/main/java/com/twelvemonkeys/imageio/stream/Cache.java

@@ -0,0 +1,7 @@
+package com.twelvemonkeys.imageio.stream;
+
+import java.nio.channels.SeekableByteChannel;
+
+interface Cache extends SeekableByteChannel {
+    void flushBefore(long pos);
+}

imageio/imageio-core/src/main/java/com/twelvemonkeys/imageio/stream/FileCache.java

@@ -26,19 +26,19 @@ import static java.nio.file.StandardOpenOption.WRITE;
 //      the usual {@link #read read} and {@link #write write} methods.  From the
 //      standpoint of performance it is generally only worth mapping relatively
 //      large files into memory.
-final class DiskCache implements SeekableByteChannel {
+final class FileCache implements Cache {
     final static int BLOCK_SIZE = 1 << 13;
 
     private final FileChannel cache;
     private final ReadableByteChannel channel;
 
     // TODO: Perhaps skip this constructor?
-    DiskCache(InputStream stream, File cacheDir) throws IOException {
+    FileCache(InputStream stream, File cacheDir) throws IOException {
         // Stream will be closed with channel, documented behavior
         this(Channels.newChannel(notNull(stream, "stream")), cacheDir);
     }
 
-    public DiskCache(ReadableByteChannel channel, File cacheDir) throws IOException {
+    public FileCache(ReadableByteChannel channel, File cacheDir) throws IOException {
         this.channel = notNull(channel, "channel");
         isTrue(cacheDir == null || cacheDir.isDirectory(), cacheDir, "%s is not a directory");
 
@@ -65,12 +65,7 @@ final class DiskCache implements SeekableByteChannel {
 
     @Override
     public void close() throws IOException {
-        try {
-            cache.close();
-        }
-        finally {
-            channel.close();
-        }
+        cache.close();
     }
 
     @Override
@@ -110,5 +105,8 @@ final class DiskCache implements SeekableByteChannel {
     public SeekableByteChannel truncate(long size) {
         throw new NonWritableChannelException();
     }
+
+    @Override public void flushBefore(long pos) {
+    }
 }
 

imageio/imageio-core/src/main/java/com/twelvemonkeys/imageio/stream/MemoryCache.java

@@ -13,7 +13,7 @@ import java.util.List;
 import static com.twelvemonkeys.lang.Validate.notNull;
 import static java.lang.Math.min;
 
-public final class MemoryCache implements SeekableByteChannel {
+final class MemoryCache implements Cache {
 
     final static int BLOCK_SIZE = 1 << 13;
 
@@ -78,12 +78,7 @@ public final class MemoryCache implements SeekableByteChannel {
 
     @Override
     public void close() throws IOException {
-        try {
-            cache.clear();
-        }
-        finally {
-            channel.close();
-        }
+        cache.clear();
     }
 
     @Override
@@ -135,7 +130,8 @@ public final class MemoryCache implements SeekableByteChannel {
         throw new NonWritableChannelException();
     }
 
-    void flushBefore(long pos) {
+    @Override
+    public void flushBefore(long pos) {
         if (pos < start) {
             throw new IndexOutOfBoundsException("pos < flushed position");
         }

imageio/imageio-core/src/main/java/com/twelvemonkeys/imageio/stream/URLImageInputStreamSpi.java

@@ -80,7 +80,7 @@ public final class URLImageInputStreamSpi extends ImageInputStreamSpi {
 
             // Otherwise revert to cached
             InputStream urlStream = url.openStream();
-            return new BufferedChannelImageInputStream(useCacheFile ? new DiskCache(urlStream, cacheDir) : new MemoryCache(urlStream));
+            return new BufferedChannelImageInputStream(useCacheFile ? new FileCache(urlStream, cacheDir) : new MemoryCache(urlStream));
         }
 
         throw new IllegalArgumentException("Expected input of type URL: " + input);