ClassHierarchyResolver using Reflection information

[Michael van Acken]

Am Fr., 17. März 2023 um 19:34 Uhr schrieb Brian Goetz <
brian.goetz at oracle.com>:

> 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.
>

The default classHierarchyResolver has an opaque (to me) error
mode that tripped me up.  Similar to my experience with ASM some
years ago, it works fine for a lot (most?) of code, but then some
input causes it to silently produce a stack map the verifier does
not agree with.  For me, this happens usually quite late in the
game and at first is completely baffling.  Back in July it was
mostly luck and half buried memories that sent me into the
direction of CHR as the solution to the verifier error.  If the
default CHR had raised an exception at the point where
information was missing, the path to the fix (preloading the CHR
with information about future classes) would have been clearer.

--mva



>
> 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
>
>
-------------- next part --------------
An HTML attachment was scrubbed...
URL: <https://mail.openjdk.org/pipermail/classfile-api-dev/attachments/20230318/24703280/attachment-0001.htm>