JDK-8027351: (ref) Base's class finalize method not invoked if a private finalize method exists in its subclass

[Mandy Chung]

On 11/2/2013 3:41 AM, Peter Levart wrote:
>
> On 11/01/2013 10:11 PM, Mandy Chung wrote:
>> On 11/1/13 1:37 PM, mark.reinhold at oracle.com wrote:
>>> 2013/11/1 4:15 -0700, mandy.chung at 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/
>>
>> Thanks for the review.
>> Mandy
>>
>
> Hi Mandy,
>
> FWIW, I don't know if you gain anything by caching static field in a 
> static field, performance wise.It looks nicer at use site though. If 
> performance was really extremely important in this use case (which I 
> think is not) then "jla" should be a local variable inside 
> FinalizerThread.run() (and possibly in the anonymous Runnables too) 
> and passed as an argument to Finalizer.runFinalizer().
>
> Looking at code once more, I see a possible data race. Is it remotely 
> possible, that either Finalizer.runFinalization() or 
> Finalizer.runAllFinalizers() is executed and new secondary finalizer 
> thread spawned and executed before FinalizerThread manages to call 
> ensureAccessAvailable(), therefore executing the 
> Finalizer.runFinalizer() methods before static Finalizer.jla is 
> initialized? 

Finalizer.runFinalization() or Finalizer.runAllFinalizers() can only be 
called after the VM initialization completes.  The Finalizer class is 
initialized (which creates and starts the Finalizer thread) before 
System.initializeSystemClass is called.   At that point, the system 
class loader is not initialized yet. When it returns (i.e. set booted), 
it notifies the Finalizer thread.  The Finalizer thread has a higher 
priority and I don't see in practice it won't get notified to set jla 
before the secondary finalizer thread starts (which will have to wait 
the main entry point starts).


> Even though jla field was initialized, what guarantees it's value 
> being visible in spawned secondary finalizer threads? FinalizerThread 
> is started in Finalizer's static initializer, yes, but 
> ensureAccessAvailable() is called by FinalizerThread after it starts...
>

Good catch.   Perhaps to make it less confusing, the secondary finalizer 
thread could call ensureAccessAvailable (with some modification).  I'll 
revise the patch and send a new webrev out.

Mandy

> Considering all this, it might be safer to just ready the 
> SharedSecrets field every time it's needed.
>
>
> Regards, Peter
>