Bug 58480 - HSSFWorkbook.write(OutputStream) does not close the NPOIFSFileSystem in use
Summary: HSSFWorkbook.write(OutputStream) does not close the NPOIFSFileSystem in use
Alias: None
Product: POI
Classification: Unclassified
Component: HSSF (show other bugs)
Version: 3.13-dev
Hardware: PC All
: P2 normal (vote)
Target Milestone: ---
Assignee: POI Developers List
Depends on:
Reported: 2015-10-05 10:09 UTC by Kasper Sørensen
Modified: 2015-10-14 16:57 UTC (History)
2 users (show)

My proposed patch (3.03 KB, patch)
2015-10-05 18:14 UTC, Kasper Sørensen
Details | Diff
Output from run with File Leak Detector active (paths anonymized) (7.66 KB, text/plain)
2015-10-07 09:42 UTC, d
Patch which uses sun.nio stuff to free mapped resources (5.14 KB, patch)
2015-10-08 08:19 UTC, Dominik Stadler
Details | Diff

Note You need to log in before you can comment on or make changes to this bug.
Description Kasper Sørensen 2015-10-05 10:09:16 UTC
It seems that HSSFWorkbook's write(OutputStream) method is dependent on garbage collection to close the NPOIFSFileSystem which is created in HSSFWorkbook line 1376 (POI v. 3.13).

We have an application where we first write the workbook to a byte-array and then we copy that byte-array back to the original file location. Our copy function will fail with FileNotFoundException "The process cannot access the file because it is being used by another process."

What we finally discovered as a workaround for us is to call System.gc(), System.runFinalization() and then Thread.sleep(1000) just after the Workbook.write(..) call. But obviously this is a very silly way for us to work around the issue.

The solution is quite simple: Create a try-finally block for the NPOIFSFileSystem and make sure to close it in the finally-block.
Comment 1 Kasper Sørensen 2015-10-05 18:14:52 UTC
Created attachment 33166 [details]
My proposed patch

Adds a try-finally construct with fs.close() in the finally block.
Comment 2 Dominik Stadler 2015-10-05 19:18:12 UTC
Did you try to close() the OutputStream that you pass to write() afterwards? At least that is what unit tests do which run and where we ensure via file-leak-detector that they do not leave file-resources behind.
Comment 3 Kasper Sørensen 2015-10-06 12:26:55 UTC
That's not the problem IMO. I am closing the stream.

Please see this test case that I just built to reproduce the issue on current trunk:


package org.apache.poi.hssf.usermodel;

import java.io.ByteArrayOutputStream;
import java.io.File;
import java.io.FileOutputStream;
import java.io.IOException;

import org.apache.poi.ss.usermodel.Row;
import org.apache.poi.ss.usermodel.Sheet;
import org.apache.poi.ss.usermodel.Workbook;
import org.apache.poi.ss.usermodel.WorkbookFactory;
import org.junit.Test;

public class HSSFWorkbookTest {

    public void testWriteScenario() throws Exception {
        final File file = new File(

        // create new workbook
            final Workbook workbook = new HSSFWorkbook();
            final Sheet sheet = workbook.createSheet("foo");
            final Row row = sheet.createRow(1);
            writeAndCloseWorkbook(workbook, file);

        // edit the workbook
            final Workbook workbook = WorkbookFactory.create(file);
            final Sheet sheet = workbook.getSheet("foo");
            writeAndCloseWorkbook(workbook, file);

    private void writeAndCloseWorkbook(Workbook workbook, File file)
            throws IOException {
        final ByteArrayOutputStream bytesOut = new ByteArrayOutputStream();

        final byte[] byteArray = bytesOut.toByteArray();

        final FileOutputStream fileOut = new FileOutputStream(file);

Comment 4 Kasper Sørensen 2015-10-06 12:29:55 UTC
Actually... That test still fails even after applying my patch. So the patch is invalidated I guess, but the problem remains. How can we figure out what is keeping a "user-mapped section open" on the file?
Comment 5 Dominik Stadler 2015-10-06 15:17:28 UTC
I usually use http://file-leak-detector.kohsuke.org , which can show via a stacktrace and filename which files remain open if you run your test with it.
Comment 6 Dominik Stadler 2015-10-06 18:14:13 UTC
BTW, file-leak-detector does not show any file remaining open if I run the test-code that you posted. I also verified by looking at open file-handles in Linux that there are none remaining after the close() is executed.

Are you sure you are not holding onto the file elsewhere?
Comment 7 Kasper Sørensen 2015-10-06 20:52:41 UTC
The issue only happens on Windows. I also didn't experience it on Linux.

I'm not holding on to the file - it's created by the test.
Comment 8 Kasper Sørensen 2015-10-06 20:58:17 UTC
The crazy thing is that if I add:


Just after the Workbook.write(...) call, then it never happens! This tells me that _something_ must rely on garbage collection/finalization.

I see that RandomAccessFile is used internally in POI, and I have little experience with that. Don't know if something could still be holding on to it?
Comment 9 d 2015-10-07 09:42:53 UTC
Created attachment 33175 [details]
Output from run with File Leak Detector active (paths anonymized)

Reproduced on Windows 7. File Leak Detector seems to think everything is okay, but the OS/filesystem doesn't (I assume the "The requested operation cannot be performed on a file with a user-mapped section open" error message is coming from the OS).

I'm pretty sure no Unix derivative would ever be mad about this. File locking is almost exclusively advisory.
Comment 10 d 2015-10-07 18:23:46 UTC
I guess we can go back to NEW with reproduce code and a confirmation.
Comment 11 Dominik Stadler 2015-10-08 08:19:14 UTC
Created attachment 33176 [details]
Patch which uses sun.nio stuff to free mapped resources

Hmm, file-leak-detector does not show any open file, also Process Explorer does not show the file being still opened for the process if I debug and stop before the place where it fails.

For me even if I do the close() of NPOIFSFileSystem as you suggest, I still get the error.

It seems we are creating Buffers during reading via NPOIFSFileSystem and these seem to hold on to resources even if the channel is closed.

Some related discussion on Stackoverflow shows similar problems and lists some Java bugs involved as well, e.g. http://stackoverflow.com/questions/3602783/file-access-synchronized-on-java-object, http://stackoverflow.com/a/5036003/411846 and http://stackoverflow.com/a/9779181/411846 as well as http://bugs.java.com/view_bug.do?bug_id=4724038

So the problem seems to be that there is no clean way to free the mapped buffers!

Finally I found that there is a DirectBuffer.cleaner.clean() method which seems to free resources that are still held by the ByteBuffer instances, however DirectBuffer is in package "sun.nio" so it is likely not a good idea to use it, especially as Oracle announced to ditch all these forbidden packages in Java 9. 

See the attached patch for my current solution, not sure what we need to do make this a valid solution for POI itself.
Comment 12 Dominik Stadler 2015-10-14 14:55:03 UTC
I have now implemented a work around for this problem. 

We use reflection as DirectBuffer is in package sun.com and thus may be replaced with Java 9. 

See r1708609
Comment 13 Kasper Sørensen 2015-10-14 16:57:50 UTC
Great - thanks! :-)