On Fri, 26 May 2023 18:39:53 GMT, Mandy Chung <mch...@openjdk.org> wrote:
>> Chen Liang has updated the pull request incrementally with one additional
>> commit since the last revision:
>>
>> Remove assertion, no longer true with teleport definition in MHP
>
> src/java.base/share/classes/java/lang/invoke/MethodHandleProxies.java line
> 209:
>
>> 207: if (intfc.isHidden())
>> 208: throw newIllegalArgumentException("a hidden interface",
>> intfc.getName());
>> 209: if (!VM.isModuleSystemInited())
>
> I don't expect this is needed. I assume you are thinking for LMF to use this
> API?
Proxy-based impl had this check, so I'd assume MHP might want to defend against
the same kind of improper usage.
> src/java.base/share/classes/java/lang/invoke/MethodHandleProxies.java line
> 433:
>
>> 431:
>> 432: private static WrapperInstance asWrapperInstance(Object x) {
>> 433: if (x instanceof WrapperInstance wrapperInstance)
>
> In the previous version, `WrapperInstance` was not needed. Instead it checks
> if the class is a MHProxy class. Did you run into any issue that you
> resurrect `WrapperInstance`? Is it just because of accessing
> `ensureOriginalLookup`?
John Rose argues against using class data for Leyden. Since class data is the
only known way to defend against user-manufactured annotations, I switched back
to an extra wrapper interface. This approach also requires the Class loading
checks since the interface is not unconditionally exported and fails security
manager.
-------------
PR Review Comment: https://git.openjdk.org/jdk/pull/13197#discussion_r1207350363
PR Review Comment: https://git.openjdk.org/jdk/pull/13197#discussion_r1207349212
