RFR: 8153749 - New capability can_generate_early_class_hook_events

[serguei.spitsyn at oracle.com]

On 5/24/16 17:24, Daniel D. Daugherty wrote:
> On 5/24/16 6:03 PM, serguei.spitsyn at oracle.com wrote:
>> On 5/24/16 16:23, Daniel D. Daugherty wrote:
>>> > Testing:
>>> >    Altered the nsk.jvmti co-located test 
>>> nsk/jvmti/ClassFileLoadHook/classfloadhk002
>>> >    to enable the can_generate_early_class_hook_events and checked 
>>> that new CFLH events
>>> >    are posted in the primordial phase and also they are not posted 
>>> otherwise.
>>>
>>> Sorry for looping back on this...
>>>
>>> I've been re-reading this thread off and on for a while now...
>>>
>>> I'm okay with this part of the testing statement:
>>>
>>> > checked that new CFLH events are posted in the primordial phase
>>>
>>> but this part bothers me:
>>>
>>> > and also they are not posted otherwise
>>>
>>> I'm not sure what that last part means exactly. I think it might
>>> be saying that there are certain CFLH events that the test expects
>>> to be posted in the primordial phase and the test verifies that
>>> those CFLH events are posted in the primordial phase and not in
>>> the next phase (the start phase). I'm hoping that the testable
>>> assertion is more clear than the above text.
>>
>> Dan, sorry that my statement above is not clear.
>> The 'otherwise' means the same test but unaltered.
>> Unaltered test does not enable new capability and so, the CFLH events
>> should not be posted in the primordial phase.
>
> Got it. I'm good with a test to verify that the older JVM/TI
> behavior works as expected along with a test for the newer
> behavior.
>
>
>>
>>>
>>> One other possible issue: if the primordial phase is still
>>> single threaded, then there is no race between a thread doing
>>> this early class loading (and posting of the CFLH events) and
>>> the thread that is changing the JVM/TI phase from "primordial
>>> phase" to "start phase". However, if the primordial phase is
>>> now multi-threaded, then there might be a race between the
>>> thread posting the CFLH event and JVM/TI phase switch...
>>
>> This is interesting observation, thanks.
>> Why do you think the primordial thread is multi-threaded now?
>
> I haven't been paying close attention to Jigsaw so I'm not sure
> if primordial phase is multi-threaded or not...
I can be wrong but ...
My current understanding is that it is still single-threaded
as I did not see any discussions or related changes in the code.

>
>
>> We have this kind of race problem in general, especially
>> with the JVM/TI phase switch to the DEAD phase.
>> The phase can be switched even in the middle of the event post.
>> I'm currently looking at several bugs related to 
>> JVMTI_ERROR_WRONG_PHASE(112).
>> This problem needs a good solution, at least to make the nightly 
>> testing stable.
>
> Yes, I'm well aware of that problem. I believe I've made a number
> of comments on various JVMTI_ERROR_WRONG_PHASE bugs over the years.
> It's going to need some sort of handshake to solve it. Much like
> the suspend/resume dance that we do on thread exit...
>
> We'll have to brainstorm on how to make things more robust...

Great, thanks!
I've already started thinking on this. :)


Thanks,
Serguei

>
> Dan
>
>
>>
>>
>> Thanks,
>> Serguei
>>
>>
>>>
>>> Dan
>>>
>>>
>>> On 4/14/16 2:24 AM, serguei.spitsyn at oracle.com wrote:
>>>> Please, review the Jigsaw-related fix for:
>>>> https://bugs.openjdk.java.net/browse/JDK-8153749
>>>>
>>>>
>>>> Hotspot webrev:
>>>> http://cr.openjdk.java.net/~sspitsyn/webrevs/2016/hotspot/8153749-Jigsaw-newcap.hs1/ 
>>>>
>>>>
>>>> Jdk webrev:
>>>> http://cr.openjdk.java.net/~sspitsyn/webrevs/2016/jdk/8153749-Jigsaw-newcap.jdk1/ 
>>>>
>>>>
>>>>
>>>> Summary:
>>>>
>>>>    This is a Jigsaw related enhancement.
>>>>    Some agents need to get a CFLH event for classes loaded in the 
>>>> primordial phase.
>>>>    This is not possible in JDK 9 because existing agents may 
>>>> instrument code in the
>>>>    primordial or start phase before the module system has completed 
>>>> initialization.
>>>>
>>>>    We introduce a new capability: 
>>>> can_generate_early_class_hook_events.
>>>>    If this capability and can_generate_all_class_hook_events are 
>>>> enabled then
>>>>    the CFLH event could be posted for classes loaded in the 
>>>> primordial phase.
>>>>    We leave can_generate_early_vmstart as is, no changes.
>>>>
>>>>    This enhancement needs a CCC request filed.
>>>>    I will file it once the JVMTI spec changes are reviewed.
>>>>
>>>>
>>>> Testing:
>>>>    Altered the nsk.jvmti co-located test 
>>>> nsk/jvmti/ClassFileLoadHook/classfloadhk002
>>>>    to enable the can_generate_early_class_hook_events and checked 
>>>> that new CFLH events
>>>>    are posted in the primordial phase and also they are not posted 
>>>> otherwise.
>>>>
>>>>
>>>> Thanks,
>>>> Serguei
>>>>
>>>
>>
>