CODEWITHSUNDEEP
javaapache-poi
manutd
manutd

Reputation: 291

FileInputStream not closed

I have the below code to set an Excel file:

public static void setExcelFile(String Path) throws Exception {
    try {
        FileInputStream ExcelFile = new FileInputStream(Path);
        ExcelWBook = new XSSFWorkbook(ExcelFile);
    } catch (Exception e){
        Log.error("Class Utils | Method setExcelFile | Exception desc : "+e.getMessage());
    }
}

This will be called from a loop in another class. This loop will be repeated for each Excel file in a location. Do I need to close the FileInputStream every time for each Excel file? If I am not closing at the end of each Excel file. Will it have an effect on memory utilization? Or everytime when the new Filestream object is created for next Excel file will it close the previous one automatically and create for current file? I faced an issue with following error message.

Exception in thread "main" java.lang.reflect.InvocationTargetException
  at sun.reflect.NativeMethodAccessorImpl.invoke0(Native Method)
  at sun.reflect.NativeMethodAccessorImpl.invoke(Unknown Source)
  at sun.reflect.DelegatingMethodAccessorImpl.invoke(Unknown Source)
  at java.lang.reflect.Method.invoke(Unknown Source)
  at org.eclipse.jdt.internal.jarinjarloader.JarRsrcLoader.main(JarRsrcLoader.java:58)
Caused by: java.lang.OutOfMemoryError: Java heap space
  at org.apache.xmlbeans.impl.store.Cur.createElementXobj(Cur.java:260)
  at org.apache.xmlbeans.impl.store.Cur$CurLoadContext.startElement(Cur.java:2997)
  at org.apache.xmlbeans.impl.store.Locale$SaxHandler.startElement(Locale.java:3211)
  at org.apache.xmlbeans.impl.piccolo.xml.Piccolo.reportStartTag(Piccolo.java:1082)
  at org.apache.xmlbeans.impl.piccolo.xml.PiccoloLexer.parseAttributesNS(PiccoloLexer.java:1822)
  at org.apache.xmlbeans.impl.piccolo.xml.PiccoloLexer.parseOpenTagNS(PiccoloLexer.java:1521)
  at org.apache.xmlbeans.impl.piccolo.xml.PiccoloLexer.parseTagNS(PiccoloLexer.java:1362)
  at org.apache.xmlbeans.impl.piccolo.xml.PiccoloLexer.yylex(PiccoloLexer.java:4682)
  at org.apache.xmlbeans.impl.piccolo.xml.Piccolo.yylex(Piccolo.java:1290)
  at org.apache.xmlbeans.impl.piccolo.xml.Piccolo.yyparse(Piccolo.java:1400)
  at org.apache.xmlbeans.impl.piccolo.xml.Piccolo.parse(Piccolo.java:714)
  at org.apache.xmlbeans.impl.store.Locale$SaxLoader.load(Locale.java:3479)
  at org.apache.xmlbeans.impl.store.Locale.parseToXmlObject(Locale.java:1277)
  at org.apache.xmlbeans.impl.store.Locale.parseToXmlObject(Locale.java:1264)
  at org.apache.xmlbeans.impl.schema.SchemaTypeLoaderBase.parse(SchemaTypeLoaderBase.java:345)
  at org.openxmlformats.schemas.spreadsheetml.x2006.main.WorksheetDocument$Factory.parse(Unknown Source)
  at org.apache.poi.xssf.usermodel.XSSFSheet.read(XSSFSheet.java:194)
  at org.apache.poi.xssf.usermodel.XSSFSheet.onDocumentRead(XSSFSheet.java:186)
  at org.apache.poi.xssf.usermodel.XSSFWorkbook.onDocumentRead(XSSFWorkbook.java:354)
  at org.apache.poi.POIXMLDocument.load(POIXMLDocument.java:166)
  at org.apache.poi.xssf.usermodel.XSSFWorkbook.<init>(XSSFWorkbook.java:263)
  at utility...

Upvotes: 0

Views: 2318

Answers (4)

mfulton26
mfulton26

Reputation: 31264

From XSSFWorkbook(InputStream):

Constructs a XSSFWorkbook object, by buffering the whole stream into memory and then opening an OPCPackage object for it.

Using an InputStream requires more memory than using a File, so if a File is available then you should instead do something like

       OPCPackage pkg = OPCPackage.open(path);
       XSSFWorkbook wb = new XSSFWorkbook(pkg);
       // work with the wb object
       ......
       pkg.close(); // gracefully closes the underlying zip file

As you have a path string you should use XSSFWorkbook(File) or simply XSSFWorkbook(String).

As far as closing resources: Always close streams. From Java Practices -> Recovering resources:

Expensive resources should be reclaimed as soon as possible, by an explict call to a clean-up method defined for this purpose. If this is not done, then system performance can degrade. In the worst cases, the system can even fail entirely.

Resources include:

  • input-output streams
  • database result sets, statements, and connections
  • threads
  • graphic resources
  • sockets

Upvotes: 2

mtj
mtj

Reputation: 3554

As others already told you: yes, you always should close the streams. Additionally, each unclosed and non-finalized stream will hold a file handle on some systems, and you might run into a total maximum value per process. (This depends on your OS.)

Probably, the easiest thing in your context above is to auto-close by using a try-with-resources construct:

try (
   FileInputStream ExcelFile = new FileInputStream(Path);
) {
  ExcelWBook = new XSSFWorkbook(ExcelFile);
} catch (Exception e) {
  Log.error("Class Utils | Method setExcelFile | Exception desc : "+ e.getMessage());
}

Upvotes: 0

Gaurav
Gaurav

Reputation: 821

If you do not close the stream, file will be locked until inputstream has been closed or JVM is shutdown. You should be closing stream otherwise you may run into IOException during next time reading that file from java or directly using windows

Upvotes: 3

Debosmit Ray
Debosmit Ray

Reputation: 5413

You should add ExcelFile.close() [ref] in your catch block and after you are done using the resource. This is done to prevent memory leaks, and in your case, exceptions.

Upvotes: 2

Related Questions