Bug 59166 - util TempFile could cause memory leak
Summary: util TempFile could cause memory leak
Status: RESOLVED FIXED
Alias: None
Product: POI
Classification: Unclassified
Component: POI Overall (show other bugs)
Version: 3.13-FINAL
Hardware: PC All
: P2 enhancement (vote)
Target Milestone: ---
Assignee: POI Developers List
URL:
Keywords:
Depends on: 59788
Blocks:
  Show dependency tree
 
Reported: 2016-03-11 16:32 UTC by kong.shijun
Modified: 2016-07-04 01:13 UTC (History)
0 users



Attachments

Note You need to log in before you can comment on or make changes to this bug.
Description kong.shijun 2016-03-11 16:32:52 UTC
Currently implementation will register not only temp directory, but also individual files to java.io.DeleteOnExitHook via java.io.File#deleteOnExit

For some long running applications which generate tons of files via POI, it will create huge garbage to the hook, even the actual temp files have already been deleted. 

Today, there is only one system property flag "poi.keep.tmp.files". If turn off, both temp directory and temp files won't be deleted on exit. Can we always ignore files, and only register temp directory?
Comment 1 Dominik Stadler 2016-03-11 22:38:09 UTC
Would you be able to put together a patch together with some unit-tests that would address and verify this?
Comment 2 Dominik Stadler 2016-03-12 15:27:36 UTC
I am setting this to Enhancement for now as you can already implement your own TempFileCreationStrategy, which allows to do this differently if this is actually a problem for your application.
Comment 3 Javen O'Neal 2016-06-17 09:53:39 UTC
None of the POI unit tests run long enough to experience the problems you're experiencing. While we could register just the directory for deletion on JVM exit, this strategy seems dangerous if the directory was not empty before starting or a separate application wrote data to the directory. Under these conditions, data loss could occur.

Another strategy for long-running processes is to keep a fixed-length queue of files and delete the files FIFO as the queue overflows.
This would mean not registering deleteOnExit with the JVM, so you would need to intercept a JVM exit message to delete the files remaining in the queue before the JVM shut down.

You could also implement a strategy that deletes all temp files that have been created. This is essentially the same as above using a variable-length set that never implicitly deletes files.

I don't think any of these strategies are safe enough to implement without getting into trouble: either leaving temp files behind or accidentally deleting files that shouldn't have been deleted. If you have a better TempFileCreationStrategy that would appeal to many users, feel free to re-open this bug with the corresponding patch.
Comment 4 Javen O'Neal 2016-07-04 01:13:08 UTC
With the changes from bug 59788, the strategy described in Comment 0 could be accomplished by:

File tempDir = DefaultTempFileCreationStrategy.createTempDirectory("registeredDirectory");
for (String f : filesToAdd) {
    createSomeFile(new File(tempDir, f));
}

I updated the documentation in r1751190 to make the purpose of DefaultTempFileCreationStrategy clearer and suggestions for other TempFileCreationStrategy's that may be useful outside of the POI project.