Bug 57666 - SXSSFWorkbook.dispose() does not delete temp xml files of sheets copied from an existing XSSFWorkbook at construction time
Summary: SXSSFWorkbook.dispose() does not delete temp xml files of sheets copied from ...
Status: RESOLVED FIXED
Alias: None
Product: POI
Classification: Unclassified
Component: SXSSF (show other bugs)
Version: 3.11-FINAL
Hardware: PC Windows NT
: P2 normal (vote)
Target Milestone: ---
Assignee: POI Developers List
URL:
Keywords:
Depends on:
Blocks:
 
Reported: 2015-03-05 12:07 UTC by Yaniv Kunda
Modified: 2015-04-29 19:28 UTC (History)
0 users



Attachments

Note You need to log in before you can comment on or make changes to this bug.
Description Yaniv Kunda 2015-03-05 12:07:45 UTC
Any sheets copied from an existing workbook when constructing a SXSSFWorkbook are not deleted by the dispose() method.

For example, assume "existingWorkbook" contains n sheets:

    SXSSFWorkbook wb = SXSSFWorkbook(existingWorkbook);
    wb.createSheet("New");
    wb.dispose();

After calling the constructor, a temporary xml file is created for each new sheet copied from an existing sheet.
After calling wb.createSheet a temporary xml file is created for the fresh sheet.
After calling dispose(), only the temporary xml file created by createSheet() will be deleted.

I didn't provide a unit test since it's hard to to check for the internally created temp files, and also because File.delete() is not guaranteed to be actually deleted after returning with "true" because the file system operation might be async.
Comment 1 Nick Burch 2015-03-05 12:34:29 UTC
Any chance of a patch to fix this?
Comment 2 Yaniv Kunda 2015-03-05 14:15:13 UTC
(In reply to Nick Burch from comment #1)
> Any chance of a patch to fix this?

Actually, I've looked further into this and the problem might be different:
In my case (and I failed to mentioned this), after creating the SXSSFWorkbook using the existing XSSFWorkbook, I've deleted the sheet created as a copy from the XSSFWorkbook, using removeSheetAt().

So the problem might in fact be that removeSheetAt() does not delete its writer's temp file.
Comment 3 Yaniv Kunda 2015-03-05 14:22:02 UTC
I don't have a proper environment for patching right now, but I would change SXSSFWorkbook.removeSheetAt(int) from

    public void removeSheetAt(int index)
    {
        XSSFSheet xSheet=_wb.getSheetAt(index);
        _wb.removeSheetAt(index);
        deregisterSheetMapping(xSheet);
    }

to

    public void removeSheetAt(int index)
    {
        XSSFSheet xSheet=_wb.getSheetAt(index);
        _wb.removeSheetAt(index);
        SXSSFSheet sxSheet = getSXSSFSheet(xSheet);
        sxSheet.dispose();
        deregisterSheetMapping(xSheet);
    }
Comment 4 Yaniv Kunda 2015-03-05 14:33:28 UTC
I also found a workaround applicable for my situation -

This is my problematic code:

            // open a template with a single sheet
            XSSFWorkbook template = new XSSFWorkbook("data/template.xlsx");
            SXSSFWorkbook wb = new SXSSFWorkbook(template);
            try {
                Sheet templateSheet = template.getSheetAt(0);
                // read some stuff from the template
                ...

                // create new sheet
                Sheet sheet = wb.createSheet("Data");

                // remove sheet copied from template
                wb.removeSheetAt(0);

                // fill the new sheet with data
                ...

                wb.write(out);
            } finally {
                wb.dispose();
            }

The workaround is to call wb.dispose() before creating the new sheet.
Comment 5 Nick Burch 2015-04-29 19:28:50 UTC
Thanks, I've added the dispose call on remove in r1676833.