#438 CompoundDocument file descriptor fix

(cherry picked from commit 04f27a1)
This commit is contained in:
Harald Kuhr 2018-09-08 16:21:01 +02:00
parent 586359e7ab
commit 0cd250646a
9 changed files with 111 additions and 97 deletions

View File

@ -186,7 +186,7 @@ abstract class AbstractCachedSeekableStream extends SeekableInputStream {
}
protected void closeImpl() throws IOException {
cache.flush(position);
cache.close();
cache = null;
stream.close();
}
@ -198,12 +198,12 @@ abstract class AbstractCachedSeekableStream extends SeekableInputStream {
* @author last modified by $Author: haku $
* @version $Id: //depot/branches/personal/haraldk/twelvemonkeys/release-2/twelvemonkeys-core/src/main/java/com/twelvemonkeys/io/AbstractCachedSeekableStream.java#2 $
*/
public static abstract class StreamCache {
static abstract class StreamCache {
/**
* Creates a {@code StreamCache}.
*/
protected StreamCache() {
StreamCache() {
}
/**
@ -303,5 +303,7 @@ abstract class AbstractCachedSeekableStream extends SeekableInputStream {
* @throws IOException if the position can't be determined because of a problem in the cache backing mechanism.
*/
abstract long getPosition() throws IOException;
abstract void close() throws IOException;
}
}

View File

@ -124,7 +124,9 @@ public final class FileCacheSeekableStream extends AbstractCachedSeekableStream
@Override
protected void closeImpl() throws IOException {
// TODO: Close cache file
super.closeImpl();
buffer = null;
}
@ -194,39 +196,45 @@ public final class FileCacheSeekableStream extends AbstractCachedSeekableStream
return length;
}
// TODO: We need to close the cache file!!! Otherwise we are leaking file descriptors
final static class FileCache extends StreamCache {
private RandomAccessFile mCacheFile;
private RandomAccessFile cacheFile;
public FileCache(final File pFile) throws FileNotFoundException {
Validate.notNull(pFile, "file");
mCacheFile = new RandomAccessFile(pFile, "rw");
cacheFile = new RandomAccessFile(pFile, "rw");
}
public void write(final int pByte) throws IOException {
mCacheFile.write(pByte);
cacheFile.write(pByte);
}
@Override
public void write(final byte[] pBuffer, final int pOffset, final int pLength) throws IOException {
mCacheFile.write(pBuffer, pOffset, pLength);
cacheFile.write(pBuffer, pOffset, pLength);
}
public int read() throws IOException {
return mCacheFile.read();
return cacheFile.read();
}
@Override
public int read(final byte[] pBuffer, final int pOffset, final int pLength) throws IOException {
return mCacheFile.read(pBuffer, pOffset, pLength);
return cacheFile.read(pBuffer, pOffset, pLength);
}
public void seek(final long pPosition) throws IOException {
mCacheFile.seek(pPosition);
cacheFile.seek(pPosition);
}
public long getPosition() throws IOException {
return mCacheFile.getFilePointer();
return cacheFile.getFilePointer();
}
@Override
void close() throws IOException {
cacheFile.close();
}
}
}

View File

@ -53,7 +53,6 @@ import java.io.*;
* which this class imitates reads big endian quantities.
* <p/>
* <em>Warning:
* <!-- Beware of little indians! -->
* The {@code DataInput} and {@code DataOutput} interfaces
* specifies big endian byte order in their documentation.
* This means that this class is, strictly speaking, not a proper

View File

@ -52,7 +52,6 @@ import java.io.*;
* imitates uses big endian integers.
* <p/>
* <em>Warning:
* <!-- Beware of little indians! -->
* The {@code DataInput} and {@code DataOutput} interfaces
* specifies big endian byte order in their documentation.
* This means that this class is, strictly speaking, not a proper

View File

@ -38,7 +38,6 @@ import java.nio.channels.FileChannel;
* and writing data in little endian byte order.
* <p/>
* <em>Warning:
* <!-- Beware of little indians! -->
* The {@code DataInput} and {@code DataOutput} interfaces
* specifies big endian byte order in their documentation.
* This means that this class is, strictly speaking, not a proper

View File

@ -67,7 +67,7 @@ public final class MemoryCacheSeekableStream extends AbstractCachedSeekableStrea
final static class MemoryCache extends StreamCache {
final static int BLOCK_SIZE = 1 << 13;
private final List<byte[]> cache = new ArrayList<byte[]>();
private final List<byte[]> cache = new ArrayList<>();
private long length;
private long position;
private long start;
@ -186,6 +186,11 @@ public final class MemoryCacheSeekableStream extends AbstractCachedSeekableStrea
start = pPosition;
}
@Override
void close() throws IOException {
cache.clear();
}
public long getPosition() {
return position;
}

View File

@ -35,11 +35,14 @@ import com.twelvemonkeys.lang.StringUtil;
import javax.imageio.stream.ImageInputStream;
import java.io.*;
import java.nio.ByteOrder;
import java.util.Arrays;
import java.util.SortedSet;
import java.util.TreeSet;
import java.util.UUID;
import static com.twelvemonkeys.lang.Validate.notNull;
/**
* Represents a read-only OLE2 compound document.
* <p/>
@ -52,7 +55,7 @@ import java.util.UUID;
* @author last modified by $Author: haku $
* @version $Id: //depot/branches/personal/haraldk/twelvemonkeys/release-2/twelvemonkeys-core/src/main/java/com/twelvemonkeys/io/ole2/CompoundDocument.java#4 $
*/
public final class CompoundDocument {
public final class CompoundDocument implements AutoCloseable {
// TODO: Write support...
// TODO: Properties: http://support.microsoft.com/kb/186898
@ -96,13 +99,18 @@ public final class CompoundDocument {
/**
* Creates a (for now) read only {@code CompoundDocument}.
* <p/>
* <em>Warning! You must invoke {@link #close()} on the compound document
* created from this constructor when done, to avoid leaking file
* descriptors.</em>
*
* @param pFile the file to read from
* @param file the file to read from
*
* @throws IOException if an I/O exception occurs while reading the header
*/
public CompoundDocument(final File pFile) throws IOException {
input = new LittleEndianRandomAccessFile(FileUtil.resolve(pFile), "r");
public CompoundDocument(final File file) throws IOException {
// TODO: We need to close this (or it's underlying RAF)! Otherwise we're leaking file descriptors!
input = new LittleEndianRandomAccessFile(FileUtil.resolve(file), "r");
// TODO: Might be better to read header on first read operation?!
// OTOH: It's also good to be fail-fast, so at least we should make
@ -113,17 +121,17 @@ public final class CompoundDocument {
/**
* Creates a read only {@code CompoundDocument}.
*
* @param pInput the input to read from
* @param pInput the input to read from.
*
* @throws IOException if an I/O exception occurs while reading the header
*/
public CompoundDocument(final InputStream pInput) throws IOException {
this(new FileCacheSeekableStream(pInput));
this(new MemoryCacheSeekableStream(pInput));
}
// For testing only, consider exposing later
CompoundDocument(final SeekableInputStream pInput) throws IOException {
input = new SeekableLittleEndianDataInputStream(pInput);
CompoundDocument(final SeekableInputStream stream) throws IOException {
input = new SeekableLittleEndianDataInputStream(stream);
// TODO: Might be better to read header on first read operation?!
// OTOH: It's also good to be fail-fast, so at least we should make
@ -134,12 +142,15 @@ public final class CompoundDocument {
/**
* Creates a read only {@code CompoundDocument}.
*
* @param pInput the input to read from
* @param input the input to read from
*
* @throws IOException if an I/O exception occurs while reading the header
*/
public CompoundDocument(final ImageInputStream pInput) throws IOException {
input = pInput;
public CompoundDocument(final ImageInputStream input) throws IOException {
this.input = notNull(input, "input");
// This implementation only supports little endian (Intel) CompoundDocuments
input.setByteOrder(ByteOrder.LITTLE_ENDIAN);
// TODO: Might be better to read header on first read operation?!
// OTOH: It's also good to be fail-fast, so at least we should make
@ -147,6 +158,27 @@ public final class CompoundDocument {
readHeader();
}
/**
* This method will close the underlying {@link RandomAccessFile} if any,
* but will leave any stream created outside the document open.
*
* @see #CompoundDocument(File)
* @see RandomAccessFile#close()
*
* @throws IOException if an I/O error occurs.
*/
@Override
public void close() throws IOException {
if (input instanceof RandomAccessFile) {
((RandomAccessFile) input).close();
}
else if (input instanceof LittleEndianRandomAccessFile) {
((LittleEndianRandomAccessFile) input).close();
}
// Other streams are left open
}
public static boolean canRead(final DataInput pInput) {
return canRead(pInput, true);
}

View File

@ -72,64 +72,64 @@ public class CompoundDocumentTest {
@Test
public void testRoot() throws IOException {
CompoundDocument document = createTestDocument();
try (CompoundDocument document = createTestDocument()) {
Entry root = document.getRootEntry();
Entry root = document.getRootEntry();
assertNotNull(root);
assertEquals("Root Entry", root.getName());
assertTrue(root.isRoot());
assertFalse(root.isFile());
assertFalse(root.isDirectory());
assertEquals(0, root.length());
assertNull(root.getInputStream());
assertNotNull(root);
assertEquals("Root Entry", root.getName());
assertTrue(root.isRoot());
assertFalse(root.isFile());
assertFalse(root.isDirectory());
assertEquals(0, root.length());
assertNull(root.getInputStream());
}
}
@Test
public void testContents() throws IOException {
CompoundDocument document = createTestDocument();
try (CompoundDocument document = createTestDocument()) {
Entry root = document.getRootEntry();
Entry root = document.getRootEntry();
assertNotNull(root);
assertNotNull(root);
SortedSet<Entry> children = new TreeSet<Entry>(root.getChildEntries());
assertEquals(25, children.size());
SortedSet<Entry> children = new TreeSet<Entry>(root.getChildEntries());
assertEquals(25, children.size());
// Weirdness in the file format, name is *written backwards* 1-24 + Catalog
for (String name : "1,2,3,4,5,6,7,8,9,01,02,11,12,21,22,31,32,41,42,51,61,71,81,91,Catalog".split(",")) {
assertEquals(name, children.first().getName());
children.remove(children.first());
// Weirdness in the file format, name is *written backwards* 1-24 + Catalog
for (String name : "1,2,3,4,5,6,7,8,9,01,02,11,12,21,22,31,32,41,42,51,61,71,81,91,Catalog".split(",")) {
assertEquals(name, children.first().getName());
children.remove(children.first());
}
}
}
@Test(expected = UnsupportedOperationException.class)
public void testChildEntriesUnmodifiable() throws IOException {
CompoundDocument document = createTestDocument();
try (CompoundDocument document = createTestDocument()) {
Entry root = document.getRootEntry();
Entry root = document.getRootEntry();
assertNotNull(root);
assertNotNull(root);
SortedSet<Entry> children = root.getChildEntries();
SortedSet<Entry> children = root.getChildEntries();
// Should not be allowed, as it modifies the internal structure
children.remove(children.first());
// Should not be allowed, as it modifies the internal structure
children.remove(children.first());
}
}
@Test
public void testReadThumbsCatalogFile() throws IOException {
CompoundDocument document = createTestDocument();
try (CompoundDocument document = createTestDocument()) {
Entry root = document.getRootEntry();
Entry root = document.getRootEntry();
assertNotNull(root);
assertEquals(25, root.getChildEntries().size());
assertNotNull(root);
assertEquals(25, root.getChildEntries().size());
Entry catalog = root.getChildEntry("Catalog");
Entry catalog = root.getChildEntry("Catalog");
assertNotNull(catalog);
assertNotNull("Input stream may not be null", catalog.getInputStream());
assertNotNull(catalog);
assertNotNull("Input stream may not be null", catalog.getInputStream());
}
}
@Test

View File

@ -33,12 +33,12 @@ package com.twelvemonkeys.io.ole2;
import com.twelvemonkeys.io.InputStreamAbstractTest;
import com.twelvemonkeys.io.LittleEndianDataOutputStream;
import com.twelvemonkeys.io.MemoryCacheSeekableStream;
import com.twelvemonkeys.io.SeekableInputStream;
import org.junit.Test;
import java.io.*;
import java.net.URISyntaxException;
import java.net.URL;
import java.io.ByteArrayInputStream;
import java.io.ByteArrayOutputStream;
import java.io.IOException;
import java.io.InputStream;
import java.nio.charset.Charset;
import java.util.Arrays;
@ -51,35 +51,7 @@ import static org.junit.Assert.*;
* @author last modified by $Author: haraldk$
* @version $Id: CompoundDocument_StreamTestCase.java,v 1.0 13.10.11 12:01 haraldk Exp$
*/
//@Ignore("Need proper in-memory creation of CompoundDocuments")
public class CompoundDocument_StreamTest extends InputStreamAbstractTest {
private static final String SAMPLE_DATA = "/Thumbs-camera.db";
protected final CompoundDocument createTestDocument() throws IOException {
URL input = getClass().getResource(SAMPLE_DATA);
assertNotNull("Missing test resource!", input);
assertEquals("Test resource not a file:// resource", "file", input.getProtocol());
try {
return new CompoundDocument(new File(input.toURI()));
}
catch (URISyntaxException e) {
throw new AssertionError(e);
}
}
private SeekableInputStream createRealInputStream() {
try {
Entry first = createTestDocument().getRootEntry().getChildEntries().first();
assertNotNull(first);
return first.getInputStream();
}
catch (IOException e) {
throw new AssertionError(e);
}
}
@Override
protected InputStream makeInputStream(byte[] data) {
try {
@ -182,15 +154,13 @@ public class CompoundDocument_StreamTest extends InputStreamAbstractTest {
return pad;
}
// @Ignore
@Test
public void testDev() throws IOException {
public void testStreamRead() throws IOException {
InputStream stream = makeInputStream(makeOrderedArray(32));
int read;
int count = 0;
while ((read = stream.read()) >= 0) {
// System.out.printf("read %02d: 0x%02x%n", count, read & 0xFF);
assertEquals(count, read);
count++;
}