On Fri, 2 Jun 2023 03:36:35 GMT, Chen Liang <li...@openjdk.org> wrote:
>> Add API to explore Class Hierarchy with a `ClassLoader` or a `Lookup` with >> proper privileges, with tests. >> >> This addition is useful in case classes at runtime are not loaded from the >> system class loader, such as Proxy. This is also useful to APIs that >> generate bytecode with a `Lookup` object, such as a custom >> single-abstract-method class implementations from a method handle. >> >> See >> https://mail.openjdk.org/pipermail/classfile-api-dev/2023-March/000249.html >> as well. >> >> Current questions, which I wish to discuss with @asotona: >> 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. >> 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 redefined with instrumentation or jfr, which may >> not be reflected in the system resources. >> 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 Liang has updated the pull request with a new target base due to a merge > or a rebase. The pull request now contains 12 commits: > > - 1. Moved the default resolver to a static method, in anticipation of > future changes > 2. Removed SecurityManager related content > 3. Changed ClassHierarchyInfo into an interface > 4. Moved caching method from static to instance method > - Merge branch 'master' into hierarchy-resolve > - rename to ofClassLoading/ofResourceParsing > convert the default class provider to bypass security manager restrictions > - Merge branch 'master' into hierarchy-resolve > - Merge branch 'master' into hierarchy-resolve > - Test both cached and uncached resolvers > - Update the class hierarchy resolver api as per mailing list last week > - Merge branch 'master' into hierarchy-resolve > - Update > src/java.base/share/classes/jdk/internal/classfile/impl/ClassHierarchyImpl.java > > Co-authored-by: Andrey Turbanov <turban...@gmail.com> > - Make lookup based resolver throw on illegal access eagerly > - ... and 2 more: https://git.openjdk.org/jdk/compare/101bf229...9e9079fb Globas static cache needs to be synchronised If this PR goes in before #14180 Or a `defaultResolver()` should create a new cache instance (and document it is thread-unsafe) if it goes after #14180 src/java.base/share/classes/jdk/internal/classfile/ClassHierarchyResolver.java line 57: > 55: */ > 56: static ClassHierarchyResolver defaultResolver() { > 57: return ClassHierarchyImpl.DEFAULT_RESOLVER; defaultResolver() delegates to a thread-unsafe global static instance of cached resolver. src/java.base/share/classes/jdk/internal/classfile/impl/ClassHierarchyImpl.java line 130: > 128: @Override > 129: public ClassHierarchyInfo getClassInfo(ClassDesc classDesc) { > 130: var ret = resolvedCache.computeIfAbsent(classDesc, new > Function<>() { couldn't this function be cached? test/jdk/jdk/classfile/ClassHierarchyInfoTest.java line 34: > 32: * java.base/jdk.internal.classfile.impl > 33: * java.base/jdk.internal.classfile.impl.verifier > 34: * java.base/jdk.internal.classfile.java.lang.constant jdk.internal.classfile.java.lang.constant package has been removed ------------- Changes requested by asotona (Reviewer). PR Review: https://git.openjdk.org/jdk/pull/13082#pullrequestreview-1466930855 PR Review Comment: https://git.openjdk.org/jdk/pull/13082#discussion_r1221124185 PR Review Comment: https://git.openjdk.org/jdk/pull/13082#discussion_r1221102574 PR Review Comment: https://git.openjdk.org/jdk/pull/13082#discussion_r1221108184