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

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(
+       this(null, file, readOnly, true);
-           (new RandomAccessFile(file, readOnly? "r" : "rw")).getChannel(),
-           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) {

src/java/org/apache/poi/poifs/nio/FileBackedDataSource.java

@@ -35,14 +35,20 @@ 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()) {
+       this(newSrcFile(file, "r"), true);
-         throw new FileNotFoundException(file.toString());
    }
-      this.channel = (new RandomAccessFile(file, "r")).getChannel();
+
-      this.writable = false;
+   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) {
@@ -54,6 +60,10 @@ public class FileBackedDataSource extends DataSource {
        return this.writable;
    }
    
+   public FileChannel getChannel() {
+       return this.channel;
+   }
+
    @Override
    public ByteBuffer read(int length, long position) throws IOException {
       if(position >= size()) {
@@ -105,6 +115,18 @@ public class FileBackedDataSource extends DataSource {
 
    @Override
    public void close() throws IOException {
+      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);
+   }
 }

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,6 +829,7 @@ public class TestWrite
      */
     @Test
     public void inPlaceNPOIFSWrite() throws Exception {
+        try {
             NPOIFSFileSystem fs = null;
             DirectoryEntry root = null;
             DocumentNode sinfDoc = null;
@@ -1000,6 +1003,16 @@ public class TestWrite
             // 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;
+        }
     }