On Tue, 16 Jul 2024 13:35:50 GMT, Alan Bateman <al...@openjdk.org> wrote:
>> Please review this patch that address the reflection member filtering flaws. >> >> 1. Remove a pointless bootstrap check to ensure no filter is accidentally >> bypassed due to class-loading order. >> 2. Clarify the scenarios for filtering, and the correct filter actions for >> each scenario. >> 3. Remove the filter in `sun.misc.Unsafe`; users are already using other >> ways to steal this instance, bypassing the filtered getter. The >> `jdk.internal.misc.Unsafe`'s filter was already removed in JDK-8161129, so >> this filter is meaningless. > > src/java.base/share/classes/jdk/internal/reflect/Reflection.java line 58: > >> 56: // 3 filter scenarios: >> 57: // 1. Classes loaded before Reflection, may (System) or may not >> 58: // (ConstantPool) be initialized before main call: below > > This comment is confusing. I assume you want to say that these classes may or > may not be loaded before the application main method. It may be simpler to > just drop the proposed comments as they beg too many questions. Let's put these 4 points in time: class A load, A init, Reflection init, main run. (If an event is after "main run", it may be completely optional) There can be these arrangements: 1. A load, Reflection init, main run; subcases include: a. A load, A init, Reflection init, main run (System) b. A load, Reflection init, A init, main run (none known yet) c. A load, Reflection init, main run, A init (ConstantPool) Handling: register in Reflection static block 2. Reflection init, A load, A init, main run (MethodHandles.Lookup) Handling: register in their own static block 3. Reflection init, main run, A init; subcases incldue: a. Reflection init, A load, main run, A init (none known yet) b. Reflection init, main run, A load, A init (sun.misc.Unsafe) Handling: Quite hard, easily causing extra class loading How should I phrase my comments? ------------- PR Review Comment: https://git.openjdk.org/jdk/pull/20058#discussion_r1679515927
