RFR: 8344319: SM cleanup in jdk.dynalink module [v2]

[Roger Riggs]

On Sat, 16 Nov 2024 15:24:03 GMT, Attila Szegedi <attila at openjdk.org> wrote:

>> Roger Riggs has updated the pull request incrementally with one additional commit since the last revision:
>> 
>>   Apply review comments:
>>   Retain public static constants for permission names to avoid source/binary compatible changes that affect cross version use.
>>   Remove obsolete mention of security considerations.
>>   (The protected constructor is retained to avoid changing the access for subclass use)
>
> src/jdk.dynalink/share/classes/jdk/dynalink/SecureLookupSupplier.java line 46:
> 
>> 44:      * {@link #getLookup()} method.
>> 45:      */
>> 46:     public static final String GET_LOOKUP_PERMISSION_NAME = "dynalink.getLookup";
> 
> This public member is [referenced from Nashorn](https://github.com/search?q=repo%3Aopenjdk%2Fnashorn%20GET_LOOKUP_PERMISSION_NAME&type=code). If we remove it, Nashorn will have to attempt to look it up reflectively as long as it supports older versions of Java. (Presuming the `java.security.AccessController` and related classes aren't also removed altogether. Are they?)
> 
> FWIW, as long as we're making breaking changes, this whole class could be removed – all it did was serve as a secure gate to access to a `MethodHandles.Lookup`. References to it could be replaced by the lookup object itself. I can understand if this might be too much work for this PR, though.
> 
> On the other hand, if we do _not_ remove the whole class, I'd prefer to keep the public string constant for the permission name so that we don't break binary compatibility for whoever might be referencing it (specifically, Nashorn.)

Agreed, it is cleaner to retain the permission names in the public API.

> src/jdk.dynalink/share/classes/jdk/dynalink/linker/GuardingDynamicLinkerExporter.java line 65:
> 
>> 63:         if (sm != null) {
>> 64:             sm.checkPermission(AUTOLOAD_PERMISSION);
>> 65:         }
> 
> We can probably remove the whole empty default constructor now. The class is abstract, the autogenerated default constructor will be protected anyway.

Keeping the explicitly declared constructor for clarity.

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

PR Review Comment: https://git.openjdk.org/jdk/pull/22152#discussion_r1846872687
PR Review Comment: https://git.openjdk.org/jdk/pull/22152#discussion_r1846873840