ClassHierarchyResolver using Reflection information

[Brian Goetz]

I'm not so comfortable adding this to DEFAULT_CHA.  In addition to 
adding another degree of environmental dependence, and the issues of 
additional exceptions you raise, I suspect it may also have a 
performance cost.  I would like for programmers to opt into which CHA 
they use, and the default should be the most minimal.  This means having 
a menu of CHAs to choose from, rather than guessing what the user wants.

On 3/17/2023 2:25 PM, - wrote:
> I've submitted a patch at https://github.com/openjdk/jdk/pull/13082
> Yet with it comes 3 questions:
>
> 1. Should the resolver fail fast on IllegalAccessException from the
> lookup? This usually indicates the hierarchy resolver is set up
> improperly, and proceeding may simply yield verification errors in
> class loading that are hard to track. For bytecode-generating APIs,
> throwing access errors for the Lookup eagerly is also more preferable
> than later silent generation failure.
>
> The main concern for me to fail fast is that I don't understand how
> the Classfile API propagates resolver exceptions. If wrapping the
> IllegalAccessException in an IllegalAccessError is safe, then I may
> change it and add a test case with HashMap.
>
> 2. Whether the default resolver should be reading from jrt alone,
> reflection alone, or jrt then reflection. I personally believe
> reflection alone is more reliable, for classes may be redefined with
> instrumentation or jfr, which may not be reflected in the system
> resources.
>
> This idea came to me while I was working on jfr and instrumentation
> tests. Luckily, it seems they don't touch class hierarchy that the
> default resolver suffices.
>
> 3. In addition, I don't think chaining system class loader reflection
> after system resource retrieval is really meaningful: is there any
> case where reflection always works while the system resource retrieval
> always fails?
>
> Chen
>
> On Fri, Mar 17, 2023 at 7:11 AM Adam Sotona <adam.sotona at oracle.com> wrote:
>> 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.
>>
>>
>>
>>     /**
>>
>>       * Default singleton instance of {@linkplain ClassHierarchyResolver}
>>
>>       * using {@link ClassLoader#getSystemResourceAsStream(String)}
>>
>>       * as the {@code ClassStreamResolver} with fallback to
>>
>>       * {@link ClassLoader.getSystemClassLoader()} class loading resolver.
>>
>>       */
>>
>>      ClassHierarchyResolver DEFAULT_CLASS_HIERARCHY_RESOLVER
>>
>>              = new ClassHierarchyImpl.CachedClassHierarchyResolver(
>>
>>              new Function<ClassDesc, InputStream>() {
>>
>>                  @Override
>>
>>                  public InputStream apply(ClassDesc classDesc) {
>>
>>                      return ClassLoader.getSystemResourceAsStream(Util.toInternalName(classDesc) + ".class");
>>
>>                  }
>>
>>              }).orElse(ClassHierarchyResolver.of(ClassLoader.getSystemClassLoader()));
>>
>>
>>
>> Thanks,
>>
>> Adam
>>
>>
>>
>> On 16.03.2023 17:05, "liangchenblue at gmail.com" <liangchenblue at gmail.com> wrote:
>>
>>
>>
>> For context, in 8294961
>> https://github.com/openjdk/jdk/pull/10991/files#r1133086265 I wondered
>> about whether to use a hierarchy resolver for LambdaMetafactory, that
>> I think resolution may encounter problems as the default resolver may
>> not be able to resolve user-supplied interfaces.
>>
>> In addition, many of the class file generation usages I've seen
>> include firing events via an event bus, calling patched external addon
>> code (as seen in Minecraft modding), in which Reflection-based
>> hierarchy resolution would work better than reading bytecode files.
>>
>> Thus, I've prepared a patch creating ClassHierarchyResolver
>> https://github.com/openjdk/jdk/commit/0c888ba1e2953cf946012244446f4f8c05ba5d77
>> to ease up resolution with reflection, when a ClassLoader (for older
>> APIs) or a MethodHandle.Lookup (for modern APIs) is available.
>>
>> Does this appear feasible?
>>
>> Chen