Backups/TwelveMonkeys
2
0
Fork 0
mirror of https://github.com/haraldk/TwelveMonkeys.git synced 2025-08-03 11:35:29 -04:00
Code Issues Projects Releases Wiki Activity

#705 No longer closes streams we didn't open

Browse Source 
(cherry picked from commit ab08ec1e0d699963fde7dc14e9a3cf32b9d10ea1)
This commit is contained in:
Harald Kuhr 2022-10-18 20:25:14 +02:00
parent 73a58266be
commit a98224e652
9 changed files with 86 additions and 63 deletions
Show Stats Download Patch File Download Diff File Expand all files Collapse all files

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

@ -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);
}
}
}

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

@ -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);

7
imageio/imageio-core/src/main/java/com/twelvemonkeys/imageio/stream/Cache.java Normal file
View File

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

16
imageio/imageio-core/src/main/java/com/twelvemonkeys/imageio/stream/DiskCache.java → imageio/imageio-core/src/main/java/com/twelvemonkeys/imageio/stream/FileCache.java
View File

@ -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) {
}
}

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

@ -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");
}

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

@ -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);

40
imageio/imageio-core/src/test/java/com/twelvemonkeys/imageio/stream/BufferedChannelImageInputStreamDiskCacheTest.java → imageio/imageio-core/src/test/java/com/twelvemonkeys/imageio/stream/BufferedChannelImageInputStreamFileCacheTest.java
View File

@ -57,7 +57,7 @@ import static org.mockito.Mockito.verify;
* @version $Id: BufferedFileImageInputStreamTestCase.java,v 1.0 Apr 21, 2009 10:58:48 AM haraldk Exp$
*/
// TODO: Remove this test, and instead test the disk cache directly!
public class BufferedChannelImageInputStreamDiskCacheTest {
public class BufferedChannelImageInputStreamFileCacheTest {
private final Random random = new Random(170984354357234566L);
private InputStream randomDataToInputStream(byte[] data) {
@ -68,7 +68,7 @@ public class BufferedChannelImageInputStreamDiskCacheTest {
@Test
public void testCreate() throws IOException {
try (BufferedChannelImageInputStream stream = new BufferedChannelImageInputStream(new DiskCache(new ByteArrayInputStream(new byte[0]), null))) {
try (BufferedChannelImageInputStream stream = new BufferedChannelImageInputStream(new FileCache(new ByteArrayInputStream(new byte[0]), null))) {
assertEquals("Stream length should be unknown", -1, stream.length());
}
}
@ -76,7 +76,7 @@ public class BufferedChannelImageInputStreamDiskCacheTest {
@Test
public void testCreateNullStream() throws IOException {
try {
new DiskCache((InputStream) null, null);
new FileCache((InputStream) null, null);
fail("Expected IllegalArgumentException");
}
catch (IllegalArgumentException expected) {
@ -90,7 +90,7 @@ public class BufferedChannelImageInputStreamDiskCacheTest {
@Test
public void testCreateNullChannel() throws IOException {
try {
new DiskCache((ReadableByteChannel) null, null);
new FileCache((ReadableByteChannel) null, null);
fail("Expected IllegalArgumentException");
}
catch (IllegalArgumentException expected) {
@ -106,7 +106,7 @@ public class BufferedChannelImageInputStreamDiskCacheTest {
byte[] data = new byte[1024 * 1024];
InputStream input = randomDataToInputStream(data);
try (BufferedChannelImageInputStream stream = new BufferedChannelImageInputStream(new DiskCache(input, null))) {
try (BufferedChannelImageInputStream stream = new BufferedChannelImageInputStream(new FileCache(input, null))) {
assertEquals("Stream length should be unknown", -1, stream.length());
for (byte value : data) {
@ -122,7 +122,7 @@ public class BufferedChannelImageInputStreamDiskCacheTest {
byte[] data = new byte[1024 * 1024];
InputStream input = randomDataToInputStream(data);
try (BufferedChannelImageInputStream stream = new BufferedChannelImageInputStream(new DiskCache(input, null))) {
try (BufferedChannelImageInputStream stream = new BufferedChannelImageInputStream(new FileCache(input, null))) {
assertEquals("Stream length should be unknown", -1, stream.length());
byte[] result = new byte[1024];
@ -141,7 +141,7 @@ public class BufferedChannelImageInputStreamDiskCacheTest {
byte[] data = new byte[1024 * 14];
InputStream input = randomDataToInputStream(data);
try (BufferedChannelImageInputStream stream = new BufferedChannelImageInputStream(new DiskCache(input, null))) {
try (BufferedChannelImageInputStream stream = new BufferedChannelImageInputStream(new FileCache(input, null))) {
assertEquals("Stream length should be unknown", -1, stream.length());
byte[] result = new byte[7];
@ -159,7 +159,7 @@ public class BufferedChannelImageInputStreamDiskCacheTest {
byte[] data = new byte[1024 * 18];
InputStream input = randomDataToInputStream(data);
try (BufferedChannelImageInputStream stream = new BufferedChannelImageInputStream(new DiskCache(input, null))) {
try (BufferedChannelImageInputStream stream = new BufferedChannelImageInputStream(new FileCache(input, null))) {
assertEquals("Stream length should be unknown", -1, stream.length());
byte[] result = new byte[9];
@ -180,7 +180,7 @@ public class BufferedChannelImageInputStreamDiskCacheTest {
byte[] data = new byte[256];
InputStream input = randomDataToInputStream(data);
try (BufferedChannelImageInputStream stream = new BufferedChannelImageInputStream(new DiskCache(input, null))) {
try (BufferedChannelImageInputStream stream = new BufferedChannelImageInputStream(new FileCache(input, null))) {
assertEquals("Stream length should be unknown", -1, stream.length());
byte[] buffer = new byte[data.length * 2];
@ -198,7 +198,7 @@ public class BufferedChannelImageInputStreamDiskCacheTest {
long value = ByteBuffer.wrap(bytes).getLong();
// Create stream
try (ImageInputStream stream = new BufferedChannelImageInputStream(new DiskCache(input, null))) {
try (ImageInputStream stream = new BufferedChannelImageInputStream(new FileCache(input, null))) {
for (int i = 1; i <= 64; i++) {
assertEquals(String.format("bit %d differ", i), (value << (i - 1L)) >>> 63L, stream.readBit());
}
@ -212,7 +212,7 @@ public class BufferedChannelImageInputStreamDiskCacheTest {
long value = ByteBuffer.wrap(bytes).getLong();
// Create stream
try (ImageInputStream stream = new BufferedChannelImageInputStream(new DiskCache(input, null))) {
try (ImageInputStream stream = new BufferedChannelImageInputStream(new FileCache(input, null))) {
for (int i = 1; i <= 64; i++) {
stream.seek(0);
assertEquals(String.format("bit %d differ", i), value >>> (64L - i), stream.readBits(i));
@ -228,7 +228,7 @@ public class BufferedChannelImageInputStreamDiskCacheTest {
long value = ByteBuffer.wrap(bytes).getLong();
// Create stream
try (ImageInputStream stream = new BufferedChannelImageInputStream(new DiskCache(input, null))) {
try (ImageInputStream stream = new BufferedChannelImageInputStream(new FileCache(input, null))) {
for (int i = 1; i <= 60; i++) {
stream.seek(0);
stream.setBitOffset(i % 8);
@ -244,7 +244,7 @@ public class BufferedChannelImageInputStreamDiskCacheTest {
InputStream input = randomDataToInputStream(bytes);
ByteBuffer buffer = ByteBuffer.wrap(bytes).order(ByteOrder.BIG_ENDIAN);
try (final ImageInputStream stream = new BufferedChannelImageInputStream(new DiskCache(input, null))) {
try (final ImageInputStream stream = new BufferedChannelImageInputStream(new FileCache(input, null))) {
stream.setByteOrder(ByteOrder.BIG_ENDIAN);
for (int i = 0; i < bytes.length / 2; i++) {
@ -282,7 +282,7 @@ public class BufferedChannelImageInputStreamDiskCacheTest {
InputStream input = randomDataToInputStream(bytes);
ByteBuffer buffer = ByteBuffer.wrap(bytes).order(ByteOrder.BIG_ENDIAN);
try (final ImageInputStream stream = new BufferedChannelImageInputStream(new DiskCache(input, null))) {
try (final ImageInputStream stream = new BufferedChannelImageInputStream(new FileCache(input, null))) {
stream.setByteOrder(ByteOrder.BIG_ENDIAN);
for (int i = 0; i < bytes.length / 4; i++) {
@ -320,7 +320,7 @@ public class BufferedChannelImageInputStreamDiskCacheTest {
InputStream input = randomDataToInputStream(bytes);
ByteBuffer buffer = ByteBuffer.wrap(bytes).order(ByteOrder.BIG_ENDIAN);
try (final ImageInputStream stream = new BufferedChannelImageInputStream(new DiskCache(input, null))) {
try (final ImageInputStream stream = new BufferedChannelImageInputStream(new FileCache(input, null))) {
stream.setByteOrder(ByteOrder.BIG_ENDIAN);
for (int i = 0; i < bytes.length / 8; i++) {
@ -357,7 +357,7 @@ public class BufferedChannelImageInputStreamDiskCacheTest {
byte[] bytes = new byte[9];
InputStream input = randomDataToInputStream(bytes);
try (final ImageInputStream stream = new BufferedChannelImageInputStream(new DiskCache(input, null))) {
try (final ImageInputStream stream = new BufferedChannelImageInputStream(new FileCache(input, null))) {
stream.seek(1000);
assertEquals(-1, stream.read());
@ -406,11 +406,11 @@ public class BufferedChannelImageInputStreamDiskCacheTest {
@Test
public void testClose() throws IOException {
// Create wrapper stream
InputStream mock = mock(InputStream.class);
ImageInputStream stream = new BufferedChannelImageInputStream(new DiskCache(mock, null));
Cache cache = mock(Cache.class);
ImageInputStream stream = new BufferedChannelImageInputStream(cache);
stream.close();
verify(mock, only()).close();
verify(cache, only()).close();
}
@Test
@ -422,7 +422,7 @@ public class BufferedChannelImageInputStreamDiskCacheTest {
byte[] bytes = new byte[size];
InputStream input = randomDataToInputStream(bytes);
try (BufferedChannelImageInputStream stream = new BufferedChannelImageInputStream(new DiskCache(input, null))) {
try (BufferedChannelImageInputStream stream = new BufferedChannelImageInputStream(new FileCache(input, null))) {
byte[] result = new byte[size];
int head = stream.read(result, 0, 12); // Provoke a buffered read
int len = stream.read(result, 12, size - 12); // Rest of buffer + direct read

6
imageio/imageio-core/src/test/java/com/twelvemonkeys/imageio/stream/BufferedChannelImageInputStreamMemoryCacheTest.java
View File

@ -406,11 +406,11 @@ public class BufferedChannelImageInputStreamMemoryCacheTest {
@Test
public void testClose() throws IOException {
// Create wrapper stream
InputStream mock = mock(InputStream.class);
ImageInputStream stream = new BufferedChannelImageInputStream(new MemoryCache(mock));
Cache cache = mock(Cache.class);
ImageInputStream stream = new BufferedChannelImageInputStream(cache);
stream.close();
verify(mock, only()).close();
verify(cache, only()).close();
}
@Test

11
imageio/imageio-core/src/test/java/com/twelvemonkeys/imageio/stream/BufferedChannelImageInputStreamTest.java
View File

@ -47,7 +47,7 @@ import java.util.Random;
import static com.twelvemonkeys.imageio.stream.BufferedImageInputStreamTest.rangeEquals;
import static org.junit.Assert.*;
import static org.mockito.Mockito.mock;
import static org.mockito.Mockito.only;
import static org.mockito.Mockito.never;
import static org.mockito.Mockito.verify;
/**
@ -403,14 +403,11 @@ public class BufferedChannelImageInputStreamTest {
@Test
public void testCloseChannel() throws IOException {
// NOTE: As the stream-based constructor is chained to the channel-based one,
// we'll rely on the fact that closing the channel will close the stream.
SeekableByteChannel mock = mock(SeekableByteChannel.class);
ImageInputStream stream = new BufferedChannelImageInputStream(mock);
SeekableByteChannel channel = mock(SeekableByteChannel.class);
ImageInputStream stream = new BufferedChannelImageInputStream(channel);
stream.close();
verify(mock, only()).close();
verify(channel, never()).close();
}
@Test