Use IOUtils.closeQuietly() in more places

Avoid two possible file-handle leaks when opening files fails with an exception
Make tests close resources properly to not spam the output when running with file-leak-detector

git-svn-id: https://svn.apache.org/repos/asf/poi/trunk@1763485 13f79535-47bb-0310-9956-ffa450edef68

diff --git a/src/ooxml/java/org/apache/poi/openxml4j/opc/OPCPackage.java b/src/ooxml/java/org/apache/poi/openxml4j/opc/OPCPackage.java
index b7720ee42a2a258d73d52ef36e4f82ea643f9ffc..759cfdea9fccc9ab9ad05d2462b37a68ae3a3956 100644 (file)
--- a/src/ooxml/java/org/apache/poi/openxml4j/opc/OPCPackage.java
+++ b/src/ooxml/java/org/apache/poi/openxml4j/opc/OPCPackage.java
@@ -54,6 +54,7 @@ import org.apache.poi.openxml4j.opc.internal.unmarshallers.PackagePropertiesUnma
 import org.apache.poi.openxml4j.opc.internal.unmarshallers.UnmarshallContext;
 import org.apache.poi.openxml4j.util.Nullable;
 import org.apache.poi.openxml4j.util.ZipEntrySource;
+import org.apache.poi.util.IOUtils;
 import org.apache.poi.util.NotImplemented;
 import org.apache.poi.util.POILogFactory;
 import org.apache.poi.util.POILogger;
@@ -221,18 +222,10 @@ public abstract class OPCPackage implements RelationshipSource, Closeable {
            // pack.originalPackagePath = file.getAbsolutePath();
            return pack;
        } catch (InvalidFormatException e) {
-           try {
-               pack.close();
-           } catch (IOException e1) {
-               throw new IllegalStateException(e);
-           }
+                  IOUtils.closeQuietly(pack);
            throw e;
        } catch (RuntimeException e) {
-           try {
-               pack.close();
-           } catch (IOException e1) {
-               throw new IllegalStateException(e);
-           }
+                  IOUtils.closeQuietly(pack);
            throw e;
        }
    }
@@ -277,6 +270,7 @@ public abstract class OPCPackage implements RelationshipSource, Closeable {
                                }
                        }
                }
+
                pack.originalPackagePath = new File(path).getAbsolutePath();
                return pack;
        }
@@ -310,18 +304,10 @@ public abstract class OPCPackage implements RelationshipSource, Closeable {
                   pack.originalPackagePath = file.getAbsolutePath();
                   return pack;
           } catch (InvalidFormatException e) {
-                  try {
-                          pack.close();
-                  } catch (IOException e1) {
-                          throw new IllegalStateException(e);
-                  }
+                  IOUtils.closeQuietly(pack);
                   throw e;
           } catch (RuntimeException e) {
-                  try {
-                          pack.close();
-                  } catch (IOException e1) {
-                          throw new IllegalStateException(e);
-                  }
+                  IOUtils.closeQuietly(pack);
                   throw e;
           }
    }
@@ -340,8 +326,16 @@ public abstract class OPCPackage implements RelationshipSource, Closeable {
        public static OPCPackage open(InputStream in) throws InvalidFormatException,
                        IOException {
                OPCPackage pack = new ZipPackage(in, PackageAccess.READ_WRITE);
-               if (pack.partList == null) {
-                       pack.getParts();
+               try {
+                       if (pack.partList == null) {
+                               pack.getParts();
+                       }
+               } catch (InvalidFormatException e) {
+                       IOUtils.closeQuietly(pack);
+                       throw e;
+               } catch (RuntimeException e) {
+                       IOUtils.closeQuietly(pack);
+                       throw e;
                }
                return pack;
        }
@@ -357,13 +351,11 @@ public abstract class OPCPackage implements RelationshipSource, Closeable {
         *             Throws if the specified file exist and is not valid.
         */
        public static OPCPackage openOrCreate(File file) throws InvalidFormatException {
-               OPCPackage retPackage = null;
                if (file.exists()) {
-                       retPackage = open(file.getAbsolutePath());
+                       return open(file.getAbsolutePath());
                } else {
-                       retPackage = create(file);
+                       return create(file);
                }
-               return retPackage;
        }
 
        /**

diff --git a/src/scratchpad/testcases/org/apache/poi/hwpf/usermodel/TestHWPFWrite.java b/src/scratchpad/testcases/org/apache/poi/hwpf/usermodel/TestHWPFWrite.java
index a45b031da2507d235775e931415fba85b8d88f1a..f829c10cc6d7d3a39749548445e37d6adb214931 100644 (file)
--- a/src/scratchpad/testcases/org/apache/poi/hwpf/usermodel/TestHWPFWrite.java
+++ b/src/scratchpad/testcases/org/apache/poi/hwpf/usermodel/TestHWPFWrite.java
@@ -17,11 +17,7 @@
 
 package org.apache.poi.hwpf.usermodel;
 
-import java.io.ByteArrayInputStream;
-import java.io.ByteArrayOutputStream;
-import java.io.File;
-import java.io.FileInputStream;
-import java.io.FileOutputStream;
+import java.io.*;
 
 import org.apache.poi.POIDataSamples;
 import org.apache.poi.hwpf.HWPFDocument;
@@ -90,10 +86,17 @@ public final class TestHWPFWrite extends HWPFTestCase {
    public void testInPlaceWrite() throws Exception {
        // Setup as a copy of a known-good file
        final File file = TempFile.createTempFile("TestDocument", ".doc");
-       IOUtils.copy(
-               POIDataSamples.getDocumentInstance().openResourceAsStream("SampleDoc.doc"),
-               new FileOutputStream(file)
-       );
+       InputStream inputStream = POIDataSamples.getDocumentInstance().openResourceAsStream("SampleDoc.doc");
+       try {
+           FileOutputStream outputStream = new FileOutputStream(file);
+           try {
+               IOUtils.copy(inputStream, outputStream);
+           } finally {
+               outputStream.close();
+           }
+       } finally {
+           inputStream.close();
+       }
 
        // Open from the temp file in read-write mode
        HWPFDocument doc = new HWPFDocument(new NPOIFSFileSystem(file, false).getRoot());
@@ -108,7 +111,9 @@ public final class TestHWPFWrite extends HWPFTestCase {
        doc.close();
 
        doc = new HWPFDocument(new NPOIFSFileSystem(file).getRoot());
+       r = doc.getRange();
        assertEquals("X XX a test document\r", r.getParagraph(0).text());
+       doc.close();
    }
 
    @SuppressWarnings("resource")
@@ -121,7 +126,10 @@ public final class TestHWPFWrite extends HWPFTestCase {
        try {
            doc.write();
            fail("Shouldn't work for InputStream");
-       } catch (IllegalStateException e) {}
+       } catch (IllegalStateException e) {
+           // expected here
+       }
+       doc.close();
 
        // Can't work for OPOIFS
        OPOIFSFileSystem ofs = new OPOIFSFileSystem(
@@ -130,7 +138,10 @@ public final class TestHWPFWrite extends HWPFTestCase {
        try {
            doc.write();
            fail("Shouldn't work for OPOIFSFileSystem");
-       } catch (IllegalStateException e) {}
+       } catch (IllegalStateException e) {
+           // expected here
+       }
+       doc.close();
 
        // Can't work for Read-Only files
        NPOIFSFileSystem fs = new NPOIFSFileSystem(
@@ -139,6 +150,9 @@ public final class TestHWPFWrite extends HWPFTestCase {
        try {
            doc.write();
            fail("Shouldn't work for Read Only");
-       } catch (IllegalStateException e) {}
+       } catch (IllegalStateException e) {
+           // expected here
+       }
+       doc.close();
    }
 }

diff --git a/src/testcases/org/apache/poi/hssf/usermodel/TestHSSFWorkbook.java b/src/testcases/org/apache/poi/hssf/usermodel/TestHSSFWorkbook.java
index 6eb4700161f494385188eace1f581cc3e5828b2c..07fd7910f826c5bb3276c50d2e8c8631a74bbb0d 100644 (file)
--- a/src/testcases/org/apache/poi/hssf/usermodel/TestHSSFWorkbook.java
+++ b/src/testcases/org/apache/poi/hssf/usermodel/TestHSSFWorkbook.java
@@ -1222,7 +1222,10 @@ public final class TestHSSFWorkbook extends BaseTestWorkbook {
         try {
             wb.write();
             fail("Shouldn't work for new files");
-        } catch (IllegalStateException e) {}
+        } catch (IllegalStateException e) {
+            // expected here
+        }
+        wb.close();
         
         // Can't work for InputStream opened files
         wb = new HSSFWorkbook(
@@ -1230,7 +1233,10 @@ public final class TestHSSFWorkbook extends BaseTestWorkbook {
         try {
             wb.write();
             fail("Shouldn't work for InputStream");
-        } catch (IllegalStateException e) {}
+        } catch (IllegalStateException e) {
+            // expected here
+        }
+        wb.close();
         
         // Can't work for OPOIFS
         OPOIFSFileSystem ofs = new OPOIFSFileSystem(
@@ -1239,7 +1245,10 @@ public final class TestHSSFWorkbook extends BaseTestWorkbook {
         try {
             wb.write();
             fail("Shouldn't work for OPOIFSFileSystem");
-        } catch (IllegalStateException e) {}
+        } catch (IllegalStateException e) {
+            // expected here
+        }
+        wb.close();
         
         // Can't work for Read-Only files
         NPOIFSFileSystem fs = new NPOIFSFileSystem(
@@ -1248,17 +1257,27 @@ public final class TestHSSFWorkbook extends BaseTestWorkbook {
         try {
             wb.write();
             fail("Shouldn't work for Read Only");
-        } catch (IllegalStateException e) {}
+        } catch (IllegalStateException e) {
+            // expected here
+        }
+        wb.close();
     }
     
     @Test
     public void inPlaceWrite() throws Exception {
         // Setup as a copy of a known-good file
         final File file = TempFile.createTempFile("TestHSSFWorkbook", ".xls");
-        IOUtils.copy(
-                POIDataSamples.getSpreadSheetInstance().openResourceAsStream("SampleSS.xls"),
-                new FileOutputStream(file)
-        );
+        InputStream inputStream = POIDataSamples.getSpreadSheetInstance().openResourceAsStream("SampleSS.xls");
+        try {
+            FileOutputStream outputStream = new FileOutputStream(file);
+            try {
+                IOUtils.copy(inputStream, outputStream);
+            } finally {
+                outputStream.close();
+            }
+        } finally {
+            inputStream.close();
+        }
         
         // Open from the temp file in read-write mode
         HSSFWorkbook wb = new HSSFWorkbook(new NPOIFSFileSystem(file, false));
@@ -1276,6 +1295,8 @@ public final class TestHSSFWorkbook extends BaseTestWorkbook {
         wb = new HSSFWorkbook(new NPOIFSFileSystem(file));
         assertEquals(1, wb.getNumberOfSheets());
         assertEquals("Changed!", wb.getSheetAt(0).getRow(0).getCell(0).toString());
+
+        wb.close();
     }
     
     @Test

diff --git a/src/testcases/org/apache/poi/poifs/filesystem/TestNPOIFSFileSystem.java b/src/testcases/org/apache/poi/poifs/filesystem/TestNPOIFSFileSystem.java
index 1833b184f35010ae688a4ab0f9cda28ca533c3e4..f4a18a03d28194004f584502d5583d705c2cb4f9 100644 (file)
--- a/src/testcases/org/apache/poi/poifs/filesystem/TestNPOIFSFileSystem.java
+++ b/src/testcases/org/apache/poi/poifs/filesystem/TestNPOIFSFileSystem.java
@@ -24,11 +24,7 @@ import static org.junit.Assert.assertNotNull;
 import static org.junit.Assert.assertThat;
 import static org.junit.Assert.fail;
 
-import java.io.ByteArrayInputStream;
-import java.io.ByteArrayOutputStream;
-import java.io.File;
-import java.io.FileOutputStream;
-import java.io.IOException;
+import java.io.*;
 import java.nio.ByteBuffer;
 import java.util.Iterator;
 
@@ -96,20 +92,25 @@ public final class TestNPOIFSFileSystem {
        HeaderBlock header = new HeaderBlock(new ByteArrayInputStream(baos.toByteArray()));
        return header;
    }
-   
-   protected static NPOIFSFileSystem writeOutAndReadBack(NPOIFSFileSystem original) throws IOException {
-       ByteArrayOutputStream baos = new ByteArrayOutputStream();
-       original.writeFilesystem(baos);
-       original.close();
-       return new NPOIFSFileSystem(new ByteArrayInputStream(baos.toByteArray()));
-   }
-   protected static NPOIFSFileSystem writeOutFileAndReadBack(NPOIFSFileSystem original) throws IOException {
-       final File file = TempFile.createTempFile("TestPOIFS", ".ole2");
-       final FileOutputStream fout = new FileOutputStream(file);
-       original.writeFilesystem(fout);
-       original.close();
-       return new NPOIFSFileSystem(file, false);
-   }
+
+    protected static NPOIFSFileSystem writeOutAndReadBack(NPOIFSFileSystem original) throws IOException {
+        ByteArrayOutputStream baos = new ByteArrayOutputStream();
+        original.writeFilesystem(baos);
+        original.close();
+        return new NPOIFSFileSystem(new ByteArrayInputStream(baos.toByteArray()));
+    }
+
+    protected static NPOIFSFileSystem writeOutFileAndReadBack(NPOIFSFileSystem original) throws IOException {
+        final File file = TempFile.createTempFile("TestPOIFS", ".ole2");
+        final OutputStream fout = new FileOutputStream(file);
+        try {
+            original.writeFilesystem(fout);
+        } finally {
+            fout.close();
+        }
+        original.close();
+        return new NPOIFSFileSystem(file, false);
+    }
    
    @Test
    public void basicOpen() throws Exception {

diff --git a/src/testcases/org/apache/poi/ss/usermodel/BaseTestCell.java b/src/testcases/org/apache/poi/ss/usermodel/BaseTestCell.java
index a19cf6684bc81eabd35d525d1ed1121aba01c90b..be097d3482e0d1d001bd3a0262a947645c509596 100644 (file)
--- a/src/testcases/org/apache/poi/ss/usermodel/BaseTestCell.java
+++ b/src/testcases/org/apache/poi/ss/usermodel/BaseTestCell.java
@@ -1017,7 +1017,7 @@ public abstract class BaseTestCell {
     }
 
     @Test
-    public void primitiveToEnumReplacementDoesNotBreakBackwardsCompatibility() {
+    public void primitiveToEnumReplacementDoesNotBreakBackwardsCompatibility() throws IOException {
         // bug 59836
         // until we have changes POI from working on primitives (int) to enums,
         // we should make sure we minimize backwards compatibility breakages.
@@ -1046,5 +1046,7 @@ public abstract class BaseTestCell {
             default:
                 fail("unexpected cell type: " + cell.getCellType());
         }
+
+        wb.close();
     }
 }