Review Request JDK-8228336: Refactor native library loading implementation

[David Holmes]

Hi Mandy,

On 10/03/2020 9:33 am, Mandy Chung wrote:
> This patch refactors the native library loading implementation out of 
> ClassLoader and move them to jdk.internal.loader package. This 
> introduces a new NativeLibraries abstraction which loads and registers 
> the loaded native libraries.  Previously it was maintained in a 
> nativeLibrary map in each ClassLoader instance and one 
> systemNativeLibraries for the null loader case.  With this change, each 
> ClassLoader and BootLoader have its own NativeLibraries instead.
> 
> NativeLibraries.java and NativeLibraries.c are mostly refactored from 
> the existing code.  Only minor cleanups are applied.
> 
> This refactoring enables Panama LookupLibrary to further experiment and 
> allow loading of a native library without the restriction that a native 
> library can only be loaded by one class loader.  The restriction is 
> important for System::loadLibrary in loading JNI libraries that 
> implements JNI native methods and so remain unchanged.
> 
> This patch follows up the change introduced by JDK-8227587 which adds 
> BootLoader::loadLibrary.  It should belong a helper classes instead of 
> BootLoader and this helper method is to avoid doPrivileged call if no 
> security manager is installed for startup performance improvement. 
> Several `privilegedXXX` methods were added for the same purpose for 
> example sun.security.action.GetPropertyAction::privilegedGetProperty and 
> privilegedGetProperties.  So I think 
> sun.security.action.LoadLibraryAction is a proper place.

That's a core-libs decision but I'm not sure that's a namespace we want 
to increase usage of.

> Webrev:
> http://cr.openjdk.java.net/~mchung/jdk15/webrevs/8228336/webrev.00/

Overall refactoring looks okay to me. A couple of minor things I noticed:

src/java.base/share/classes/java/lang/ClassLoader.java

!                     " in java.library.path: " + 
StaticProperty.sunBootLibraryPath());

s/java/sun.boot/

src/java.base/share/classes/java/lang/Runtime.java

I don't see the point in changing load0 and loadLibrary0 to return 
NativeLibrary when it is unused. Is this to allow for direct use of 
these methods in the future?

Thanks,
David
-----

> Thanks
> Mandy