RFR: 8349145: Make Class.getProtectionDomain() non-native [v4]

[David Holmes]

On Mon, 3 Feb 2025 16:11:06 GMT, Coleen Phillimore <coleenp at openjdk.org> wrote:

>> This change removes the native call and injected field for ProtectionDomain in the java.lang.Class instance, and moves the field to be declared in Java.
>> Tested with tier1-4.
>
> Coleen Phillimore has updated the pull request incrementally with one additional commit since the last revision:
> 
>   Fix test that knows which fields are hidden from reflection in jvmci.

src/java.base/share/classes/java/lang/Class.java line 239:

> 237:      * generated.
> 238:      */
> 239:     private Class(ClassLoader loader, Class<?> arrayComponentType, ProtectionDomain pd) {

If this constructor is not used then why do we need to add the PD argument, rather than just set it to null? For that matter why do we even need the field if nothing is ever setting it? I'm missing something here.

src/java.base/share/classes/java/lang/Class.java line 2701:

> 2699: 
> 2700:     @Stable
> 2701:     private transient final ProtectionDomain protectionDomain;

Isn't `@Stable` superfluous with a final field?

src/java.base/share/classes/java/lang/Class.java line 2722:

> 2720:      */
> 2721:     public ProtectionDomain getProtectionDomain() {
> 2722:         if (protectionDomain == null) {

Does this imply the class is a primitive class?

test/jdk/java/lang/reflect/AccessibleObject/TrySetAccessibleTest.java line 205:

> 203: 
> 204:     /**
> 205:      * Test that some Class fields cannot be make accessible.

Suggestion:

     * Test that some Class fields cannot be made accessible.

test/jdk/java/lang/reflect/AccessibleObject/TrySetAccessibleTest.java line 216:

> 214: 
> 215:         assertTrue(pd == null);
> 216:     }

Suggestion:

    public void testJavaLangClassFields() throws Exception {
        try {
            // This field is explicitly hidden from reflection.
            Class.class.getDeclaredField("protectionDomain");
            assertTrue(false);
        } catch (NoSuchFieldException expected) { }
    }

The above is more in-style with the other test cases.

-------------

PR Review Comment: https://git.openjdk.org/jdk/pull/23396#discussion_r1942271834
PR Review Comment: https://git.openjdk.org/jdk/pull/23396#discussion_r1942261857
PR Review Comment: https://git.openjdk.org/jdk/pull/23396#discussion_r1942270930
PR Review Comment: https://git.openjdk.org/jdk/pull/23396#discussion_r1942265829
PR Review Comment: https://git.openjdk.org/jdk/pull/23396#discussion_r1942268434