Bug 58480: Work around problem where on Windows systems a Mapped Buffer can still lock a file even if the Channel was closed properly. Use reflection as DirectBuffer is in package sun.com and thus likely to go away with Java 9.

git-svn-id: https://svn.apache.org/repos/asf/poi/trunk@1708609 13f79535-47bb-0310-9956-ffa450edef68

2 files changed, 79 insertions, 29 deletions

diff --git a/src/java/org/apache/poi/hssf/usermodel/HSSFWorkbook.java b/src/java/org/apache/poi/hssf/usermodel/HSSFWorkbook.java
index 09c3066668..fe1eb26d1a 100644
--- a/src/java/org/apache/poi/hssf/usermodel/HSSFWorkbook.java
+++ b/src/java/org/apache/poi/hssf/usermodel/HSSFWorkbook.java
@@ -1372,36 +1372,38 @@ public final class HSSFWorkbook extends POIDocument implements org.apache.poi.ss
 	public void write(OutputStream stream)
             throws IOException
     {
-        byte[] bytes = getBytes();
         NPOIFSFileSystem fs = new NPOIFSFileSystem();
-
-        // For tracking what we've written out, used if we're
-        //  going to be preserving nodes
-        List<String> excepts = new ArrayList<String>(1);
-
-        // Write out the Workbook stream
-        fs.createDocument(new ByteArrayInputStream(bytes), "Workbook");
-
-        // Write out our HPFS properties, if we have them
-        writeProperties(fs, excepts);
-
-        if (preserveNodes) {
-            // Don't write out the old Workbook, we'll be doing our new one
-            // If the file had an "incorrect" name for the workbook stream,
-            // don't write the old one as we'll use the correct name shortly
-        	excepts.addAll(Arrays.asList(WORKBOOK_DIR_ENTRY_NAMES));
-
-            // Copy over all the other nodes to our new poifs
-            EntryUtils.copyNodes(
-                    new FilteringDirectoryNode(this.directory, excepts)
-                    , new FilteringDirectoryNode(fs.getRoot(), excepts)
-            );
-
-            // YK: preserve StorageClsid, it is important for embedded workbooks,
-            // see Bugzilla 47920
-            fs.getRoot().setStorageClsid(this.directory.getStorageClsid());
+        try {
+            // For tracking what we've written out, used if we're
+            //  going to be preserving nodes
+            List<String> excepts = new ArrayList<String>(1);
+    
+            // Write out the Workbook stream
+            fs.createDocument(new ByteArrayInputStream(getBytes()), "Workbook");
+    
+            // Write out our HPFS properties, if we have them
+            writeProperties(fs, excepts);
+    
+            if (preserveNodes) {
+                // Don't write out the old Workbook, we'll be doing our new one
+                // If the file had an "incorrect" name for the workbook stream,
+                // don't write the old one as we'll use the correct name shortly
+            	excepts.addAll(Arrays.asList(WORKBOOK_DIR_ENTRY_NAMES));
+    
+                // Copy over all the other nodes to our new poifs
+                EntryUtils.copyNodes(
+                        new FilteringDirectoryNode(this.directory, excepts)
+                        , new FilteringDirectoryNode(fs.getRoot(), excepts)
+                );
+    
+                // YK: preserve StorageClsid, it is important for embedded workbooks,
+                // see Bugzilla 47920
+                fs.getRoot().setStorageClsid(this.directory.getStorageClsid());
+            }
+            fs.writeFilesystem(stream);
+        } finally {
+            fs.close();
         }
-        fs.writeFilesystem(stream);
     }
 
     /**
diff --git a/src/java/org/apache/poi/poifs/nio/FileBackedDataSource.java b/src/java/org/apache/poi/poifs/nio/FileBackedDataSource.java
index 2e4c7cd5fc..424fad1ced 100644
--- a/src/java/org/apache/poi/poifs/nio/FileBackedDataSource.java
+++ b/src/java/org/apache/poi/poifs/nio/FileBackedDataSource.java
@@ -22,10 +22,14 @@ import java.io.FileNotFoundException;
 import java.io.IOException;
 import java.io.OutputStream;
 import java.io.RandomAccessFile;
+import java.lang.reflect.InvocationTargetException;
+import java.lang.reflect.Method;
 import java.nio.ByteBuffer;
 import java.nio.channels.Channels;
 import java.nio.channels.FileChannel;
 import java.nio.channels.WritableByteChannel;
+import java.util.ArrayList;
+import java.util.List;
 
 import org.apache.poi.util.IOUtils;
 
@@ -37,6 +41,14 @@ public class FileBackedDataSource extends DataSource {
    private boolean writable;
    // remember file base, which needs to be closed too
    private RandomAccessFile srcFile;
+   
+   // Buffers which map to a file-portion are not closed automatically when the Channel is closed
+   // therefore we need to keep the list of mapped buffers and do some ugly reflection to try to 
+   // clean the buffer during close().
+   // See https://bz.apache.org/bugzilla/show_bug.cgi?id=58480, 
+   // http://stackoverflow.com/questions/3602783/file-access-synchronized-on-java-object and
+   // http://bugs.java.com/view_bug.do?bug_id=4724038 for related discussions
+   private List<ByteBuffer> buffersToClean = new ArrayList<ByteBuffer>();
 
    public FileBackedDataSource(File file) throws FileNotFoundException {
        this(newSrcFile(file, "r"), true);
@@ -91,6 +103,9 @@ public class FileBackedDataSource extends DataSource {
       // Ready it for reading
       dst.position(0);
 
+      // remember the buffer for cleanup if necessary
+      buffersToClean.add(dst);
+      
       // All done
       return dst;
    }
@@ -115,7 +130,14 @@ public class FileBackedDataSource extends DataSource {
 
    @Override
    public void close() throws IOException {
-      if (srcFile != null) {
+	   // also ensure that all buffers are unmapped so we do not keep files locked on Windows
+	   // We consider it a bug if a Buffer is still in use now! 
+       for(ByteBuffer buffer : buffersToClean) {
+           unmap(buffer);
+       }
+       buffersToClean.clear();
+
+       if (srcFile != null) {
           // see http://bugs.java.com/bugdatabase/view_bug.do?bug_id=4796385
           srcFile.close();
       } else {
@@ -129,4 +151,30 @@ public class FileBackedDataSource extends DataSource {
         }
         return new RandomAccessFile(file, mode);
    }
+
+   // need to use reflection to avoid depending on the sun.nio internal API
+   // unfortunately this might break silently with newer/other Java implementations, 
+   // but we at least have unit-tests which will indicate this when run on Windows
+    private static void unmap(ByteBuffer bb) {
+        Class<?> fcClass = bb.getClass();
+        try {
+            // invoke bb.cleaner().clean(), but do not depend on sun.nio
+            // interfaces
+            Method cleanerMethod = fcClass.getDeclaredMethod("cleaner");
+            cleanerMethod.setAccessible(true);
+            Object cleaner = cleanerMethod.invoke(bb);
+            Method cleanMethod = cleaner.getClass().getDeclaredMethod("clean");
+            cleanMethod.invoke(cleaner);
+        } catch (NoSuchMethodException e) {
+            // e.printStackTrace();
+        } catch (SecurityException e) {
+            // e.printStackTrace();
+        } catch (IllegalAccessException e) {
+            // e.printStackTrace();
+        } catch (IllegalArgumentException e) {
+            // e.printStackTrace();
+        } catch (InvocationTargetException e) {
+            // e.printStackTrace();
+        }
+    }
 }