native memory leak results in container crash

• Key: FILEUPLOAD-95
• Type: Improvement
• Status: Closed
• Resolution: Fixed
• Priority: Minor
• Assignee: Unassigned
• Reporter: Forrest Girouard
• Affects Version/s: 1.1 Final

Environment

Operating System: All
Platform: All

Description

Struts File Upload uses DiskFileUpload which in turn uses DefaultFileItem whose getTempFile() method
calls File.deleteOnExit() which is known to leak memory in the native heap. With our web app running
in WebSphere on AIX it crashes (coredumps) within hours under a constant load. IBM is unable to fix
the problem as it is in the JVM code they license from Sun and they are not allowed to modify. This
means it exists in all JVMs. Please see http://
developer.java.sun.com/developer/bugParade/bugs/4513817.html

Noel Bergman is playing around with using PhantomReferences in order to avoid this problem.

Clearly File.deleteOnExit() must be avoided but I'm not sure that using the PhantomReferences isn't
going to have it's own problems. For example, won't temporary files be leaked with the JVM is stopped
and restarted?

Comments

[Forrest Girouard]

Created an attachment (id=10689)
o.a.commons.fileupload.DefaultFileItm used by o.a.struts.upload package.

[Forrest Girouard]

Created an attachment (id=10690)
o.a.commons.fileupload.DeferredFileOutputStream used by o.a.struts.upload package.

[Forrest Girouard]

Created an attachment (id=10691)
Archive containing ServerFileItem.java, ServerFileItemFactory.java and ServerFileMultipartRequestHandler.java.

[Forrest Girouard]

I just attached to patch files. One for org.apache.commons.fileupload.DefaultFileItem.java and the
other for org.apache.commons.fileupload.DeferredFileOutputStream.java. The changes to
DefaultFileItem enable subclassing while the change to DeferredFileOutputStream fixes what appears to
be a bug according to the existing javadocs.

I also attached a Java archive containing three new classes: ServerFileItem, ServerFileItemFactory and
ServerFileMultipartRequestHandler. The first two classes simply sublcass DefaultFileItem and
DefaultFileItemFactory, respectively. They do this to remove the call to File.deleteOnExit() which should
never be called from a long running server (and hence the name ServerFile). The third class uses the
first two. I've stress tested all these changes and they appear to alleviate the massive memory leak in
the native heap caused by the File.deleteOnExit() method. The disadvantage is that file space will be
leaked instead but that can be pragmatically addressed through the use of a cron job or some such.

A better solution IMO is to make use a mechanism that "manages" temporary file space (FileManager).
For example, ManagedFileItem, ManagedFileItemFactory, and ManagedFileMultipartRequestHandler are
the names of classes that I've created to make use of such a file manager (internally developed and not
publicly available at this time). To get this working it was also necessary to create a
DeferredManagedFileOutputStream that takes a FileManager reference in the contructor and only calls
FileManager.createManagedFile(...) from within the thresholdReached() method.

[Noel J. Bergman]

• • • COM-1192 has been marked as a duplicate of this bug. ***

[Martin Cooper]

• • • *** Bug 27777 has been marked as a duplicate of this bug. *** has been marked as a duplicate of this bug. ***

[Martin Cooper]

Fixed in the 20041012 nightly build.

Instead of using File.deleteOnExit(), we now use the FileCleaner class from
Commons IO, which will delete the file when its associated FileItem is garbage
collected.

[Niall Pemberton]

• • • COM-2432 has been marked as a duplicate of this bug. ***