RFR: 8227587: Add internal privileged System.loadLibrary

[Mandy Chung]

On 7/17/19 3:25 AM, Claes Redestad wrote:
> Hi Peter,
>
> On 2019-07-17 10:29, Peter Levart wrote:
>> Hi Claes,
>>
>> Am I reading correct that package-private 
>> ClassLoader.loadLibrary(Class<?> fromClass, String name, boolean 
>> isAbsolute) wouldn't need to consult SecurityManager wasn't it for 
>> accessing system properties in private 
>> ClassLoader.initializePath(String propName). So perhaps if the two 
>> calls to initializePath() in the if block of 
>> ClassLoader.loadLibrary() are wrapped with a single doPrivileged(), 
>> you wouldn't have to wrap the call to JavaLangAccess.loadLibrary in 
>> BootLoader. And since initializePath() would only be called once and 
>> then the results cached, you end up with less doPrivileged() 
>> wrappings when SecurityManager is present as soon as 
>> BootLoader.loadLibrary is called the 2nd time.
>
> hmm, fromClass.getClassLoader() also does a security check, though.
>

Class::getClassLoader is caller-sensitive and the call to 
fromClass.getClassLoader() does not trigger any security permission 
check as it's called from NativeLibrary who is loaded by the boot loader.

> That said, there's a case for wrapping the initializePath calls in a
> doPriv of their own, since we now have an implicit requirement that some
> system library is loaded first (so that sys_paths is initialized) before
> any user code attempts to load a library of their own...
>

I created https://bugs.openjdk.java.net/browse/JDK-8228336 to track the 
idea to refactor the system native library loading to BootLoader.   I 
also have JDK-8228336 capturing Peter's observation.

>>
>> BTW, there is a data race when accessing usr_paths and sys_paths 
>> static fields in multi-threaded scenario. Their type is String[] 
>> which means that the contents of the arrays could be observed 
>> uninitialized (elements being null) in some thread. Both fields 
>> should be made volatile (not only sys_paths) as assignment to them 
>> can be performed more than once in idempotent racy initialization.
>
> .. and we typically seem to be loading at least one library before
> initializing any user code, which is probably why this has never been an
> issue. It seems like a good idea to remove this subtle fragility,
> though. I wonder if we shouldn't "simply" initialize the two paths
> during bootstrap and put them in final fields.
>

JDK-8228336 tracks this.

Mandy