RFR JDK-8232222: Set state to 'linked' when an archived class is restored at runtime

[Ioi Lam]

On 6/2/20 10:16 PM, David Holmes wrote:
> Hi Ioi,
>
> On 3/06/2020 2:55 pm, Ioi Lam wrote:
>>
>>
>> On 5/27/20 11:13 PM, David Holmes wrote:
>>> Hi Jiangli,
>>>
>>> On 28/05/2020 11:35 am, Ioi Lam wrote:
>>>>
>>>>
>>>>>>
>>>>> I was going to take the suggestion, but realized that it would add
>>>>> unnecessary complications for archived boot classes with class
>>>>> pre-initialization support. Some agents may set
>>>>> JvmtiExport::should_post_class_prepare(). It's worthwhile to support
>>>>> class pre-init uniformly for archived boot classes with
>>>>> JvmtiExport::should_post_class_prepare() enabled or disabled.
>>>>
>>>> This would introduce behavioral changes when JVMTI is enabled:
>>>>
>>>> + The order of JvmtiExport::post_class_prepare is different than 
>>>> before
>>>> + JvmtiExport::post_class_prepare may be called for a class that 
>>>> was not called before (if the class is never linked during run time)
>>>> + JvmtiExport::post_class_prepare was called inside the init_lock, 
>>>> now it's called outside of the init_lock
>>>
>>> I have to say I share Ioi's concerns here. This change will impact 
>>> JVM TI agents in a way we can't be sure of. From a specification 
>>> perspective I think we are fine as linking can be lazy or eager, so 
>>> there's no implied order either. But this would be a behavioural 
>>> change that will be observable by agents. (I'm less concerned about 
>>> the init_lock situation as it seems potentially buggy to me to call 
>>> out to an agent with the init_lock held in the first place! I find 
>>> it hard to imagine an agent only working correctly if the init_lock 
>>> is held.)
>>
>> David,
>>
>> The init_lock has a serializing effect. The callback for a subclass 
>> will not be executed until the callback for its super class has been 
>> finished.
>
> Sorry I don't see that is the case. The init_lock for the subclass is 
> distinct from the init_lock of the superclass, and linking of 
> subclasses and superclasses is independent.


In InstanceKlass::link_class_impl, you first link all of your super classes.

If another thread is already linking your super class, you will block on 
that superclass's init_lock.

Of course, I may be wrong and my analysis may be bogus. But I hope you 
can appreciate that this is not going to be a trivial change to analyze.

Thanks
- Ioi
>
> David
> -----
>
>> With the proposed patch, the callback for both the super class and 
>> subclass can proceed in parallel. So if an agent performs class 
>> hierarchy analysis, for example, it may need to perform extra 
>> synchronization.
>>
>> This is just one example that I can think of. I am sure there are 
>> other issues that we have not thought about.
>>
>> The fact is we are dealing with arbitrary code in the callbacks, and 
>> we are changing the conditions of how they are called. The calls 
>> happen inside very delicate code (class loading, system dictionary). 
>> I am reluctant to do the due diligence, which is substantial, of 
>> verifying that this is a safe change, unless we have a really 
>> compelling reason to do so.
>>
>> Thanks
>> - Ioi
>>
>>