The Source for Java Technology Collaboration
User: Password:



Tom Ball

Tom Ball's Blog

Finally, a Good Use for Finalizers

Posted by tball on August 17, 2005 at 03:46 AM | Comments (12)

For years Java developers have been warned about the dangers of using finalize methods to release system resources. Josh Block describes the issues thoroughly in his book, Effective Java (Item 6: Avoid finalizers), but just Google for "avoid finalizers" or "finalizers considered harmful" to find hundreds of similar discussions not just about Java, but most other languages that offer the facility. Yet I just found out that the problem cropped up again in a recent internal Mustang build. What is it that is so alluring about this bad programming practice that it can blind even very experienced Java developers with its false charms?

Finalizers bit me hard back in the JDK pre-1.0 days, because they were originally used to free the Windows resources from AWT graphic objects. This technique gave Tumbling Duke the power to cause the Blue Screen of Death, but such power is cannot be entrusted to a mere applet, or for any class for that matter. Any class that allocates a critical resource needs an explicit "close", "release" or similarly named method in its API. Its clients must then always use that method.

The problem is that a library provider cannot always regulate its use, or misuse, by developers who use that library. The java.io documentation, for example, can warn, argue, even plead that developers close file streams after using them, but it cannot enforce that requirement. So what a lot of library engineers do is add a fallback routine in a finalize method to check whether a resource has been released, and if not, release it during finalization. It's an ugly hack, but often necessary to avoid granting Tumbling Duke-like powers to their client programs.

But cleaning up after one's messy clients doesn't encourage good behavior on their part. I found this to be true while on vacation this summer with our children, who didn't understand why food wrappers cannot just be thrown on our hotel floor since a maid will pick them up later. "Because you have to" may work for making children clean up after themselves (it didn't for me, but maybe for other parents), but such an approach is guaranteed not to work for developers. As long as the maid (or library) silently cleans up, messes will be left to clean.

But what if the maid left a nasty note whenever such a mess was left? Something like (using java.util.Logger):


protected void finalize() throws Throwable {
   if (handle != null) {
       Logger.global.log(Level.WARNING, "my handle not released"); // new
       release();
   }
}
Now when a client tests his poorly written code which seemed to work because the library was silently cleaning up after it, several nasty-grams start appearing. Even better, add a stack trace which will point to where in the client code is the problem:

private Throwable trace;
...
   // during handle allocation
   callerTrace = new Throwable("handle allocation");
...
protected void finalize() throws Throwable {
   if (handle != null) {
       Logger.global.log(Level.WARNING, "my handle not released", trace); // new       
       release();
   }
}
This takes advantage of how scary stack traces are to clients, plus it forwards all the information a library engineer has to the offending developer. Finally, we have a way finalizers can improve quality instead of detracting from it, by throwing up warning flags when library resources are being leaked.

Bookmark blog post: del.icio.us del.icio.us Digg Digg DZone DZone Furl Furl Reddit Reddit
Comments
Comments are listed in date ascending order (oldest first) | Post Comment

  • OK, but having a finalizer keeps an extra reference to the class which can have an effect on an app that struggles to get good GC performance.

    PhantomReferences are also good for pre-mortem cleanup - as the javadoc for tha class suggests.

    I think if you're going to have a "nasty-gram" though, it should be something like:
    assert handle == null : "the client developer is an idiot"

    :-)

    Posted by: goron on August 17, 2005 at 05:57 AM

  • I've heard that finalizers slow down GC a great deal, enough to make it better off for most people to stop using them altogether.

    Posted by: keithkml on August 17, 2005 at 07:17 AM

  • That is what I've heard, too. I didn't mean to suggest that people add new finalize methods to classes that don't have them, but only that library owners who have existing finalizers to catch leaks from bad client code make it very obvious when that such cleanup is needed. Unfortunately, if you have a class that allocates system resources but doesn't control its lifecycle (like java.io.FileInputStream), then you should already have a finalizer to minimize leaks from bad client code (like FileInputStream has to). Most classes don't need to do this, however.

    Posted by: tball on August 17, 2005 at 08:01 AM

  • This deals with the situation where a library supplies a resource to a client who then uses it and is then is supposed to close or other wise do the right thing to release it.

    As I library designer I think a better design is to avoid this idiom completely, basically because you can't trust the client.
    Therefore say to the client, give me the code that wants to use the resource, and I'll allocate, run your code, passit the resource, and I'll clean up afterwards.

    java.security.AccessController.doPriviledged() does this, and is where I first got the idea.

    I have used this idiom often where I don't trust the client, and need to carefully allocate and clean up afterwards. Its my library that has this requirement to clean up, not the client - so why force it on them when I can look after myself.

    In the case where you are also writing the client, the code is simpler because the try / finally block (you do have one don't you?) is in the library, not in each piece of client code.

    Now if java had a nicer way of passing around blocks of code (closures etc) rather than anonymous inner classes, this would be even nicer, but anonymous inner classes to pass a Runnable (or similar) to such a library method are no more onerous than a try / finally, AND at least the compiler can enforce the requirements (pass me a Runnable) whereas it can't enforce them for the aquire-use-release idiom (clean up afteryourself - always - inside a finally block.)

    Posted by: brucechapman on August 17, 2005 at 07:12 PM

  • A question not directly about this :
    Using a jtable with a model that i create. When i click a button the call the model who make an sql request and put in a obj vector my data.
    When i push in the seccond time i call the method removeAllElements(). Is it sufficient to use this or i must implement a finalise method in my obj containing data from sql request?

    Posted by: wowo on August 18, 2005 at 12:19 AM

  • Now if java had a nicer way of passing around blocks of code...

    Java might not, but AspectJ5 does. The around() advice, along with java5 annotations, could accomplish this. Tag a method with some kind of @UsesResourceX annotation then use around() advice to wrap any calls to that method. The advice could make sure the resource is allocated and deallocated correctly.

    Posted by: gbarton on August 18, 2005 at 11:10 AM

  • I don't understand, as I've been passing blocks of code around since before the language was called Java. One simply defines a method interface, implement it in one or more classes, and pass references of those classes to a method which invokes the method. Think about your first applet, which probably created an ImageObserver delegate that implemented imageUpdate(), or java.lang.Comparable with its single compareTo() method. One doesn't need closures or even innerclasses to support this programming model.

    Posted by: tball on August 20, 2005 at 03:42 PM

  • I think the most important part of your idiom described above is the presentation of a helpful diagnostic message by recording the stack trace at the point to resource is created rather than just throwing a useless stack trace from the point the problem has been discovered - the fact that finalizes are involved is secondary.

    I have used a similar approach for debugging deadlocks in a fairly complex web application. By implementing a simple DataSource&Connection proxy that, for each query, records the stack trace where the query was executed and also the DB's internal ProcessID for that query I was able to get extremely good visibility into the interactions that were leading up to the deadlock. It was simple to cross reference between the DB's lock table and the actual code which was executing the SQL causing the locks.

    Posted by: oliverhutchison on August 23, 2005 at 06:02 PM

  • I really like the idea of remebering the stack of the caller that allocated the handle (and then forgot to free it). But beware that can kill performance on certain platforms. It's fast on Windows, Linux and Solaris, but its very slow on AIX (at least it was a year or two ago).

    Posted by: kimbomundy on August 23, 2005 at 06:26 PM

  • I don't understand...

    that's possibly because my comment said the idiom would benefit from a nicer way, not that it "needed a nicer way". Yes I have been doing it for ages as well, also since before anonymous inner classes, (but not before it was called java :).

    By nicer, I meant I would like to see a reduction in the "baggage" for simple cases. One aspect of that baggage (which most commentators don't mention) is the 2 extra levels of indent, expected for correctness, over and above the 1 required to reflect that the code is inside the method call. The other baggage is well known - tho' I'm not convinced what, if any, is the right way to get rid of it.

    Posted by: brucechapman on August 29, 2005 at 08:29 PM

  • give me the code that wants to use the resource, and I'll allocate, run your code, passit the resource, and I'll clean up afterwards

    Bruce Chapman gave us a good modern device that I've seen gain support in a number of applications -- Inversion of Control. I dare say this is a whole chapter that should be added to any book that writes about the Visitor design pattern.

    Posted by: kpauls on October 28, 2005 at 03:49 PM

  • Nice idea if there is a finalizer anyway.
    Instead of holding the whole Stacktrace, I use the direct calling Class, Method, Line for information; see below:

    protected String callerInfo = "Unknown";
    // ...
    protected void initializeCallerInfo()
    {
    // get info about calling Class
    final StackTraceElement[] stackTraceElements = new Throwable().getStackTrace();
    final String myPackage = getClass().getPackage().getName();
    for (int i = 0; i < stackTraceElements.length; i++)
    {
    StackTraceElement element = stackTraceElements[i];
    // get 1st element not contained in com.xxx.yyy.* package
    if (!(element.getClassName().toLowerCase().startsWith(myPackage)))
    {
    callerInfo = element.toString();
    log.debug("Caller:\t{0}", callerInfo);
    break;
    }
    }
    }

    Posted by: ricon on November 18, 2005 at 07:56 AM





Powered by
Movable Type 3.01D
 Feed java.net RSS Feeds