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

[David Holmes]

Hi Mandy,

I'll wait for the updated webrev.

FYI I wasn't suggesting any VM changes regarding GetMethodID. I assumed, 
incorrectly, that once you had the method ID you could query to see if 
it was private. That said if you called GetMethodID on Object.class and 
then did the CallVoidMethod(env, obj) would that actually work? (I 
hadn't realized JNI calling mechanism were so limited with regards to 
how to select the desired method.)

Thanks,
David


On 4/11/2013 2:45 PM, Mandy Chung wrote:
>
> On 11/3/2013 5:32 PM, David Holmes wrote:
>> Hi Mandy,
>>
>> On 2/11/2013 7:11 AM, 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/
>>
>> 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 ??
>>
>
> protected members can be accessed by the same package and subclasses.
> This is the implementation of JavaLangAccess in java.lang.System that is
> in the same package as java.lang.Object.
>
>> 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 sounds a good idea.  Let me think about it and get back to this.
>
>> 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.
>
> Hmm... agents calling System.runFinalization during startup - like Alan
> described, the agent is playing fire.
>
> The potential issue that could happen is that during the VM
> initialization the heap is so small that triggers GC and also the
> startup code has finalizers and those objects with finalizers are
> awaiting for finalization in order for the sufficient memory to be freed
> up.  The VM initialization couldn't get completed and the Finalizer
> thread is blocked and thus due to insufficient memory, eventually it
> would get out of memory.  An agent instrumenting classes early in the
> startup and creates lots of objects and finalizers, that might also
> cause problem.
>
> I think it's good to have the secondary finalizer thread to call
> ensureAccessAvailable (with some modification to ensure jla is
> initialized).
>
>>
>> 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.
>
> There is other issue (e.g. static method with same name/descriptor) that
> JNI_GetMethodID has to resolve.  This will be a bigger change in the VM
> that probably can't make jdk8.
>
> I think the proposed patch with slight change in the secondary finalizer
> thread is a relative safe approach (I wil revise the patch and send out
> another rev tomorrow).
>
> thanks
> Mandy