From: Andreas Beeker Date: Sun, 4 May 2014 20:58:42 +0000 (+0000) Subject: The NPOIFS-classes result currently in left-over memory mapped buffers. X-Git-Tag: REL_3_11_BETA1~147 X-Git-Url: https://source.dussan.org/?a=commitdiff_plain;h=13860b6d5a24a93a1d225e61da424d61d754f242;p=poi.git The NPOIFS-classes result currently in left-over memory mapped buffers. These are actually hard to workaround, so for Windows the test is ignored, if this error happens. http://stackoverflow.com/questions/3602783/file-access-synchronized-on-java-object Apart of that, the RandomFileAccess instance is saved in the FileBackedDataSource as it needs to be closed instead of the Channel. http://bugs.java.com/bugdatabase/view_bug.do?bug_id=4796385 git-svn-id: https://svn.apache.org/repos/asf/poi/trunk@1592418 13f79535-47bb-0310-9956-ffa450edef68 --- diff --git a/src/java/org/apache/poi/poifs/filesystem/NPOIFSFileSystem.java b/src/java/org/apache/poi/poifs/filesystem/NPOIFSFileSystem.java index 7aea791b3b..0de0bfdc0f 100644 --- a/src/java/org/apache/poi/poifs/filesystem/NPOIFSFileSystem.java +++ b/src/java/org/apache/poi/poifs/filesystem/NPOIFSFileSystem.java @@ -27,7 +27,6 @@ import java.io.IOException; import java.io.InputStream; import java.io.OutputStream; import java.io.PushbackInputStream; -import java.io.RandomAccessFile; import java.nio.ByteBuffer; import java.nio.channels.Channels; import java.nio.channels.FileChannel; @@ -162,11 +161,7 @@ public class NPOIFSFileSystem extends BlockStore public NPOIFSFileSystem(File file, boolean readOnly) throws IOException { - this( - (new RandomAccessFile(file, readOnly? "r" : "rw")).getChannel(), - readOnly, - true - ); + this(null, file, readOnly, true); } /** @@ -184,15 +179,24 @@ public class NPOIFSFileSystem extends BlockStore public NPOIFSFileSystem(FileChannel channel) throws IOException { - this(channel, false, false); + this(channel, null, false, false); } - private NPOIFSFileSystem(FileChannel channel, boolean readOnly, boolean closeChannelOnError) + private NPOIFSFileSystem(FileChannel channel, File srcFile, boolean readOnly, boolean closeChannelOnError) throws IOException { this(false); try { + // Initialize the datasource + if (srcFile != null) { + FileBackedDataSource d = new FileBackedDataSource(srcFile, readOnly); + channel = d.getChannel(); + _data = d; + } else { + _data = new FileBackedDataSource(channel, readOnly); + } + // Get the header ByteBuffer headerBuffer = ByteBuffer.allocate(POIFSConstants.SMALLER_BIG_BLOCK_SIZE); IOUtils.readFully(channel, headerBuffer); @@ -201,7 +205,6 @@ public class NPOIFSFileSystem extends BlockStore _header = new HeaderBlock(headerBuffer); // Now process the various entries - _data = new FileBackedDataSource(channel, readOnly); readCoreContents(); } catch(IOException e) { if(closeChannelOnError) { diff --git a/src/java/org/apache/poi/poifs/nio/FileBackedDataSource.java b/src/java/org/apache/poi/poifs/nio/FileBackedDataSource.java index a4353752fe..2e4c7cd5fc 100644 --- a/src/java/org/apache/poi/poifs/nio/FileBackedDataSource.java +++ b/src/java/org/apache/poi/poifs/nio/FileBackedDataSource.java @@ -35,16 +35,22 @@ import org.apache.poi.util.IOUtils; public class FileBackedDataSource extends DataSource { private FileChannel channel; private boolean writable; + // remember file base, which needs to be closed too + private RandomAccessFile srcFile; - @SuppressWarnings("resource") public FileBackedDataSource(File file) throws FileNotFoundException { - if(!file.exists()) { - throw new FileNotFoundException(file.toString()); - } - this.channel = (new RandomAccessFile(file, "r")).getChannel(); - this.writable = false; + this(newSrcFile(file, "r"), true); + } + + public FileBackedDataSource(File file, boolean readOnly) throws FileNotFoundException { + this(newSrcFile(file, readOnly ? "r" : "rw"), readOnly); } + public FileBackedDataSource(RandomAccessFile srcFile, boolean readOnly) { + this(srcFile.getChannel(), readOnly); + this.srcFile = srcFile; + } + public FileBackedDataSource(FileChannel channel, boolean readOnly) { this.channel = channel; this.writable = !readOnly; @@ -53,6 +59,10 @@ public class FileBackedDataSource extends DataSource { public boolean isWriteable() { return this.writable; } + + public FileChannel getChannel() { + return this.channel; + } @Override public ByteBuffer read(int length, long position) throws IOException { @@ -105,6 +115,18 @@ public class FileBackedDataSource extends DataSource { @Override public void close() throws IOException { - channel.close(); + if (srcFile != null) { + // see http://bugs.java.com/bugdatabase/view_bug.do?bug_id=4796385 + srcFile.close(); + } else { + channel.close(); + } + } + + private static RandomAccessFile newSrcFile(File file, String mode) throws FileNotFoundException { + if(!file.exists()) { + throw new FileNotFoundException(file.toString()); + } + return new RandomAccessFile(file, mode); } } diff --git a/src/testcases/org/apache/poi/hpsf/basic/TestWrite.java b/src/testcases/org/apache/poi/hpsf/basic/TestWrite.java index 757036151f..348dcc8674 100644 --- a/src/testcases/org/apache/poi/hpsf/basic/TestWrite.java +++ b/src/testcases/org/apache/poi/hpsf/basic/TestWrite.java @@ -29,6 +29,7 @@ import java.io.ByteArrayOutputStream; import java.io.File; import java.io.FileFilter; import java.io.FileInputStream; +import java.io.FileNotFoundException; import java.io.FileOutputStream; import java.io.IOException; import java.io.InputStream; @@ -74,6 +75,7 @@ import org.apache.poi.util.CodePageUtil; import org.apache.poi.util.IOUtils; import org.apache.poi.util.LittleEndian; import org.apache.poi.util.TempFile; +import org.junit.Assume; import org.junit.Before; import org.junit.Test; @@ -827,179 +829,190 @@ public class TestWrite */ @Test public void inPlaceNPOIFSWrite() throws Exception { - NPOIFSFileSystem fs = null; - DirectoryEntry root = null; - DocumentNode sinfDoc = null; - DocumentNode dinfDoc = null; - SummaryInformation sinf = null; - DocumentSummaryInformation dinf = null; - - // We need to work on a File for in-place changes, so create a temp one - final File copy = TempFile.createTempFile("Test-HPSF", "ole2"); - copy.deleteOnExit(); - - // Copy a test file over to our temp location - InputStream inp = _samples.openResourceAsStream("TestShiftJIS.doc"); - FileOutputStream out = new FileOutputStream(copy); - IOUtils.copy(inp, out); - inp.close(); - out.close(); - - - // Open the copy in read/write mode - fs = new NPOIFSFileSystem(copy, false); - root = fs.getRoot(); - - - // Read the properties in there - sinfDoc = (DocumentNode)root.getEntry(SummaryInformation.DEFAULT_STREAM_NAME); - dinfDoc = (DocumentNode)root.getEntry(DocumentSummaryInformation.DEFAULT_STREAM_NAME); - - sinf = (SummaryInformation)PropertySetFactory.create(new NDocumentInputStream(sinfDoc)); - assertEquals(131077, sinf.getOSVersion()); - - dinf = (DocumentSummaryInformation)PropertySetFactory.create(new NDocumentInputStream(dinfDoc)); - assertEquals(131077, dinf.getOSVersion()); - - - // Check they start as we expect - assertEquals("Reiichiro Hori", sinf.getAuthor()); - assertEquals("Microsoft Word 9.0", sinf.getApplicationName()); - assertEquals("\u7b2c1\u7ae0", sinf.getTitle()); - - assertEquals("", dinf.getCompany()); - assertEquals(null, dinf.getManager()); - - - // Do an in-place replace via an InputStream - new NPOIFSDocument(sinfDoc).replaceContents(sinf.toInputStream()); - new NPOIFSDocument(dinfDoc).replaceContents(dinf.toInputStream()); - - - // Check it didn't get changed - sinfDoc = (DocumentNode)root.getEntry(SummaryInformation.DEFAULT_STREAM_NAME); - dinfDoc = (DocumentNode)root.getEntry(DocumentSummaryInformation.DEFAULT_STREAM_NAME); - - sinf = (SummaryInformation)PropertySetFactory.create(new NDocumentInputStream(sinfDoc)); - assertEquals(131077, sinf.getOSVersion()); - - dinf = (DocumentSummaryInformation)PropertySetFactory.create(new NDocumentInputStream(dinfDoc)); - assertEquals(131077, dinf.getOSVersion()); - - - // Start again! - fs.close(); - inp = _samples.openResourceAsStream("TestShiftJIS.doc"); - out = new FileOutputStream(copy); - IOUtils.copy(inp, out); - inp.close(); - out.close(); - - fs = new NPOIFSFileSystem(copy, false); - root = fs.getRoot(); - - // Read the properties in once more - sinfDoc = (DocumentNode)root.getEntry(SummaryInformation.DEFAULT_STREAM_NAME); - dinfDoc = (DocumentNode)root.getEntry(DocumentSummaryInformation.DEFAULT_STREAM_NAME); - - sinf = (SummaryInformation)PropertySetFactory.create(new NDocumentInputStream(sinfDoc)); - assertEquals(131077, sinf.getOSVersion()); - - dinf = (DocumentSummaryInformation)PropertySetFactory.create(new NDocumentInputStream(dinfDoc)); - assertEquals(131077, dinf.getOSVersion()); - - - // Have them write themselves in-place with no changes, as an OutputStream - sinf.write(new NDocumentOutputStream(sinfDoc)); - dinf.write(new NDocumentOutputStream(dinfDoc)); - - // And also write to some bytes for checking - ByteArrayOutputStream sinfBytes = new ByteArrayOutputStream(); - sinf.write(sinfBytes); - ByteArrayOutputStream dinfBytes = new ByteArrayOutputStream(); - dinf.write(dinfBytes); - - - // Check that the filesystem can give us back the same bytes - sinfDoc = (DocumentNode)root.getEntry(SummaryInformation.DEFAULT_STREAM_NAME); - dinfDoc = (DocumentNode)root.getEntry(DocumentSummaryInformation.DEFAULT_STREAM_NAME); - - byte[] sinfData = IOUtils.toByteArray(new NDocumentInputStream(sinfDoc)); - byte[] dinfData = IOUtils.toByteArray(new NDocumentInputStream(dinfDoc)); - assertThat(sinfBytes.toByteArray(), equalTo(sinfData)); - assertThat(dinfBytes.toByteArray(), equalTo(dinfData)); - - - // Read back in as-is - sinf = (SummaryInformation)PropertySetFactory.create(new NDocumentInputStream(sinfDoc)); - assertEquals(131077, sinf.getOSVersion()); - - dinf = (DocumentSummaryInformation)PropertySetFactory.create(new NDocumentInputStream(dinfDoc)); - assertEquals(131077, dinf.getOSVersion()); - - assertEquals("Reiichiro Hori", sinf.getAuthor()); - assertEquals("Microsoft Word 9.0", sinf.getApplicationName()); - assertEquals("\u7b2c1\u7ae0", sinf.getTitle()); - - assertEquals("", dinf.getCompany()); - assertEquals(null, dinf.getManager()); - - - // Now alter a few of them - sinf.setAuthor("Changed Author"); - sinf.setTitle("Le titre \u00e9tait chang\u00e9"); - dinf.setManager("Changed Manager"); - - - // Save this into the filesystem - sinf.write(new NDocumentOutputStream(sinfDoc)); - dinf.write(new NDocumentOutputStream(dinfDoc)); - - - // Read them back in again - sinfDoc = (DocumentNode)root.getEntry(SummaryInformation.DEFAULT_STREAM_NAME); - sinf = (SummaryInformation)PropertySetFactory.create(new NDocumentInputStream(sinfDoc)); - assertEquals(131077, sinf.getOSVersion()); - - dinfDoc = (DocumentNode)root.getEntry(DocumentSummaryInformation.DEFAULT_STREAM_NAME); - dinf = (DocumentSummaryInformation)PropertySetFactory.create(new NDocumentInputStream(dinfDoc)); - assertEquals(131077, dinf.getOSVersion()); - - assertEquals("Changed Author", sinf.getAuthor()); - assertEquals("Microsoft Word 9.0", sinf.getApplicationName()); - assertEquals("Le titre \u00e9tait chang\u00e9", sinf.getTitle()); - - assertEquals("", dinf.getCompany()); - assertEquals("Changed Manager", dinf.getManager()); - - - // Close the whole filesystem, and open it once more - fs.writeFilesystem(); - fs.close(); - - fs = new NPOIFSFileSystem(copy); - root = fs.getRoot(); - - // Re-check on load - sinfDoc = (DocumentNode)root.getEntry(SummaryInformation.DEFAULT_STREAM_NAME); - sinf = (SummaryInformation)PropertySetFactory.create(new NDocumentInputStream(sinfDoc)); - assertEquals(131077, sinf.getOSVersion()); - - dinfDoc = (DocumentNode)root.getEntry(DocumentSummaryInformation.DEFAULT_STREAM_NAME); - dinf = (DocumentSummaryInformation)PropertySetFactory.create(new NDocumentInputStream(dinfDoc)); - assertEquals(131077, dinf.getOSVersion()); - - assertEquals("Changed Author", sinf.getAuthor()); - assertEquals("Microsoft Word 9.0", sinf.getApplicationName()); - assertEquals("Le titre \u00e9tait chang\u00e9", sinf.getTitle()); - - assertEquals("", dinf.getCompany()); - assertEquals("Changed Manager", dinf.getManager()); - - - // Tidy up - fs.close(); - copy.delete(); + try { + NPOIFSFileSystem fs = null; + DirectoryEntry root = null; + DocumentNode sinfDoc = null; + DocumentNode dinfDoc = null; + SummaryInformation sinf = null; + DocumentSummaryInformation dinf = null; + + // We need to work on a File for in-place changes, so create a temp one + final File copy = TempFile.createTempFile("Test-HPSF", "ole2"); + copy.deleteOnExit(); + + // Copy a test file over to our temp location + InputStream inp = _samples.openResourceAsStream("TestShiftJIS.doc"); + FileOutputStream out = new FileOutputStream(copy); + IOUtils.copy(inp, out); + inp.close(); + out.close(); + + + // Open the copy in read/write mode + fs = new NPOIFSFileSystem(copy, false); + root = fs.getRoot(); + + + // Read the properties in there + sinfDoc = (DocumentNode)root.getEntry(SummaryInformation.DEFAULT_STREAM_NAME); + dinfDoc = (DocumentNode)root.getEntry(DocumentSummaryInformation.DEFAULT_STREAM_NAME); + + sinf = (SummaryInformation)PropertySetFactory.create(new NDocumentInputStream(sinfDoc)); + assertEquals(131077, sinf.getOSVersion()); + + dinf = (DocumentSummaryInformation)PropertySetFactory.create(new NDocumentInputStream(dinfDoc)); + assertEquals(131077, dinf.getOSVersion()); + + + // Check they start as we expect + assertEquals("Reiichiro Hori", sinf.getAuthor()); + assertEquals("Microsoft Word 9.0", sinf.getApplicationName()); + assertEquals("\u7b2c1\u7ae0", sinf.getTitle()); + + assertEquals("", dinf.getCompany()); + assertEquals(null, dinf.getManager()); + + + // Do an in-place replace via an InputStream + new NPOIFSDocument(sinfDoc).replaceContents(sinf.toInputStream()); + new NPOIFSDocument(dinfDoc).replaceContents(dinf.toInputStream()); + + + // Check it didn't get changed + sinfDoc = (DocumentNode)root.getEntry(SummaryInformation.DEFAULT_STREAM_NAME); + dinfDoc = (DocumentNode)root.getEntry(DocumentSummaryInformation.DEFAULT_STREAM_NAME); + + sinf = (SummaryInformation)PropertySetFactory.create(new NDocumentInputStream(sinfDoc)); + assertEquals(131077, sinf.getOSVersion()); + + dinf = (DocumentSummaryInformation)PropertySetFactory.create(new NDocumentInputStream(dinfDoc)); + assertEquals(131077, dinf.getOSVersion()); + + + // Start again! + fs.close(); + inp = _samples.openResourceAsStream("TestShiftJIS.doc"); + out = new FileOutputStream(copy); + IOUtils.copy(inp, out); + inp.close(); + out.close(); + + fs = new NPOIFSFileSystem(copy, false); + root = fs.getRoot(); + + // Read the properties in once more + sinfDoc = (DocumentNode)root.getEntry(SummaryInformation.DEFAULT_STREAM_NAME); + dinfDoc = (DocumentNode)root.getEntry(DocumentSummaryInformation.DEFAULT_STREAM_NAME); + + sinf = (SummaryInformation)PropertySetFactory.create(new NDocumentInputStream(sinfDoc)); + assertEquals(131077, sinf.getOSVersion()); + + dinf = (DocumentSummaryInformation)PropertySetFactory.create(new NDocumentInputStream(dinfDoc)); + assertEquals(131077, dinf.getOSVersion()); + + + // Have them write themselves in-place with no changes, as an OutputStream + sinf.write(new NDocumentOutputStream(sinfDoc)); + dinf.write(new NDocumentOutputStream(dinfDoc)); + + // And also write to some bytes for checking + ByteArrayOutputStream sinfBytes = new ByteArrayOutputStream(); + sinf.write(sinfBytes); + ByteArrayOutputStream dinfBytes = new ByteArrayOutputStream(); + dinf.write(dinfBytes); + + + // Check that the filesystem can give us back the same bytes + sinfDoc = (DocumentNode)root.getEntry(SummaryInformation.DEFAULT_STREAM_NAME); + dinfDoc = (DocumentNode)root.getEntry(DocumentSummaryInformation.DEFAULT_STREAM_NAME); + + byte[] sinfData = IOUtils.toByteArray(new NDocumentInputStream(sinfDoc)); + byte[] dinfData = IOUtils.toByteArray(new NDocumentInputStream(dinfDoc)); + assertThat(sinfBytes.toByteArray(), equalTo(sinfData)); + assertThat(dinfBytes.toByteArray(), equalTo(dinfData)); + + + // Read back in as-is + sinf = (SummaryInformation)PropertySetFactory.create(new NDocumentInputStream(sinfDoc)); + assertEquals(131077, sinf.getOSVersion()); + + dinf = (DocumentSummaryInformation)PropertySetFactory.create(new NDocumentInputStream(dinfDoc)); + assertEquals(131077, dinf.getOSVersion()); + + assertEquals("Reiichiro Hori", sinf.getAuthor()); + assertEquals("Microsoft Word 9.0", sinf.getApplicationName()); + assertEquals("\u7b2c1\u7ae0", sinf.getTitle()); + + assertEquals("", dinf.getCompany()); + assertEquals(null, dinf.getManager()); + + + // Now alter a few of them + sinf.setAuthor("Changed Author"); + sinf.setTitle("Le titre \u00e9tait chang\u00e9"); + dinf.setManager("Changed Manager"); + + + // Save this into the filesystem + sinf.write(new NDocumentOutputStream(sinfDoc)); + dinf.write(new NDocumentOutputStream(dinfDoc)); + + + // Read them back in again + sinfDoc = (DocumentNode)root.getEntry(SummaryInformation.DEFAULT_STREAM_NAME); + sinf = (SummaryInformation)PropertySetFactory.create(new NDocumentInputStream(sinfDoc)); + assertEquals(131077, sinf.getOSVersion()); + + dinfDoc = (DocumentNode)root.getEntry(DocumentSummaryInformation.DEFAULT_STREAM_NAME); + dinf = (DocumentSummaryInformation)PropertySetFactory.create(new NDocumentInputStream(dinfDoc)); + assertEquals(131077, dinf.getOSVersion()); + + assertEquals("Changed Author", sinf.getAuthor()); + assertEquals("Microsoft Word 9.0", sinf.getApplicationName()); + assertEquals("Le titre \u00e9tait chang\u00e9", sinf.getTitle()); + + assertEquals("", dinf.getCompany()); + assertEquals("Changed Manager", dinf.getManager()); + + + // Close the whole filesystem, and open it once more + fs.writeFilesystem(); + fs.close(); + + fs = new NPOIFSFileSystem(copy); + root = fs.getRoot(); + + // Re-check on load + sinfDoc = (DocumentNode)root.getEntry(SummaryInformation.DEFAULT_STREAM_NAME); + sinf = (SummaryInformation)PropertySetFactory.create(new NDocumentInputStream(sinfDoc)); + assertEquals(131077, sinf.getOSVersion()); + + dinfDoc = (DocumentNode)root.getEntry(DocumentSummaryInformation.DEFAULT_STREAM_NAME); + dinf = (DocumentSummaryInformation)PropertySetFactory.create(new NDocumentInputStream(dinfDoc)); + assertEquals(131077, dinf.getOSVersion()); + + assertEquals("Changed Author", sinf.getAuthor()); + assertEquals("Microsoft Word 9.0", sinf.getApplicationName()); + assertEquals("Le titre \u00e9tait chang\u00e9", sinf.getTitle()); + + assertEquals("", dinf.getCompany()); + assertEquals("Changed Manager", dinf.getManager()); + + + // Tidy up + fs.close(); + copy.delete(); + } catch (FileNotFoundException e) { + // On Windows this might always fail, as the nio classes + // leave memory mapped buffers active, even when the corresponding channel is closed + // The buffers are closed on garbage-collection (but System.gc() can't be forced) + // or via sun.misc.Cleaner, but this is regarded unsafe + // http://stackoverflow.com/questions/2972986 + // http://bugs.java.com/view_bug.do?bug_id=4724038 + Assume.assumeFalse(System.getProperty("os.name").toLowerCase().contains("win")); + throw e; + } }