[foreign-abi] RFR: JDK-8243669: Improve library loading for Panama libraries

[Maurizio Cimadamore]

On 27/04/2020 23:13, Samuel Audet wrote:
> If reference counting gets used for both memory allocations and library
> loading, what about extracting that functionality into a public API that we
> could use with any other resources out there, such as GPU resources?
> JavaCPP is already doing that and it works great, but I don't see any
> reason why this kind of feature shouldn't be part of the JDK, something
> I've mentioned before...

There's no _explicit_ reference counting mechanism anywhere - not in the 
memory segment API, not in the library lookup API (which is what changed 
here). Not sure what you have in mind - but with the foreign API we have 
been trying to stay clear of explicit 'retain'/'release' calls.  Is 
there a specific reason why you think that the JDK would be in a unique 
position to provide a better solution for something like this?

Maurizio

>
> Samuel
>
> 2020年4月27日(月) 23:32 Maurizio Cimadamore <mcimadamore at openjdk.java.net>:
>
>> The code for loading native libraries has recently been cleaned up so as
>> to allow for same library to be loaded by
>> multiple loaders, in case the library being loaded is NOT a JNI library -
>> see JDK-8240975.
>>
>> This patch relaxes the Panama library loading mechanism so that the
>> classloader restriction is dropped; in fact, Panama
>> library loading is now orthogonal from the class loader in which the
>> loading operation occurs.
>>
>> The main issue with this enhancement is to decide how libraries should be
>> unloaded, given that same library might be
>> referred to by many different lookup objects in many different threads.
>>
>> If we aim for fully explicit library unloading (e.g.
>> `LibraryLookup::close`) this raises similar issues to those we
>> have for foreign memory access: we now have a library lookup which can be
>> closed while a method handle is operating on
>> an address generated by it in some other thread.
>>
>> We could solve the problem in the same way we solved the memory segment
>> problem - that is, making library lookup
>> objects thread-confined, and have each address be invalidated when a
>> lookup is closed (but then looked up addresses are
>> only usable withing the confinement thread). While doable, this seems to
>> go against how clients will use `SystemABI` to
>> generate native method handles, where they will probably want to stash a
>> bunch of method handles in static final
>> constants - and if these handles depend on a confined address, it means
>> they can't be shared.
>>
>> A saner solution is to let the GC manage library unloading - that is, we
>> setup a reference counter with each loaded
>> library; the more lookups are created from the same underlying native
>> library, the more the counter is incremented; as
>> lookup instances become unreachable, the counter is decremented. When the
>> counter reaches zero, the library is also
>> unloaded.
>>
>> To prevent races between library loading/unloading, the two routines in
>> charge of loading/unloading have been marked as
>> synchronized. This also means that the lookup on the `nativeLibraries`
>> instance has to be performed _inside_ the
>> synchronized block (we don't want to accidentally try to use a
>> `NativeLibrary` instance which has concurrently been
>> unloaded by the cleaner).
>>
>> This is a simple strategy, but also very effective: the lifetime of a
>> LibraryLookup controls that of the native library
>> it is associated with. In addition, all memory addresses generated by a
>> lookup keep a strong reference to the lookup -
>> and a native method handle generated by a `SystemABI::downcallHandle` call
>> will also keep a strong reference to the
>> address it refers to. Which means that if you store a method handle in a
>> static final field, you don't have to worry
>> about the `LibraryLookup` becoming unreachable, as it will be kept alive
>> by the method handles.
>>
>> -------------
>>
>> Commit messages:
>>   - Add support for library loading mechanism that is not dependent on
>> class loaders.
>>
>> Changes: https://git.openjdk.java.net/panama-foreign/pull/132/files
>>   Webrev: https://webrevs.openjdk.java.net/panama-foreign/132/webrev.00
>>    Issue: https://bugs.openjdk.java.net/browse/JDK-JDK-8243669
>>    Stats: 213 lines in 15 files changed: 174 ins; 10 del; 29 mod
>>    Patch: https://git.openjdk.java.net/panama-foreign/pull/132.diff
>>    Fetch: git fetch https://git.openjdk.java.net/panama-foreign
>> pull/132/head:pull/132
>>
>> PR: https://git.openjdk.java.net/panama-foreign/pull/132
>>