Re: RFR: 8283060: RawNativeLibraries should allow multiple clients to load/unload the same library

2022-03-30 Thread Mandy Chung
On Wed, 30 Mar 2022 13:21:41 GMT, Jaikiran Pai  wrote:

>> A small improvement to `RawNativeLibraries`.   `RawNativeLibraries::load` 
>> returns the same `NativeLibrary` instance of a given path if it's called 
>> multiple times. This means that multiple clients have to coordinate that the 
>> last one using the loaded library is the one to close the library by calling 
>> `RawNativeLibraries::unload`; otherwise, an exception may be thrown.
>> 
>> This patch changes `RawNativeLibraries::load` to delegate to the underlying 
>> platform-specific library loading mechanism e.g. dlopen/dlclose that keeps 
>> the reference count.  So each call to `RawNativeLibraries::load` and 
>> `unload` is simply a call to dlopen and dlclose respectively.  Each client 
>> of calling `RawNativeLibraries::load` is responsible for calling 
>> `RawNativeLibraries::unload`.  This will be consistent with the current 
>> behavior when you call `load` and `unload` with a different 
>> `RawNativeLibraries` instance.
>
> src/java.base/share/classes/jdk/internal/loader/RawNativeLibraries.java line 
> 49:
> 
>> 47: public final class RawNativeLibraries {
>> 48: final Set libraries = 
>> ConcurrentHashMap.newKeySet();
>> 49: final Class caller;
> 
> Hello Mandy, 
> Apart from the `caller` being used for checks while constructing a 
> `RawNativeLibraries` in `RawNativeLibraries.newInstance(MethodHandles.Lookup 
> trustedCaller)`, I don't see this instance member being used anywhere. Do you 
> think we need to store this as an instance member?

I keep it as a record and for debugging use in case.

-

PR: https://git.openjdk.java.net/jdk/pull/8022


Re: RFR: 8283060: RawNativeLibraries should allow multiple clients to load/unload the same library

2022-03-30 Thread Jaikiran Pai
On Tue, 29 Mar 2022 20:05:50 GMT, Mandy Chung  wrote:

> A small improvement to `RawNativeLibraries`.   `RawNativeLibraries::load` 
> returns the same `NativeLibrary` instance of a given path if it's called 
> multiple times. This means that multiple clients have to coordinate that the 
> last one using the loaded library is the one to close the library by calling 
> `RawNativeLibraries::unload`; otherwise, an exception may be thrown.
> 
> This patch changes `RawNativeLibraries::load` to delegate to the underlying 
> platform-specific library loading mechanism e.g. dlopen/dlclose that keeps 
> the reference count.  So each call to `RawNativeLibraries::load` and `unload` 
> is simply a call to dlopen and dlclose respectively.  Each client of calling 
> `RawNativeLibraries::load` is responsible for calling 
> `RawNativeLibraries::unload`.  This will be consistent with the current 
> behavior when you call `load` and `unload` with a different 
> `RawNativeLibraries` instance.

src/java.base/share/classes/jdk/internal/loader/RawNativeLibraries.java line 49:

> 47: public final class RawNativeLibraries {
> 48: final Set libraries = 
> ConcurrentHashMap.newKeySet();
> 49: final Class caller;

Hello Mandy, 
Apart from the `caller` being used for checks while constructing a 
`RawNativeLibraries` in `RawNativeLibraries.newInstance(MethodHandles.Lookup 
trustedCaller)`, I don't see this instance member being used anywhere. Do you 
think we need to store this as an instance member?

-

PR: https://git.openjdk.java.net/jdk/pull/8022


Re: RFR: 8283060: RawNativeLibraries should allow multiple clients to load/unload the same library

2022-03-30 Thread Jorn Vernee
On Tue, 29 Mar 2022 20:05:50 GMT, Mandy Chung  wrote:

> A small improvement to `RawNativeLibraries`.   `RawNativeLibraries::load` 
> returns the same `NativeLibrary` instance of a given path if it's called 
> multiple times. This means that multiple clients have to coordinate that the 
> last one using the loaded library is the one to close the library by calling 
> `RawNativeLibraries::unload`; otherwise, an exception may be thrown.
> 
> This patch changes `RawNativeLibraries::load` to delegate to the underlying 
> platform-specific library loading mechanism e.g. dlopen/dlclose that keeps 
> the reference count.  So each call to `RawNativeLibraries::load` and `unload` 
> is simply a call to dlopen and dlclose respectively.  Each client of calling 
> `RawNativeLibraries::load` is responsible for calling 
> `RawNativeLibraries::unload`.  This will be consistent with the current 
> behavior when you call `load` and `unload` with a different 
> `RawNativeLibraries` instance.

Marked as reviewed by jvernee (Reviewer).

-

PR: https://git.openjdk.java.net/jdk/pull/8022


Re: RFR: 8283060: RawNativeLibraries should allow multiple clients to load/unload the same library

2022-03-29 Thread Athijegannathan Sundararajan
On Tue, 29 Mar 2022 20:05:50 GMT, Mandy Chung  wrote:

> A small improvement to `RawNativeLibraries`.   `RawNativeLibraries::load` 
> returns the same `NativeLibrary` instance of a given path if it's called 
> multiple times. This means that multiple clients have to coordinate that the 
> last one using the loaded library is the one to close the library by calling 
> `RawNativeLibraries::unload`; otherwise, an exception may be thrown.
> 
> This patch changes `RawNativeLibraries::load` to delegate to the underlying 
> platform-specific library loading mechanism e.g. dlopen/dlclose that keeps 
> the reference count.  So each call to `RawNativeLibraries::load` and `unload` 
> is simply a call to dlopen and dlclose respectively.  Each client of calling 
> `RawNativeLibraries::load` is responsible for calling 
> `RawNativeLibraries::unload`.  This will be consistent with the current 
> behavior when you call `load` and `unload` with a different 
> `RawNativeLibraries` instance.

LGTM

-

Marked as reviewed by sundar (Reviewer).

PR: https://git.openjdk.java.net/jdk/pull/8022


RFR: 8283060: RawNativeLibraries should allow multiple clients to load/unload the same library

2022-03-29 Thread Mandy Chung
A small improvement to `RawNativeLibraries`.   `RawNativeLibraries::load` 
returns the same `NativeLibrary` instance of a given path if it's called 
multiple times. This means that multiple clients have to coordinate that the 
last one using the loaded library is the one to close the library by calling 
`RawNativeLibraries::unload`; otherwise, an exception may be thrown.

This patch changes `RawNativeLibraries::load` to delegate to the underlying 
platform-specific library loading mechanism e.g. dlopen/dlclose that keeps the 
reference count.  So each call to `RawNativeLibraries::load` and `unload` is 
simply a call to dlopen and dlclose respectively.  Each client of calling 
`RawNativeLibraries::load` is responsible for calling 
`RawNativeLibraries::unload`.  This will be consistent with the current 
behavior when you call `load` and `unload` with a different 
`RawNativeLibraries` instance.

-

Commit messages:
 - 8283060: RawNativeLibraries should allow multiple clients to load/unload the 
same library

Changes: https://git.openjdk.java.net/jdk/pull/8022/files
 Webrev: https://webrevs.openjdk.java.net/?repo=jdk=8022=00
  Issue: https://bugs.openjdk.java.net/browse/JDK-8283060
  Stats: 66 lines in 3 files changed: 43 ins; 10 del; 13 mod
  Patch: https://git.openjdk.java.net/jdk/pull/8022.diff
  Fetch: git fetch https://git.openjdk.java.net/jdk pull/8022/head:pull/8022

PR: https://git.openjdk.java.net/jdk/pull/8022