Bug 56609 - Resource leak in WorkbookFactory.create()
Summary: Resource leak in WorkbookFactory.create()
Status: RESOLVED DUPLICATE of bug 56537
Alias: None
Product: POI
Classification: Unclassified
Component: POI Overall (show other bugs)
Version: unspecified
Hardware: PC Linux
: P2 normal (vote)
Target Milestone: ---
Assignee: POI Developers List
URL:
Keywords:
Depends on:
Blocks:
 
Reported: 2014-06-09 17:46 UTC by Dzmitry Lazerka
Modified: 2014-07-02 18:03 UTC (History)
0 users



Attachments

Note You need to log in before you can comment on or make changes to this bug.
Description Dzmitry Lazerka 2014-06-09 17:46:36 UTC
WorkbookFactory.create() does not close the filesystem it opens, so after some time server JVM crashes with "Too many open files" error.

I've read bug 56537, but disagree the proposed workaround -- to copy-paste create() source code, because it's not evident that it leaks resource (until it goes to production). There's no warnings in create() javadoc about the leak, so there's no reason for user to not trust it does the right thing.

I think it's better to not have the method at all, to force user to care about closing file handles (before the buggy code is pushed to production servers, not after). In the way it is currently, it's certainly not industry-reliable: it not only stops creating Workbook-s, it crashes the whole JVM with all the services in the JVM.

Or, to make it to close file handles automatically in any way you prefer.
Comment 1 Nick Burch 2014-06-09 18:37:01 UTC
The JavaDocs already have a note on this, see http://poi.apache.org/apidocs/org/apache/poi/ss/usermodel/WorkbookFactory.html#create%28java.io.File%29 - "Note that for Workbooks opened this way, it is not possible to explicitly close the underlying File resource."

I believe that these files should be cleaned up during garbage collection, assuming you don't have references to them, so for most people it won't be a problem

If someone is able to come up with a new pattern which maintains the simplicity of WorkbookFactory and the low memory footprint of using a File (vs a Stream), and also allows for explicit closing, then we'll happily apply that!
Comment 2 Dominik Stadler 2014-07-02 15:03:02 UTC
As part of Bug 56537 we have added interface Closeable to the Workbook class which means resources can be freed by properly closing the object after use. Javadoc will be updated as well to reflect this. 

So I think this is effectively a duplicate now.

*** This bug has been marked as a duplicate of bug 56537 ***
Comment 3 Dzmitry Lazerka 2014-07-02 18:03:00 UTC
Thanks for the JavaDoc.
Nick Burch, the files are not cleaned up during garbage collection, because they are not Java objects -- these are lower level system resources (unix file handles). Once a process goes over the limit of open file handles, OS prevents it from opening new ones. For JVM that means crash. We found out that on production :)
That's why I was advocating removing the method at all -- because it crashes the whole system, not only POI-related features.