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

[Jorn Vernee]

On Tue, 28 Apr 2020 14:49:08 GMT, Maurizio Cimadamore <mcimadamore at openjdk.org> wrote:

>> 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.
>
> Maurizio Cimadamore has updated the pull request incrementally with one additional commit since the last revision:
> 
>   * addressed review comments
>   * enhanced test to also try loading same library from multiple class loaders

Marked as reviewed by jvernee (Committer).

-------------

PR: https://git.openjdk.java.net/panama-foreign/pull/132