From da45ae87d7c59a2d2611c0f425557d59aff83300 Mon Sep 17 00:00:00 2001 From: Yegor Kozlov Date: Fri, 14 Sep 2012 13:45:09 +0000 Subject: [PATCH] Bugzilla 53780: Fixed memory and temporary file leak in SXSSF git-svn-id: https://svn.apache.org/repos/asf/poi/trunk@1384784 13f79535-47bb-0310-9956-ffa450edef68 --- .../content/xdocs/spreadsheet/how-to.xml | 20 ++-- src/documentation/content/xdocs/status.xml | 1 + .../xssf/streaming/GZIPSheetDataWriter.java | 1 - .../apache/poi/xssf/streaming/SXSSFSheet.java | 8 ++ .../poi/xssf/streaming/SXSSFWorkbook.java | 42 +++++++-- .../poi/xssf/streaming/SheetDataWriter.java | 14 ++- .../poi/xssf/SXSSFITestDataProvider.java | 21 ++++- .../poi/xssf/streaming/TestSXSSFCell.java | 8 +- .../xssf/streaming/TestSXSSFHyperlink.java | 6 ++ .../poi/xssf/streaming/TestSXSSFRow.java | 6 ++ .../poi/xssf/streaming/TestSXSSFSheet.java | 7 ++ .../poi/xssf/streaming/TestSXSSFWorkbook.java | 93 +++++++++++++++---- 12 files changed, 189 insertions(+), 38 deletions(-) diff --git a/src/documentation/content/xdocs/spreadsheet/how-to.xml b/src/documentation/content/xdocs/spreadsheet/how-to.xml index f51cf7372b..bb76a6b807 100644 --- a/src/documentation/content/xdocs/spreadsheet/how-to.xml +++ b/src/documentation/content/xdocs/spreadsheet/how-to.xml @@ -626,10 +626,6 @@ public class ExampleEventUserModel {
SXSSF (Streaming Usermodel API) - - SXSSF is a brand new contribution and some features were added after it was first introduced in POI 3.8-beta3. - Users are advised to try the latest build from trunk. Instructions how to build are here. -

SXSSF (package: org.apache.poi.xssf.streaming) is an API-compatible streaming extension of XSSF to be used when very large spreadsheets have to be produced, and heap space is limited. @@ -656,6 +652,9 @@ public class ExampleEventUserModel { records that have not been flushed by a call to flushRows() are available for random access.

+

+ Note that SXSSF allocates temporary files that you must always clean up explicitly, by calling the dispose method. +

The example below writes a sheet with a window of 100 rows. When the row count reaches 101, the row with rownum=0 is flushed to disk and removed from memory, when rownum reaches 102 then the row with rownum=1 is flushed, etc.

@@ -672,7 +671,7 @@ import org.apache.poi.ss.util.CellReference; import org.apache.poi.xssf.streaming.SXSSFWorkbook; public static void main(String[] args) throws Throwable { - Workbook wb = new SXSSFWorkbook(100); // keep 100 rows in memory, exceeding rows will be flushed to disk + SXSSFWorkbook wb = new SXSSFWorkbook(100); // keep 100 rows in memory, exceeding rows will be flushed to disk Sheet sh = wb.createSheet(); for(int rownum = 0; rownum < 1000; rownum++){ Row row = sh.createRow(rownum); @@ -697,6 +696,9 @@ import org.apache.poi.xssf.streaming.SXSSFWorkbook; FileOutputStream out = new FileOutputStream("/temp/sxssf.xlsx"); wb.write(out); out.close(); + + // dispose of temporary files backing this workbook on disk + wb.dispose(); } @@ -712,7 +714,7 @@ import org.apache.poi.ss.util.CellReference; import org.apache.poi.xssf.streaming.SXSSFWorkbook; public static void main(String[] args) throws Throwable { - Workbook wb = new SXSSFWorkbook(-1); // turn off auto-flushing and accumulate all rows in memory + SXSSFWorkbook wb = new SXSSFWorkbook(-1); // turn off auto-flushing and accumulate all rows in memory Sheet sh = wb.createSheet(); for(int rownum = 0; rownum < 1000; rownum++){ Row row = sh.createRow(rownum); @@ -735,7 +737,10 @@ import org.apache.poi.xssf.streaming.SXSSFWorkbook; FileOutputStream out = new FileOutputStream("/temp/sxssf.xlsx"); wb.write(out); out.close(); - } + + // dispose of temporary files backing this workbook on disk + wb.dispose(); + } ]]> @@ -748,7 +753,6 @@ If the size of the temp files is an issue, you can tell SXSSF to use gzip compre wb.setCompressTempFiles(true); // temp files will be gzipped ]]> -
diff --git a/src/documentation/content/xdocs/status.xml b/src/documentation/content/xdocs/status.xml index 5ea601b4c1..8c9595aa4f 100644 --- a/src/documentation/content/xdocs/status.xml +++ b/src/documentation/content/xdocs/status.xml @@ -34,6 +34,7 @@ + 53780 - Fixed memory and temporary file leak in SXSSF 53380 - ArrayIndexOutOfBounds Excetion parsing word 97 document. 53434 - Subtotal is not return correct value. 53642 - [PATCH] XLS formula evaluation logging diff --git a/src/ooxml/java/org/apache/poi/xssf/streaming/GZIPSheetDataWriter.java b/src/ooxml/java/org/apache/poi/xssf/streaming/GZIPSheetDataWriter.java index 509c9df918..2a1a4a80b8 100644 --- a/src/ooxml/java/org/apache/poi/xssf/streaming/GZIPSheetDataWriter.java +++ b/src/ooxml/java/org/apache/poi/xssf/streaming/GZIPSheetDataWriter.java @@ -37,7 +37,6 @@ public class GZIPSheetDataWriter extends SheetDataWriter { */ public File createTempFile()throws IOException { File fd = File.createTempFile("poi-sxssf-sheet-xml", ".gz"); - fd.deleteOnExit(); return fd; } diff --git a/src/ooxml/java/org/apache/poi/xssf/streaming/SXSSFSheet.java b/src/ooxml/java/org/apache/poi/xssf/streaming/SXSSFSheet.java index a5a731b17c..c2c438d7d4 100644 --- a/src/ooxml/java/org/apache/poi/xssf/streaming/SXSSFSheet.java +++ b/src/ooxml/java/org/apache/poi/xssf/streaming/SXSSFSheet.java @@ -1364,4 +1364,12 @@ public class SXSSFSheet implements Sheet, Cloneable } return -1; } + + /** + * Deletes the temporary file that backed this sheet on disk. + * @return true if the file was deleted, false if it wasn't. + */ + boolean dispose() { + return _writer.dispose(); + } } diff --git a/src/ooxml/java/org/apache/poi/xssf/streaming/SXSSFWorkbook.java b/src/ooxml/java/org/apache/poi/xssf/streaming/SXSSFWorkbook.java index 73c4b934cf..207eef1224 100644 --- a/src/ooxml/java/org/apache/poi/xssf/streaming/SXSSFWorkbook.java +++ b/src/ooxml/java/org/apache/poi/xssf/streaming/SXSSFWorkbook.java @@ -760,14 +760,40 @@ public class SXSSFWorkbook implements Workbook //Save the template File tmplFile = File.createTempFile("poi-sxssf-template", ".xlsx"); - tmplFile.deleteOnExit(); - FileOutputStream os = new FileOutputStream(tmplFile); - _wb.write(os); - os.close(); - - //Substitute the template entries with the generated sheet data files - injectData(tmplFile, stream); - tmplFile.delete(); + try + { + FileOutputStream os = new FileOutputStream(tmplFile); + try + { + _wb.write(os); + } + finally + { + os.close(); + } + + //Substitute the template entries with the generated sheet data files + injectData(tmplFile, stream); + } + finally + { + tmplFile.delete(); + } + } + + /** + * Dispose of temporary files backing this workbook on disk. + * Calling this method will render the workbook unusable. + * @return true if all temporary files were deleted successfully. + */ + public boolean dispose() + { + boolean success = true; + for (SXSSFSheet sheet : _sxFromXHash.keySet()) + { + success = sheet.dispose() && success; + } + return success; } /** diff --git a/src/ooxml/java/org/apache/poi/xssf/streaming/SheetDataWriter.java b/src/ooxml/java/org/apache/poi/xssf/streaming/SheetDataWriter.java index f6c98317f5..20a1d317bd 100644 --- a/src/ooxml/java/org/apache/poi/xssf/streaming/SheetDataWriter.java +++ b/src/ooxml/java/org/apache/poi/xssf/streaming/SheetDataWriter.java @@ -57,7 +57,6 @@ public class SheetDataWriter { */ public File createTempFile()throws IOException { File fd = File.createTempFile("poi-sxssf-sheet", ".xml"); - fd.deleteOnExit(); return fd; } @@ -302,4 +301,17 @@ public class SheetDataWriter { _out.write(chars, last, length - last); } } + + /** + * Deletes the temporary file that backed this sheet on disk. + * @return true if the file was deleted, false if it wasn't. + */ + boolean dispose() { + try { + _out.close(); + return _fd.delete(); + } catch (IOException e){ + return false; + } + } } diff --git a/src/ooxml/testcases/org/apache/poi/xssf/SXSSFITestDataProvider.java b/src/ooxml/testcases/org/apache/poi/xssf/SXSSFITestDataProvider.java index d5d7026b7a..a8ca27fdef 100644 --- a/src/ooxml/testcases/org/apache/poi/xssf/SXSSFITestDataProvider.java +++ b/src/ooxml/testcases/org/apache/poi/xssf/SXSSFITestDataProvider.java @@ -30,6 +30,7 @@ import java.io.ByteArrayInputStream; import java.io.ByteArrayOutputStream; import java.io.IOException; import java.io.InputStream; +import java.util.ArrayList; /** * @author Yegor Kozlov @@ -37,12 +38,16 @@ import java.io.InputStream; public final class SXSSFITestDataProvider implements ITestDataProvider { public static final SXSSFITestDataProvider instance = new SXSSFITestDataProvider(); + private ArrayList instances = new ArrayList(); + private SXSSFITestDataProvider() { // enforce singleton } public Workbook openSampleWorkbook(String sampleFileName) { XSSFWorkbook xssfWorkbook = XSSFITestDataProvider.instance.openSampleWorkbook(sampleFileName); - return new SXSSFWorkbook(xssfWorkbook); + SXSSFWorkbook swb = new SXSSFWorkbook(xssfWorkbook); + instances.add(swb); + return swb; } public Workbook writeOutAndReadBack(Workbook wb) { @@ -62,7 +67,9 @@ public final class SXSSFITestDataProvider implements ITestDataProvider { return result; } public SXSSFWorkbook createWorkbook(){ - return new SXSSFWorkbook(); + SXSSFWorkbook wb = new SXSSFWorkbook(); + instances.add(wb); + return wb; } public byte[] getTestDataFileContent(String fileName) { return POIDataSamples.getSpreadSheetInstance().readFile(fileName); @@ -73,4 +80,14 @@ public final class SXSSFITestDataProvider implements ITestDataProvider { public String getStandardFileNameExtension() { return "xlsx"; } + + public synchronized boolean cleanup(){ + boolean ok = true; + for(int i = 0; i < instances.size(); i++){ + SXSSFWorkbook wb = instances.get(i); + ok = ok && wb.dispose(); + instances.remove(i); + } + return ok; + } } diff --git a/src/ooxml/testcases/org/apache/poi/xssf/streaming/TestSXSSFCell.java b/src/ooxml/testcases/org/apache/poi/xssf/streaming/TestSXSSFCell.java index 638dbe20d1..a0668754c5 100644 --- a/src/ooxml/testcases/org/apache/poi/xssf/streaming/TestSXSSFCell.java +++ b/src/ooxml/testcases/org/apache/poi/xssf/streaming/TestSXSSFCell.java @@ -33,6 +33,12 @@ public class TestSXSSFCell extends BaseTestCell { super(SXSSFITestDataProvider.instance); } + + @Override + public void tearDown(){ + SXSSFITestDataProvider.instance.cleanup(); + } + /** * this test involves evaluation of formulas which isn't supported for SXSSF */ @@ -82,7 +88,7 @@ public class TestSXSSFCell extends BaseTestCell { Workbook xwb = new XSSFWorkbook(); Cell xCell = xwb.createSheet().createRow(0).createCell(0); - Workbook swb = new SXSSFWorkbook(); + Workbook swb = SXSSFITestDataProvider.instance.createWorkbook(); Cell sCell = swb.createSheet().createRow(0).createCell(0); StringBuffer sb = new StringBuffer(); diff --git a/src/ooxml/testcases/org/apache/poi/xssf/streaming/TestSXSSFHyperlink.java b/src/ooxml/testcases/org/apache/poi/xssf/streaming/TestSXSSFHyperlink.java index 7184e51c1f..9cad4c10cf 100644 --- a/src/ooxml/testcases/org/apache/poi/xssf/streaming/TestSXSSFHyperlink.java +++ b/src/ooxml/testcases/org/apache/poi/xssf/streaming/TestSXSSFHyperlink.java @@ -33,4 +33,10 @@ public class TestSXSSFHyperlink extends BaseTestHyperlink { super(SXSSFITestDataProvider.instance); } + + @Override + public void tearDown(){ + SXSSFITestDataProvider.instance.cleanup(); + } + } \ No newline at end of file diff --git a/src/ooxml/testcases/org/apache/poi/xssf/streaming/TestSXSSFRow.java b/src/ooxml/testcases/org/apache/poi/xssf/streaming/TestSXSSFRow.java index 1a0ff3bfa2..9533215a71 100644 --- a/src/ooxml/testcases/org/apache/poi/xssf/streaming/TestSXSSFRow.java +++ b/src/ooxml/testcases/org/apache/poi/xssf/streaming/TestSXSSFRow.java @@ -32,6 +32,12 @@ public final class TestSXSSFRow extends BaseTestRow { super(SXSSFITestDataProvider.instance); } + + @Override + public void tearDown(){ + SXSSFITestDataProvider.instance.cleanup(); + } + public void testRowBounds() { baseTestRowBounds(SpreadsheetVersion.EXCEL2007.getLastRowIndex()); } diff --git a/src/ooxml/testcases/org/apache/poi/xssf/streaming/TestSXSSFSheet.java b/src/ooxml/testcases/org/apache/poi/xssf/streaming/TestSXSSFSheet.java index f9d8fc09f2..2116a0be52 100644 --- a/src/ooxml/testcases/org/apache/poi/xssf/streaming/TestSXSSFSheet.java +++ b/src/ooxml/testcases/org/apache/poi/xssf/streaming/TestSXSSFSheet.java @@ -29,6 +29,13 @@ public class TestSXSSFSheet extends BaseTestSheet { super(SXSSFITestDataProvider.instance); } + + @Override + public void tearDown(){ + SXSSFITestDataProvider.instance.cleanup(); + } + + /** * cloning of sheets is not supported in SXSSF */ diff --git a/src/ooxml/testcases/org/apache/poi/xssf/streaming/TestSXSSFWorkbook.java b/src/ooxml/testcases/org/apache/poi/xssf/streaming/TestSXSSFWorkbook.java index ef9149cbf8..0d4a7d3c12 100644 --- a/src/ooxml/testcases/org/apache/poi/xssf/streaming/TestSXSSFWorkbook.java +++ b/src/ooxml/testcases/org/apache/poi/xssf/streaming/TestSXSSFWorkbook.java @@ -27,11 +27,17 @@ import org.apache.poi.xssf.usermodel.XSSFWorkbook; import java.io.File; public final class TestSXSSFWorkbook extends BaseTestWorkbook { + public static final SXSSFITestDataProvider _testDataProvider = SXSSFITestDataProvider.instance; - public TestSXSSFWorkbook() { - super(SXSSFITestDataProvider.instance); + public TestSXSSFWorkbook() { + super(_testDataProvider); } + @Override + public void tearDown(){ + _testDataProvider.cleanup(); + } + /** * cloning of sheets is not supported in SXSSF */ @@ -59,19 +65,23 @@ public final class TestSXSSFWorkbook extends BaseTestWorkbook { "Only XSSFCells can be evaluated.", e.getMessage()); } } - + public void testExistingWorkbook() { XSSFWorkbook xssfWorkbook = new XSSFWorkbook(); xssfWorkbook.createSheet("S1"); SXSSFWorkbook wb = new SXSSFWorkbook(xssfWorkbook); xssfWorkbook = (XSSFWorkbook) SXSSFITestDataProvider.instance.writeOutAndReadBack(wb); - wb = new SXSSFWorkbook(xssfWorkbook); + wb.dispose(); + + wb = new SXSSFWorkbook(xssfWorkbook); assertEquals(1, wb.getNumberOfSheets()); Sheet sheet = wb.getSheetAt(0); assertNotNull(sheet); assertEquals("S1", sheet.getSheetName()); + wb.dispose(); + } - + public void testAddToExistingWorkbook() { XSSFWorkbook xssfWorkbook = new XSSFWorkbook(); xssfWorkbook.createSheet("S1"); @@ -81,26 +91,28 @@ public final class TestSXSSFWorkbook extends BaseTestWorkbook { cell.setCellValue("value 2_1_1"); SXSSFWorkbook wb = new SXSSFWorkbook(xssfWorkbook); xssfWorkbook = (XSSFWorkbook) SXSSFITestDataProvider.instance.writeOutAndReadBack(wb); - wb = new SXSSFWorkbook(xssfWorkbook); - + wb.dispose(); + + wb = new SXSSFWorkbook(xssfWorkbook); + // Add a row to the existing empty sheet Sheet sheet1 = wb.getSheetAt(0); Row row1_1 = sheet1.createRow(1); Cell cell1_1_1 = row1_1.createCell(1); cell1_1_1.setCellValue("value 1_1_1"); - + // Add a row to the existing non-empty sheet Sheet sheet2 = wb.getSheetAt(1); Row row2_2 = sheet2.createRow(2); Cell cell2_2_1 = row2_2.createCell(1); cell2_2_1.setCellValue("value 2_2_1"); - + // Add a sheet with one row Sheet sheet3 = wb.createSheet("S3"); Row row3_1 = sheet3.createRow(1); Cell cell3_1_1 = row3_1.createCell(1); cell3_1_1.setCellValue("value 3_1_1"); - + xssfWorkbook = (XSSFWorkbook) SXSSFITestDataProvider.instance.writeOutAndReadBack(wb); assertEquals(3, xssfWorkbook.getNumberOfSheets()); // Verify sheet 1 @@ -145,6 +157,7 @@ public final class TestSXSSFWorkbook extends BaseTestWorkbook { File tmp = wr.getTempFile(); assertTrue(tmp.getName().startsWith("poi-sxssf-sheet")); assertTrue(tmp.getName().endsWith(".xml")); + wb.dispose(); wb = new SXSSFWorkbook(); wb.setCompressTempFiles(true); @@ -154,6 +167,7 @@ public final class TestSXSSFWorkbook extends BaseTestWorkbook { tmp = wr.getTempFile(); assertTrue(tmp.getName().startsWith("poi-sxssf-sheet-xml")); assertTrue(tmp.getName().endsWith(".gz")); + wb.dispose(); //Test escaping of Unicode control characters wb = new SXSSFWorkbook(); @@ -162,11 +176,13 @@ public final class TestSXSSFWorkbook extends BaseTestWorkbook { Cell cell = xssfWorkbook.getSheet("S1").getRow(0).getCell(0); assertEquals("value?", cell.getStringCellValue()); + wb.dispose(); + } - + public void testGZipSheetdataWriter(){ - Workbook wb = new SXSSFWorkbook(); - ((SXSSFWorkbook)wb).setCompressTempFiles(true); + SXSSFWorkbook wb = new SXSSFWorkbook(); + wb.setCompressTempFiles(true); int rowNum = 1000; int sheetNum = 5; for(int i = 0; i < sheetNum; i++){ @@ -175,7 +191,7 @@ public final class TestSXSSFWorkbook extends BaseTestWorkbook { Row row = sh.createRow(j); Cell cell1 = row.createCell(0); cell1.setCellValue(new CellReference(cell1).formatAsString()); - + Cell cell2 = row.createCell(1); cell2.setCellValue(i); @@ -184,9 +200,9 @@ public final class TestSXSSFWorkbook extends BaseTestWorkbook { } } - wb = SXSSFITestDataProvider.instance.writeOutAndReadBack(wb); + XSSFWorkbook xwb = (XSSFWorkbook)SXSSFITestDataProvider.instance.writeOutAndReadBack(wb); for(int i = 0; i < sheetNum; i++){ - Sheet sh = wb.getSheetAt(i); + Sheet sh = xwb.getSheetAt(i); assertEquals("sheet" + i, sh.getSheetName()); for(int j = 0; j < rowNum; j++){ Row row = sh.getRow(j); @@ -202,7 +218,50 @@ public final class TestSXSSFWorkbook extends BaseTestWorkbook { } } - + wb.dispose(); + } + static void assertWorkbookDispose(SXSSFWorkbook wb) + { + int rowNum = 1000; + int sheetNum = 5; + for(int i = 0; i < sheetNum; i++){ + Sheet sh = wb.createSheet("sheet" + i); + for(int j = 0; j < rowNum; j++){ + Row row = sh.createRow(j); + Cell cell1 = row.createCell(0); + cell1.setCellValue(new CellReference(cell1).formatAsString()); + + Cell cell2 = row.createCell(1); + cell2.setCellValue(i); + + Cell cell3 = row.createCell(2); + cell3.setCellValue(j); + } + } + + for (SXSSFSheet sheet : wb._sxFromXHash.keySet()) { + assertTrue(sheet.getSheetDataWriter().getTempFile().exists()); + } + + assertTrue(wb.dispose()); + + for (SXSSFSheet sheet : wb._sxFromXHash.keySet()) { + assertFalse(sheet.getSheetDataWriter().getTempFile().exists()); + } + } + + public void testWorkbookDispose() + { + SXSSFWorkbook wb = new SXSSFWorkbook(); + // the underlying writer is SheetDataWriter + assertWorkbookDispose(wb); + + wb = new SXSSFWorkbook(); + wb.setCompressTempFiles(true); + // the underlying writer is GZIPSheetDataWriter + assertWorkbookDispose(wb); + + } } -- 2.39.5