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

src/documentation/content/xdocs/spreadsheet/how-to.xml

@@ -626,10 +626,6 @@ public class ExampleEventUserModel {
      </section>
      <anchor id="sxssf"/>
      <section><title>SXSSF (Streaming Usermodel API)</title>
-    <note>
-      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 <link href="../howtobuild.html">here</link>.
-    </note>
     <p>
     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.
@@ -655,6 +651,9 @@ public class ExampleEventUserModel {
     A windowSize of -1 indicates unlimited access. In this case all
     records that have not been flushed by a call to flushRows() are available
     for random access. 
+    </p>
+   <p> 
+   Note that SXSSF allocates temporary files that you <strong>must</strong> always clean up explicitly, by calling the dispose method.
    </p> 
     <p> 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,6 +737,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();
    }
 
 
@@ -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
 
 ]]></source>
-
      </section>
 
      <anchor id="low_level_api" />

src/documentation/content/xdocs/status.xml

@@ -34,6 +34,7 @@
 
     <changes>
         <release version="3.9-beta1" date="2012-??-??">
+          <action dev="poi-developers" type="fix">53780 - Fixed memory and temporary file leak in SXSSF </action>
           <action dev="poi-developers" type="fix">53380 - ArrayIndexOutOfBounds Excetion parsing word 97 document. </action>
           <action dev="poi-developers" type="fix">53434 - Subtotal is not return correct value. </action>
           <action dev="poi-developers" type="fix">53642 - [PATCH] XLS formula evaluation logging </action>

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;
     }
 

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();
+    }
 }

src/ooxml/java/org/apache/poi/xssf/streaming/SXSSFWorkbook.java

@@ -760,15 +760,41 @@ public class SXSSFWorkbook implements Workbook
     	
         //Save the template
         File tmplFile = File.createTempFile("poi-sxssf-template", ".xlsx");
-        tmplFile.deleteOnExit();
+        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;
+    }
 
     /**
      * @return the total number of defined names in this workbook

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;
+        }
+    }
 }

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<SXSSFWorkbook> instances = new ArrayList<SXSSFWorkbook>();
+
     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;
+    }
 }

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();

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();
+    }
+
 }

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());
     }

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
      */

src/ooxml/testcases/org/apache/poi/xssf/streaming/TestSXSSFWorkbook.java

@@ -27,9 +27,15 @@ 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);
+		super(_testDataProvider);
+	}
+
+    @Override
+    public void tearDown(){
+        _testDataProvider.cleanup();
     }
 
     /**
@@ -65,11 +71,15 @@ public final class TestSXSSFWorkbook extends BaseTestWorkbook {
     	xssfWorkbook.createSheet("S1");
     	SXSSFWorkbook wb = new SXSSFWorkbook(xssfWorkbook);
     	xssfWorkbook = (XSSFWorkbook) SXSSFITestDataProvider.instance.writeOutAndReadBack(wb);
+    	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() {
@@ -81,6 +91,8 @@ public final class TestSXSSFWorkbook extends BaseTestWorkbook {
     	cell.setCellValue("value 2_1_1");
     	SXSSFWorkbook wb = new SXSSFWorkbook(xssfWorkbook);
     	xssfWorkbook = (XSSFWorkbook) SXSSFITestDataProvider.instance.writeOutAndReadBack(wb);
+        wb.dispose();
+
         wb = new SXSSFWorkbook(xssfWorkbook);
 
     	// Add a row to the existing empty sheet
@@ -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 = new SXSSFWorkbook();
-        ((SXSSFWorkbook)wb).setCompressTempFiles(true);
+        wb.setCompressTempFiles(true);
         int rowNum = 1000;
         int sheetNum = 5;
         for(int i = 0; i < sheetNum; 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);
+
+    }
 }