Review Request: JDK-8020801: Apply the restriction of invoking MethodHandles.lookup to j.l.r.Method.invoke

[Peter Levart]

Hi Mandy,

On 05/02/2017 11:27 PM, Mandy Chung wrote:
> Hi Peter,
>
> Looking at it again, you are right that no need to skip inflation. The 
> change
> is simplified.   I have verified with and without 
> -Dsun.reflect.noInflation=true.
>
> http://cr.openjdk.java.net/~mchung/jdk9/webrevs/8020801/webrev.01/ 
> <http://cr.openjdk.java.net/%7Emchung/jdk9/webrevs/8020801/webrev.01/>

Looks good. The following line is not needed now in ReflectionFactory 
(but is harmless):

  182         boolean noInflation = ReflectionFactory.noInflation;

Regards, Peter

>
> thanks
> Mandy
>
>> On May 2, 2017, at 1:17 PM, Peter Levart <peter.levart at gmail.com 
>> <mailto:peter.levart at gmail.com>> wrote:
>>
>>
>> On 05/02/2017 06:56 PM, Mandy Chung wrote:
>>>> On May 2, 2017, at 3:14 AM, Peter Levart<peter.levart at gmail.com>  wrote:
>>>>
>>>> I don't quite understand the need for bypassing the inflation of native into generated method accessor
>>> The VM native reflection implementation does not know about this alternate `reflected$XXX` mechanism.  No VM change in this patch and so we ensure this be called via the generated bytecode for Method::invoke rather than going through the VM native reflection which reduces startup overhead.
>>
>> I don't think VM native reflection implementation needs to know 
>> anything about this alternate `reflected$XXX` mechanism. The 
>> NativeMethodAccessorImpl is constructed with the Method argument. In 
>> case of `reflected$XXX` mechanism, it is given the alternate Method 
>> object that points to the alternate method, so 
>> NativeMethodAccessorImpl.invoke0 is called with this alternate Method 
>> object. It is like reflecting over the alternate method itself, isn't it?
>>
>> Am I missing something?
>>
>>>> Is DelegatingMethodAccessorImpl/NativeMethodAccessorImpl combo not treated correctly (i.e. skipped) by the Reflection.getCallerClass(), while generated MethodAccessorImpl subclass is?
>>> As this case is forced not to go through VM reflection support, unless I miss something, DelegatingMethodAccessorImpl/NativeMethodAccessorImpl has no need to be changed.  I will double check with the VM runtime team.
>>
>> I was asking because I suspected that this might be the reason for 
>> skipping DelegatingMethodAccessorImpl/NativeMethodAccessorImpl. But 
>> if it is not the reason (and anyway it would be a bug because other 
>> @CallerSensitive methods would behave erratically if this was the 
>> case), then I still don't see a reason for skipping 
>> DelegatingMethodAccessorImpl/NativeMethodAccessorImpl and proceeding 
>> directly with generated method accessor.
>>
>> Regards, Peter
>>
>>> Mandy
>>>
>>
>