<div dir="ltr"><div dir="ltr">Am Fr., 17. März 2023 um 19:34 Uhr schrieb Brian Goetz <<a href="mailto:brian.goetz@oracle.com">brian.goetz@oracle.com</a>>:<br></div><div class="gmail_quote"><blockquote class="gmail_quote" style="margin:0px 0px 0px 0.8ex;border-left:1px solid rgb(204,204,204);padding-left:1ex">I'm not so comfortable adding this to DEFAULT_CHA. In addition to <br>
adding another degree of environmental dependence, and the issues of <br>
additional exceptions you raise, I suspect it may also have a <br>
performance cost. I would like for programmers to opt into which CHA <br>
they use, and the default should be the most minimal. This means having <br>
a menu of CHAs to choose from, rather than guessing what the user wants.<br></blockquote><div><br></div><div>The default classHierarchyResolver has an opaque (to me) error<br>mode that tripped me up. Similar to my experience with ASM some<br>years ago, it works fine for a lot (most?) of code, but then some<br>input causes it to silently produce a stack map the verifier does<br>not agree with. For me, this happens usually quite late in the<br>game and at first is completely baffling. Back in July it was<br>mostly luck and half buried memories that sent me into the<br>direction of CHR as the solution to the verifier error. If the<br>default CHR had raised an exception at the point where<br>information was missing, the path to the fix (preloading the CHR<br>with information about future classes) would have been clearer.<br></div><div><br></div><div>--mva</div><div><br></div><div> </div><blockquote class="gmail_quote" style="margin:0px 0px 0px 0.8ex;border-left:1px solid rgb(204,204,204);padding-left:1ex">
<br>
On 3/17/2023 2:25 PM, - wrote:<br>
> I've submitted a patch at <a href="https://github.com/openjdk/jdk/pull/13082" rel="noreferrer" target="_blank">https://github.com/openjdk/jdk/pull/13082</a><br>
> Yet with it comes 3 questions:<br>
><br>
> 1. Should the resolver fail fast on IllegalAccessException from the<br>
> lookup? This usually indicates the hierarchy resolver is set up<br>
> improperly, and proceeding may simply yield verification errors in<br>
> class loading that are hard to track. For bytecode-generating APIs,<br>
> throwing access errors for the Lookup eagerly is also more preferable<br>
> than later silent generation failure.<br>
><br>
> The main concern for me to fail fast is that I don't understand how<br>
> the Classfile API propagates resolver exceptions. If wrapping the<br>
> IllegalAccessException in an IllegalAccessError is safe, then I may<br>
> change it and add a test case with HashMap.<br>
><br>
> 2. Whether the default resolver should be reading from jrt alone,<br>
> reflection alone, or jrt then reflection. I personally believe<br>
> reflection alone is more reliable, for classes may be redefined with<br>
> instrumentation or jfr, which may not be reflected in the system<br>
> resources.<br>
><br>
> This idea came to me while I was working on jfr and instrumentation<br>
> tests. Luckily, it seems they don't touch class hierarchy that the<br>
> default resolver suffices.<br>
><br>
> 3. In addition, I don't think chaining system class loader reflection<br>
> after system resource retrieval is really meaningful: is there any<br>
> case where reflection always works while the system resource retrieval<br>
> always fails?<br>
><br>
> Chen<br>
><br>
> On Fri, Mar 17, 2023 at 7:11 AM Adam Sotona <<a href="mailto:adam.sotona@oracle.com" target="_blank">adam.sotona@oracle.com</a>> wrote:<br>
>> I like your proposal and I would like to see it as a fallback solution for DEFAULT_CLASS_HIERARCHY_RESOLVER when the class is not available as resource stream.<br>
>><br>
>><br>
>><br>
>> /**<br>
>><br>
>> * Default singleton instance of {@linkplain ClassHierarchyResolver}<br>
>><br>
>> * using {@link ClassLoader#getSystemResourceAsStream(String)}<br>
>><br>
>> * as the {@code ClassStreamResolver} with fallback to<br>
>><br>
>> * {@link ClassLoader.getSystemClassLoader()} class loading resolver.<br>
>><br>
>> */<br>
>><br>
>> ClassHierarchyResolver DEFAULT_CLASS_HIERARCHY_RESOLVER<br>
>><br>
>> = new ClassHierarchyImpl.CachedClassHierarchyResolver(<br>
>><br>
>> new Function<ClassDesc, InputStream>() {<br>
>><br>
>> @Override<br>
>><br>
>> public InputStream apply(ClassDesc classDesc) {<br>
>><br>
>> return ClassLoader.getSystemResourceAsStream(Util.toInternalName(classDesc) + ".class");<br>
>><br>
>> }<br>
>><br>
>> }).orElse(ClassHierarchyResolver.of(ClassLoader.getSystemClassLoader()));<br>
>><br>
>><br>
>><br>
>> Thanks,<br>
>><br>
>> Adam<br>
>><br>
>><br>
>><br>
>> On 16.03.2023 17:05, "<a href="mailto:liangchenblue@gmail.com" target="_blank">liangchenblue@gmail.com</a>" <<a href="mailto:liangchenblue@gmail.com" target="_blank">liangchenblue@gmail.com</a>> wrote:<br>
>><br>
>><br>
>><br>
>> For context, in 8294961<br>
>> <a href="https://github.com/openjdk/jdk/pull/10991/files#r1133086265" rel="noreferrer" target="_blank">https://github.com/openjdk/jdk/pull/10991/files#r1133086265</a> I wondered<br>
>> about whether to use a hierarchy resolver for LambdaMetafactory, that<br>
>> I think resolution may encounter problems as the default resolver may<br>
>> not be able to resolve user-supplied interfaces.<br>
>><br>
>> In addition, many of the class file generation usages I've seen<br>
>> include firing events via an event bus, calling patched external addon<br>
>> code (as seen in Minecraft modding), in which Reflection-based<br>
>> hierarchy resolution would work better than reading bytecode files.<br>
>><br>
>> Thus, I've prepared a patch creating ClassHierarchyResolver<br>
>> <a href="https://github.com/openjdk/jdk/commit/0c888ba1e2953cf946012244446f4f8c05ba5d77" rel="noreferrer" target="_blank">https://github.com/openjdk/jdk/commit/0c888ba1e2953cf946012244446f4f8c05ba5d77</a><br>
>> to ease up resolution with reflection, when a ClassLoader (for older<br>
>> APIs) or a MethodHandle.Lookup (for modern APIs) is available.<br>
>><br>
>> Does this appear feasible?<br>
>><br>
>> Chen<br>
<br>
</blockquote></div></div>