RFR(XS): 8163973: VM Anonymous classes should not call Class File Load Hooks

[Rachel Protacio]

Thanks, Karen!

Rachel


On 8/19/2016 11:56 AM, Karen Kinnear wrote:
> Rachel -
>
> Looks good. Thank you for changing the name.
>
> David - I had asked Rachel  to explain where VM anonymous classes came from since I do not see the term
> “VM anonymous” class anywhere. No problem taking that comment out. I’ll ask Harold to add it to the
> Unsafe.DefineAnonymousClass code as part of a separate bug fix.
>
> thanks,
> Karen
>
>> On Aug 17, 2016, at 11:56 AM, Rachel Protacio <rachel.protacio at oracle.com> wrote:
>>
>> Hi David and Coleen,
>>
>> Thank you for the reviews. I've updated the change as requested: http://cr.openjdk.java.net/~rprotacio/8163973.01/
>>
>> Rachel
>>
>> On 8/16/2016 10:04 PM, Coleen Phillimore wrote:
>>>
>>> On 8/15/16 11:36 PM, David Holmes wrote:
>>>> Hi Rachel,
>>>>
>>>> On 16/08/2016 6:48 AM, Rachel Protacio wrote:
>>>>> Hello,
>>>>>
>>>>> Please review this change, which makes sure class file load hooks are
>>>>> not called for VM anonymous classes. See
>>>>> http://mail.openjdk.java.net/pipermail/core-libs-dev/2016-January/038353.html
>>>>> for justification.
>>>>>
>>>>> Passes JPRT and RBT.
>>>>>
>>>>> Bug: https://bugs.openjdk.java.net/browse/JDK-8163973
>>>>> Open webrev: http://cr.openjdk.java.net/~rprotacio/8163973.00/
>>>> This:
>>>>
>>>> 112   // VM Anonymous classes - defined via unsafe.DefineAnonymousClass - should not
>>>> 113   // call back to a CFLH
>>>> 114   if (host_klass == NULL) {
>>>> 115     stream = prologue(stream,
>>>>
>>>> suggests that "prologue" can only do CFLH related things. If that is true then it would be much clearer in my opinion if prologue were renamed to something more explicit - like check_class_file_load_hook ? Otherwise, the host_klass should be passed in to prologue and the anonymous class check internalized there.
>>> I agree with David here.  This was sort of bothering me about your change when we talked about it before.   If the prologue did more than call the CFLH then you'd have to pass host_klass down, since we know that's all it does, the name of the function should be changed.   David's name looks good to me.
>>>> Also I don't think you need to explain where VM anonymous classes come from, it suffices to simply say "Skip class file load hook processing for VM anonymous classes"; or if the prologue is renamed then simply "Skip this processing for VM anonymous classes". :)
>>> Thanks,
>>> Coleen
>>>> Thanks,
>>>> David
>>>>
>>>>> Thank you!
>>>>> Rachel