Bug 59312 - [PATCH] SXSSF does not clean temporary files on .dispose() reliably
Summary: [PATCH] SXSSF does not clean temporary files on .dispose() reliably
Status: RESOLVED FIXED
Alias: None
Product: POI
Classification: Unclassified
Component: SXSSF (show other bugs)
Version: unspecified
Hardware: All All
: P2 blocker (vote)
Target Milestone: ---
Assignee: POI Developers List
URL:
Keywords:
Depends on:
Blocks:
 
Reported: 2016-04-12 14:50 UTC by Travis Burtrum
Modified: 2016-05-20 18:07 UTC (History)
0 users



Attachments

Note You need to log in before you can comment on or make changes to this bug.
Description Travis Burtrum 2016-04-12 14:50:28 UTC
The offending function is here:

https://github.com/apache/poi/blob/d94ff1aa8e92b443e06008c35e49a6d1f5f891ea/src/ooxml/java/org/apache/poi/xssf/streaming/SheetDataWriter.java#L378

    boolean dispose() throws IOException {
        _out.close();
        return _fd.delete();
    }

The problem is when _out.close() throws an exception, the file handle is never deleted, and to my knowledge there is no way for an outside library user to EVER delete the file?

I suggest the function be changed like so:

    boolean dispose() throws IOException {
        final boolean ret;
        try {
            _out.close();
        } finally {
            ret = _fd.delete();
        }
        return ret;
    }

The way I stumbled upon this bug and what causes it to be a blocking issue is our /tmp is a tmpfs mount with a 2 gigabyte size limit, temporary compression was turned off and SXSSF created a temporary .xml file that filled up the 2 gigabyte limit on /tmp, an exception was thrown from .flush() that the drive was out of space, and when .dispose() was called in a finally block it did not delete the file, causing all subsequent runs of SXSSF to fail in the same way since the drive was still full.

If I can provide more info or help please let me know. Thanks!
Comment 1 Javen O'Neal 2016-04-12 18:50:07 UTC
Thanks for the patch! Applied in r1738848.

Do you think you could also write a unit test to simulate running out of disk space (might need to create some OutputStream that has a fixed size that you can easily exceed)?
Comment 2 Dominik Stadler 2016-05-20 18:07:50 UTC
I took a look and did not see an easy way to do unit-testing of this particular feature. Some other unit-tests cover the "good" case of deleting the file already, the rest needs to be tested manually for now... Still setting this to fixed as the code was adjusted accordingly.