[#JAMES-585] SMTP Server may leak temporary ($[JAMES}/temp/*.m64) files

JAMES Server

SMTP Server may leak temporary ($[JAMES}/temp/*.m64) files

Created: 02/Aug/06 05:30 PM Updated: 21/Nov/07 08:31 AM

Return to search

Component/s:

SMTPServer

Affects Version/s:

2.3.0

Fix Version/s:

2.3.0

Time Tracking:

Not Specified

Resolution Date:

04/Aug/06 01:05 PM

Description

« Hide

If there is an exception during message processing, before the message is posted to the spooler, the temporary files may not be cleaned up. This appears to be due to an unintended and implied reliance on finalize() to dispose the temporary files.

All

Comments

Work Log

Change History

Subversion Commits

Sort Order:

Repository	Revision	Date	User	Message
ASF	#428097	Wed Aug 02 19:18:42 UTC 2006	noel	Fix for ~~JAMES-585~~. Explicit cleanup for temporary files when exceptions occur.
				Files Changed
				MODIFY /james/server/trunk/src/java/org/apache/james/smtpserver/core/DataCmdHandler.java MODIFY /james/server/branches/v2.3/src/java/org/apache/james/smtpserver/DataCmdHandler.java MODIFY /james/server/branches/v2.3/src/java/org/apache/james/smtpserver/SMTPHandler.java MODIFY /james/server/trunk/src/java/org/apache/james/smtpserver/SMTPHandler.java

[ Permlink | « Hide ]

Noel J. Bergman added a comment - 02/Aug/06 07:18 PM

Should be resolved by r428097.

An alternative would have been to incorporate the FileCleaner from Commons I/O, which is used for the same purpose in Commons FileUpload.

Noel J. Bergman made changes - 02/Aug/06 07:18 PM

Field	Original Value	New Value
Status	Open [ 1 ]	Resolved [ 5 ]
Resolution		Fixed [ 1 ]

[ Permlink | « Hide ]

Noel J. Bergman added a comment - 02/Aug/06 08:03 PM

I'm still seeing it ... need to find out why.

Noel J. Bergman made changes - 02/Aug/06 08:03 PM

Status	Resolved [ 5 ]	Reopened [ 4 ]
Resolution	Fixed [ 1 ]

Noel J. Bergman made changes - 02/Aug/06 08:04 PM

Assignee

Noel J. Bergman [ noel ]

[ Permlink | « Hide ]

Noel J. Bergman added a comment - 02/Aug/06 11:11 PM

Current status:

1) After applying r428097, even though some files appear to leak, they are deleted when the JVM terminates.
2) A private build, using a copy of org.apache.commons.io.FileCleaner, does not appear to leak files at all during runtime.

I will continue to test to see if the FileCleaner solution, which was written for just this purpose, resolves the issue.

[ Permlink | « Hide ]

Noel J. Bergman added a comment - 02/Aug/06 11:39 PM

Stefano had another though, involving changing

MimeMessageInputStreamSource.getInputStream from:
return new SharedFileInputStream(file);
to
return new BufferedInputStream(new FileInputStream(file));

Repository	Revision	Date	User	Message
ASF	#428228	Thu Aug 03 04:23:48 UTC 2006	noel	~~JAMES-585~~. Copy FileCleaner from Commons I/O, to help track and cleanup temporary files.
				Files Changed
				ADD /james/server/branches/v2.3/src/java/org/apache/james/util/io/FileCleaner.java (from /jakarta/commons/proper/io/trunk/src/java/org/apache/commons/io/FileCleaner.java)

Repository	Revision	Date	User	Message
ASF	#428237	Thu Aug 03 04:53:49 UTC 2006	noel	~~JAMES-585~~. This change consists of the following changes: 1) RemoteManagerHander.java Added two UNDOCUMENTED commands: 1) SHOWTRACKERS - Call the FileCleaner to display a list of files that it is tracking to delete. 2) GC - Force a JVM garbage collection, which will normally cause the FileCleaner to delete files whose marker object (MimeMessageInputStreamSource in our case) is not currently reachable. 2) MimeMessageInputStreamSource.java Three changes: 1) Register all temporary files with FileCleaner. 2) At Stefano's suggestion, revert change to use SharedFileInputStream, 3) Comment out finalize(), which is replaced by using FileCleaner. The mere presence of the finalize() method can actually cause problems. 3) FileCleaner Two changes from what I copied from Commons I/O: 1) Add some instrumentation to let us inspect the state of the tracker collection. 2) Print (to System.err, normally redirected to ${JAMES}/temp/phoenix.console) an error if we cannot delete the file when asked. In both cases, a stack trace is used to let us see how the file was allocated, just to make sure that we're not missing something about the behavior. So far, with these changes I am no longer seeing any leaks. A few times, there were some files still around, but forcing a GC cleaned them up. The instrumentation was added to help investigate this behavior.
				Files Changed
				MODIFY /james/server/branches/v2.3/src/java/org/apache/james/util/io/FileCleaner.java MODIFY /james/server/branches/v2.3/src/java/org/apache/james/remotemanager/RemoteManagerHandler.java MODIFY /james/server/branches/v2.3/src/java/org/apache/james/core/MimeMessageInputStreamSource.java

Repository	Revision	Date	User	Message
ASF	#428404	Thu Aug 03 13:55:57 UTC 2006	noel	~~JAMES-585~~. Remove the temporary file immediately if there is an IOException creating it, rather than cleaning up later.
				Files Changed
				MODIFY /james/server/branches/v2.3/src/java/org/apache/james/core/MimeMessageInputStreamSource.java

Repository	Revision	Date	User	Message
ASF	#428480	Thu Aug 03 19:09:21 UTC 2006	noel	1) Revert RemoteManagerHandler.java to RC1 state. This should be identical to r382444 for this file. It removes the temporary debugging commands SHOWTRACKERS and GC. 2) Remove the use of FileCleaner from, and restore the use of SharedFileInputStream to, MimeMessageInputStreamSource, but keep the recent improvements in exception handling, and keep finalize() commented out. 3) Remove FileCleaner.java entirely. See ~~JAMES-585~~
				Files Changed
				DEL /james/server/branches/v2.3/src/java/org/apache/james/util/io/FileCleaner.java MODIFY /james/server/branches/v2.3/src/java/org/apache/james/remotemanager/RemoteManagerHandler.java MODIFY /james/server/branches/v2.3/src/java/org/apache/james/core/MimeMessageInputStreamSource.java

[ Permlink | « Hide ]

Noel J. Bergman added a comment - 03/Aug/06 11:55 PM

NOW I am closing this issue. :-) The changes in r428404 appear to have done the trick, and r428480 backs out some of the earlier changes.

Noel J. Bergman made changes - 03/Aug/06 11:55 PM

Status	Reopened [ 4 ]	Resolved [ 5 ]
Resolution		Fixed [ 1 ]

[ Permlink | « Hide ]

Noel J. Bergman added a comment - 03/Aug/06 11:55 PM

Resolve *and* close.

Noel J. Bergman made changes - 03/Aug/06 11:55 PM

Status

Resolved [ 5 ]

Closed [ 6 ]

[ Permlink | « Hide ]

Norman Maurer added a comment - 04/Aug/06 10:44 AM

This bug is still not fixed. Stefano has a idea.. so i assign it to him ;-)

Norman Maurer made changes - 04/Aug/06 10:44 AM

Status	Closed [ 6 ]	Reopened [ 4 ]
Resolution	Fixed [ 1 ]
Assignee	Noel J. Bergman [ noel ]	Stefano Bagnara [ bago ]

[ Permlink | « Hide ]

Stefano Bagnara added a comment - 04/Aug/06 01:05 PM

Norman, you reopened the wrong bug ;-)
~~JAMES-584~~ is the one still being reproducible.

Stefano Bagnara made changes - 04/Aug/06 01:05 PM

Resolution		Fixed [ 1 ]
Status	Reopened [ 4 ]	Resolved [ 5 ]
Assignee	Stefano Bagnara [ bago ]	Noel J. Bergman [ noel ]

[ Permlink | « Hide ]

Danny Angus added a comment - 21/Nov/07 08:31 AM

Closing issue fixed in released version.

Danny Angus made changes - 21/Nov/07 08:31 AM

Status

Resolved [ 5 ]

Closed [ 6 ]