On Wed, 28 Oct 2020 18:01:44 GMT, Rémi Forax 
<github.com+828220+fo...@openjdk.org> wrote:

>>> Yes, but it is not really useful there. The super handler is captured by 
>>> the user invocation handler created in the invocation handler factory 
>>> function and it is only used by the user invocation handler. All we need in 
>>> the proxy instance is a boolean flag indicating the way the invocation 
>>> handler was created (which one of the two overloaded newProxyInstance 
>>> methods was called).
>> 
>> Either way provides the distinction if this proxy class supports default 
>> method invocation.   I chose to store the superHandler rather than a boolean 
>> that may allow us to do white-box testing (an alternative to the trick in 
>> getting the superHandler in spinning another proxy instance).   It's a 
>> trivial change  if we prefer to store a boolean field but keep the 
>> constructor to take `superHandler` parameter.  I'm fine with it.
>> 
>>> To be precise, we need to pass CCE and NPE thrown by the default method 
>>> unchanged, but we need to wrap CCE and NPE thrown by the method handle 
>>> transformation chain with IllegalArgumentException. Is that what you are 
>>> referring to? 
>> 
>> Yes and it's implementation details.  For the specification side, I agree 
>> that `InvocationHandler::invoke` throws the exception by the method and no 
>> need to wrap it with `InvocationTargetException` .
>> 
>>> "Do we want to wrap CCE and NPE thrown by the method handle transformation 
>>> chain with IllegalArgumentException?".
>> 
>> I did consider that and propose to go with IAE for simplicity and 
>> consistency with the core reflection (`Method::invoke`, `Field::set` etc) 
>> when there is an illegal argument.   Is it worth specifying `CCE` and  `NPE` 
>> explicitly when an argument fails the conversion?   Maybe but average users 
>> of `java.lang.reflect` may not use `java.lang.invoke` and `IAE` is thrown 
>> for illegal argument to the method invocation.   When invoking a default 
>> method, `Method` object representing a default method is passed to 
>> `superHandler` and it's intuitive to consider the behavior as if 
>> `Method::invoke` is called (note that the specification does not 
>> mandate/expect to use `Lookup::findSpecial` and `MethodHandle::invoke*`).   
>> While CCE and NPE provide slightly clearer reason of the illegal argument, 
>> as you said, they are subtle variants of the same thing indicating the 
>> parameters are wrong.  This only impacts the performance of the exception 
>> cases which should not be a big concern.    This explains why 
 I am in favor to throw IAE for illegal arguments which is the expected 
behavior for average core reflection users.  
>> 
>>> What to do about that. Is UncheckedReflectiveOperationException a suitable 
>>> solution? 
>> 
>> I do see your point of proposing `UncheckedReflectiveOperationException` 
>> which I thought about too and was tempted.  I'm uncertain yet if we want a  
>> unchecked  exception type for any reflective operation exceptions.     
>> Should future new `java.lang.reflect` and `java.lang.invoke` APIs use the 
>> unchecked exception?  This is a broader question to answer than one specific 
>> exception type `java.lang.reflect.InaccessibleInvocationHandlerException`??
>> 
>>> I'm not particularly fond of creating another Proxy.invocationHandler 
>>> method, because it does not solve the problem of the old method that may 
>>> also be called.
>> 
>> Me neither.
>
> Hi Mandy and Peter,
> reading your exchanges, i wonder if it's not better to stop trying to add 
> more and more features to the already clunky java.lang.reflect.Proxy but 
> instead to move to provide the same set of feature but using an 
> implementation based on java.lang.invoke.Lookup/MethodHandle.
> 
> I propose to introduce a java.lang.invoke.Proxy has a replacement of 
> java.lang.reflect.Proxy using j.l.i.MethodHandle instead of j.l.r.Method, the 
> equivalent of the InvocationHandler acting as a linker called once per the 
> first time a method of the Proxy is called instead of being called each time 
> a method is called.
> 
> Such proxy
> - has it's security model based on Lookup to define the class,
>   so no supplementary restrictions on package/module of the interface 
> implemented
> - avoid to mix java.lang.reflect API and java.invoke API
> - is transparent in term of exceptions (ther are propagated without any 
> wrapping/unwrapping)
> - can support default methods natively
> - doesn't requires any caching (the user will be responsible to do the 
> caching)
> - is more efficient, actually as efficient as the equivalent written code or 
> a little more (thanks to @ForceInline)

Hi Remi,

I appreciate your proposal to modernize Proxy API.   There are several requests 
for this enhancement to support default methods in Proxy.   Defining a new 
`java.lang.invoke.Proxy` is a much bigger project that I can't tell when the 
existing users of `java.lang.reflect.Proxy` will be able to get this default 
method invocation support.  

I do agree that this API design has many challenges caused by what you listed 
above.   Well, I believe we are very close to have a consensus:
1. New `newProxyInstance` factory method takes a handler factory doing the 
access check
2. Update `getInvocationHandler` to throw 
`InaccessibleInvocationHandlerException` if access denied to get an invocation 
handler associated with the proxy instance

If this needs more time, I think we can consider to shelf this RFE and come 
back to it later (and consider your proposal as well).

-------------

PR: https://git.openjdk.java.net/jdk/pull/313

Reply via email to