On Tue, 5 Mar 2024 12:44:10 GMT, Jan Lahoda <jlah...@openjdk.org> wrote:

>> Currently, JDK modules load by the bootstrap and platform ClassLoaders are 
>> automatically granted the native access. I am working on an upgrade of JLine 
>> inside the `jdk.internal.le` module, and I would like to replace the current 
>> native bindings with FFM-based bindings (which are now somewhat provided by 
>> JLine). But, for that, native access is needed for the `jdk.internal.le` 
>> module. We could possibly move the module to the platform ClassLoader, but 
>> it seems it might be better to have more control over which modules have the 
>> native access.
>> 
>> This patch introduces an explicit list of modules that will automatically be 
>> granted the native access. Note this patch is not yet intended to change the 
>> end behavior - the list of modules granted native access is supposed to be 
>> the same as modules in the boot and platform ClassLoaders.
>
> Jan Lahoda has updated the pull request incrementally with one additional 
> commit since the last revision:
> 
>   Apply suggestions from code review
>   
>   Co-authored-by: ExE Boss <3889017+exe-b...@users.noreply.github.com>
>   Co-authored-by: Maurizio Cimadamore 
> <54672762+mcimadam...@users.noreply.github.com>

I assume you'll bump the copyright header on all files before integrating.

make/conf/module-loader-map.conf line 139:

> 137:     jdk.unsupported \
> 138:     jdk.xml.dom \
> 139:     jdk.zipfs \

We should create a JBS issue to prune this.

src/java.base/share/classes/java/lang/ModuleLayer.java line 896:

> 894:         return nameToModule.get(name);
> 895:     }
> 896: 

What would you think about replacing this with addEnableNativeAccess(String 
name) so it can be called by JLA. addEnableNativeAccess. The reason is that the 
JLA methods are usually just calls to some non-public method but the changes 
mean mean there is "core" in the JLA method that is not easy to find.

src/java.base/share/classes/jdk/internal/module/ModuleBootstrap.java line 812:

> 810:     }
> 811: 
> 812:     private static void addEnableNativeAccess(ModuleLayer layer, 
> Set<String> moduleNames, boolean shouldWarn) {

The private methods in this class have a short comment to summarise what they 
do.

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

PR Review: https://git.openjdk.org/jdk/pull/18106#pullrequestreview-1917862955
PR Review Comment: https://git.openjdk.org/jdk/pull/18106#discussion_r1513331594
PR Review Comment: https://git.openjdk.org/jdk/pull/18106#discussion_r1513345563
PR Review Comment: https://git.openjdk.org/jdk/pull/18106#discussion_r1513347180

Reply via email to