Some Eclipse Foundation services are deprecated, or will be soon. Please ensure you've read this important communication.

Bug 313225

Summary:	Don't over use finalize()
Product:	[WebTools] WTP Java EE Tools	Reporter:	Gary Karasiuk <karasiuk>
Component:	jst.j2ee	Assignee:	Jason Sholl <jsholl>
Status:	RESOLVED WORKSFORME	QA Contact:	Chuck Bridgham <cbridgha>
Severity:	normal
Priority:	P3	CC:	stryker
Version:	3.2
Target Milestone:	---
Hardware:	PC
OS:	Windows XP
Whiteboard:

Description Gary Karasiuk

2010-05-17 15:16:03 EDT

Using the finalize() method can slow down GC and cause other memory problems. It should only be used as a last resort. 

Please remove the finalize() method from
org.eclipse.jst.jee.archive.internal.ArchiveImpl

Writing something to stderr does not seem like a strong enough reason to have this method.

Comment 1 Gary Karasiuk

2010-05-17 15:22:19 EDT

I just noticed there is a close() in the finalize methods as well, so it is not quite a bad as I first thought.

Please review if this is still needed, because relying on finalize to close your archive is not recommended.

Comment 2 Rob Stryker

2010-05-18 04:15:49 EDT

seems like something Jason would know about

Comment 3 Jason Sholl

2010-05-18 09:52:34 EDT

This is working as designed.  The finalize() method was put in place in order to help facilitate proper usage of the IArchive API.  Too often developers using the API forget to follow this pattern and fail to close the archive.

IArchive archive
try{
  archive = //open the archive
} finally{
  if(archive != null) archive.close();
}

This leads to open file handles which then require the entire workbench to be brought down in order to release the file handles.  The finalize() method was put in place to do two things; 1. warn the developers with the exception to stderr that they forgot to close the archive, and 2. close the archive if it was orphaned.  These precautions have helped track down many bugs, and unless there is another ensure archives are closed and developers are sternly reminded to close them, the finalize() method should remain.

Comment 4 Gary Karasiuk

2010-05-18 11:00:52 EDT

(In reply to comment #3)
Why does it write to syserr instead of a log?