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

[Rachel Protacio]

Thank you, David!

Rachel


On 8/17/2016 8:56 PM, David Holmes wrote:
> Hi Rachel,
>
> This looks good to me.
>
> Thanks,
> David
>
> On 18/08/2016 1:56 AM, Rachel Protacio 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
>>>
>>