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

[Peter Levart]

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? 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...

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


Regards, Peter