Java Style Guide: What should be said about finalize()?

[Andrew Grieve]

We have 12 hits for "finalize()" in non-third_party files.

The google style guide:

https://google.github.io/styleguide/javaguide.html#s6.4-finalizers

6.4 Finalizers: not usedIt is extremely rare to override Object.finalize.
Tip: Don't do it. If you absolutely must, first read and understand Effective Java Item 7, "Avoid Finalizers," very carefully, and then don't do it.

The guide does not explain *why* you shouldn't use them.

Given that:

1. finalize() on Android *is* guaranteed to be called (eventually), and

2. It's really convenient to call nativeDelete(mNativePointer) in them,

I think they "why not" needs to be spelled out.

There's a good discussion on this NativeRefAssassin review, where it sounds like the main concern is that native code could have strong references back to the object, and thus the objects would never be GC'ed.  

Are there other reasons to avoid this?

Also - the guide does not offer a replacement solution. I'm guessing the only alternative is explicit destroy() methods?

[Boris Sazonov]

Finalizers also negatively impact performance. Here's an excerpt from Effective Java 3rd edition:

There is a severe performance penalty for using finalizers and cleaners. On my machine, the time to create a simple AutoCloseable object, to close it using try -with-resources, and to have the garbage collector reclaim it is about 12 ns. Using a finalizer instead increases the time to 550 ns. In other words, it is about 50 times slower to create and destroy objects with finalizers. This is primarily because finalizers inhibit efficient garbage collection. Cleaners are comparable in speed to finalizers if you use them to clean all instances of the class (about 500 ns per instance on my machine), but cleaners are much faster if you use them only as a safety net, as discussed below. Under these circumstances, creating, cleaning, and destroying an object takes about 66 ns on my machine, which means you pay a factor of five (not fifty) for the insurance of a safety net if you don’t use it.

I don't know about exact numbers on Android, though.

--
You received this message because you are subscribed to the Google Groups "java" group.
To unsubscribe from this group and stop receiving emails from it, send an email to java+uns...@chromium.org.
To post to this group, send email to ja...@chromium.org.
To view this discussion on the web visit https://groups.google.com/a/chromium.org/d/msgid/java/CABiQX1XGUqFBsZOz-dBAOjWiwM7b%3DyCCrntaWRfcq9p0%2BBczSg%40mail.gmail.com.

[Ted Choc]

+bo...@chromium.org as they've dealt more with finalizers than anyone else.

Here's an old (Google only doc -- sorry) where Bo outlined problems with it:

https://docs.google.com/presentation/d/1ZBXbs-hPx2Hc2PYbonorA9Zw_DGgS5e7mAFals3XV2c/edit#slide=id.p

These are the high level notes:

* guaranteed to be only called once per object

* called on background thread (on android)

* exceptions are ignored

* should call super

* timeout after 10 seconds (on android)

* causes more work for garbage collector

I've seen very few cases where finalizers are worth it.  Chrome has a pretty clear destruction path on Android (triggered from onDestroy of the activity), so an explicit destruction path seems preferable across the board IMO.

- Ted

To view this discussion on the web visit https://groups.google.com/a/chromium.org/d/msgid/java/CAPtdB4nF5HFrj6RPzXO8%2BQ3Nzfj-M5U%3DLUo9ZihNWvrzWOQd%2BA%40mail.gmail.com.

[Bo Liu]

I can give some more concrete pitfalls with finalizers.

* On android, finalize is called on a background thread. Usually chrome objects are owned and destroyed on a specific thread, so delete needs to be posted to another thread.

* On android, finalize timeout after 10 seconds, which crashes the process I believe.

* There is no guaranteed call order when a bunch of objects become finalizable together. So won't work if native objects require a specific destruction order.

* Exceptions are ignored. On android, they print but do not crash, so there is no point in just putting an assert mNativeFoo == 0 in a finalizer, and need something more complex to be useful.

* Should not assume finalize is the last method called on an object. It's allowed to call methods after finalize. This generally isn't a problem for apps, but code that implements a library like content and android webview should assume the worst.

* Finalizers are not chained (unlike constructor). Again, more of a problem for library that has classes that allow client to subclass.

The better alternative to finalizer is using ReferenceQueue, which is what NativeRefAssassin uses. None of the problems above really apply to ReferenceQueue, but it has its own costs.

However neither solves the problem of java-native object ownership. Ownership is complex and difficult to get right, and does not lend itself to be abstracted away with finalizer/ReferenceQueue. I have examples of complexity encountered by webview in this doc. So I'd say understand your object ownership, and then use explicit destroy if it's needed.

[Andrew Grieve]

Finally getting back to this. Here's a proposed addition:

https://chromium-review.googlesource.com/c/chromium/src/+/1344889

### Finalizers
In line with [Google's Java style guide](https://google.github.io/styleguide/javaguide.html#s6.4-finalizers),
never override `Object.finalize()`.
Custom finalizers:
* are called on a background thread, and at an unpredicatble point in time,
* swallow all exceptions (asserts won't work),
* causes additional garbage collector jank.
Classes that need destructor logic should provide an explicit `destroy()`
method.

I tried to keep the bullets focused on those that argue why not use them, as opposed to those that point out the nuances of using them correctly.