Hi Mandy,

On 2/11/2013 7:11 AM, Mandy Chung wrote:
On 11/1/13 1:37 PM, mark.reinh...@oracle.com wrote:
2013/11/1 4:15 -0700, mandy.ch...@oracle.com:
http://cr.openjdk.java.net/~mchung/jdk8/webrevs/8027351/webrev.00/
Looks good.

Just one question: In Finalizer.java, at line 97 you look up the
JavaLangAccess object every single time.  Is it worth caching that
earlier, maybe when the finalize thread starts, since it will never
change?

I was expecting that would get optimized during runtime and it's a
simple getter method. It's a good suggestion to cache it at the finalize
thread start time and here is the revised webrev:

http://cr.openjdk.java.net/~mchung/jdk8/webrevs/8027351/webrev.01/

I'm missing something basic - how did you get this to compile:

   public void invokeFinalize(Object o) throws Throwable {
     o.finalize();
   }

given finalize is protected ??

Also VM.awaitBooted seems inherently risky as a general method as you would have to make sure that it is never called by the main VM initialization thread. Perhaps handle this in sun.misc.SharedSecrets.getJavaLangAccess so it is less 'general'? That said I think Peter may be right that there could be races with agents triggerring explicit finalization requests early in the VM initialization process - which means any blocking operation dependent on other parts of the initialization sequence could be problematic.

Overall I think a safer approach may be to fix the native JNI code so that if it gets a private version of finalize() it looks up the method in the superclass.

David


Thanks for the review.
Mandy

Reply via email to