Review Request JDK-8228336: Refactor native library loading implementation
david.holmes at oracle.com
Tue Mar 10 02:41:26 UTC 2020
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.
Overall refactoring looks okay to me. A couple of minor things I noticed:
! " in java.library.path: " +
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?
More information about the core-libs-dev