RFR: 8159746: (proxy) Support for default methods

[Peter Levart]

On Thu, 15 Oct 2020 02:54:06 GMT, Mandy Chung <mchung at openjdk.org> wrote:

>> Thanks for the input Peter.
>> 
>>> I'm in favour of following the bytecode (method handles) behaviour here even though it allows more than Java language
>>> allows.
>> 
>> The proposal intends to follow the bytecode behavior.   I can clarify in the javadoc.
>
> `invokeDefaultMethod` is moved to a static method in `InvocationHandler`.
>  I also add `@throws IllegalAccessException` if the specified default method is
> inaccessible to the caller (access check was missing in the previous version).
> The declaring interface of the default method must be exported to the caller to
> use.  If it's a non-public interface, the caller class must be in the same runtime
> package as the interface.  The example in javadoc includes the case when
> the proxy interface is modified.

Hi Mandy,

I'm beginning to think that we somewhat wondered away from the original intent which was to mimic the bytecode
behaviour as much as possible. Bytecode behaviour for super calls (invokespecial) is very special and what you do in
the last patch is guarding invocation to a super default method via @CallerSensitive method which checks the access of
the caller as though this was a normal call. Similar access check when a default method might reside in a
package-private class or when this class might not be exported is already performed when the code creates a Proxy
class/instance (Proxy::getProxyClass/newProxyInstance). This code also designates an InvocationHandler for the proxy
instance. From that point on, the designated invocation handler should have full access to invoking super default
methods on the proxy instance regardless of where it resides because it has been chosen as the delegate of the Proxy
class with full powers.

Since we are exposing the invokeDefaultMethod functionality in a public static method, it is hard (or not quite
possible) to check that the code doing the invocation is in fact the designated invocation handler code. One
approximation might be the following check:

`Proxy.getInvocationHandler(proxy).getClass() == callerClass`

...but that would not work for code residing in the invocation handler superclass even though such code might be
eligible to call the super default method. OTOH such check is over permissive as it allows calling the super method
from static context too.

So I was thinking about an alternative approach. To expose the method as a protected final instance method in an
abstract class implementing InvocationHandler:

public interface InvocationHandler {
  ...
  abstract class Support implements InvocationHandler {
    protected final Object invokeDefaultMethod(Object proxy, Method method, Object... args) throws
    InvocationTargetException {
      if (this != Proxy.getInvocationHandler(proxy)) throw ...
      ...
    }
  }
}

Users would have to extend `InvocationHandler.Support` to access this feature. This might be OK for most users, but
those wanting to extend some other class will not be able to do that.

So WDYT? In case you still prefer your approach of re-checking the permissions in invokeDefaultMethod, then I might
have some ideas of how to speed-up theses checks. As currently stands, invoking public exported interface default
methods does exhibit a little more penalty, but for non-public or non-exported interfaces, the penalty is bigger:

Mandy's original:

Benchmark             Mode  Cnt    Score    Error  Units
ProxyBench.implClass  avgt    5    3.709 ±  0.026  ns/op
ProxyBench.implProxy  avgt    5  926.215 ± 11.835  ns/op

Peter's performance patch:

Benchmark             Mode  Cnt   Score   Error  Units
ProxyBench.implClass  avgt    5   3.777 ± 0.005  ns/op
ProxyBench.implProxy  avgt    5  27.579 ± 0.250  ns/op

Moved to InvocationHandler + added access check

Benchmark               Mode  Cnt    Score   Error  Units
ProxyBench.implClass    avgt    5    3.740 ± 0.004  ns/op
ProxyBench.implProxy    avgt    5   34.226 ± 0.125  ns/op
ProxyBench.ppImplClass  avgt    5    3.780 ± 0.004  ns/op
ProxyBench.ppImplProxy  avgt    5  147.318 ± 1.422  ns/op

The updated benchmark code is here: https://gist.github.com/plevart/f6e0a5224c3ffbf14b28b47755599226
(the ppImpl... benchmarks are against package-private interface)

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

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