System.loadLibrary and CLinker.lookup exception

[Maurizio Cimadamore]

There are problems with this suggestions, and reasons as to why the code 
is "duplicated" in the way it is.

First, these are @CallerSensitive methods, which means the JDK keeps 
track of who's the caller - to perform a check (e.g. to check if the 
caller belongs to a module with --enable-native-access, a Panama 
feature). So, at the very least, the check would need to be duplicated, 
we can't just consolidate it into a private shared function, otherwise 
the caller info will be lost.

Secondly, there are two versions of nativeLibraries::load - one which 
take a path and one which take a string. The current shape, while 
verbose, keeps types straight. If we move to a shared function, then we 
need to downcast the Object parameter to either Path or String, 
depending on the case, which doesn't look great either.

There is some opportunity for reuse in the code which creates 
RawNativeLibraries and then tries to load - that part can probably be 
moved into the shared libraryLookup helper function by using a 
functional interface and method references - but it's less of a slamdunk.

Maurizio


On 27/04/2022 05:26, Jack Andrews wrote:
> On Wed, 27 Apr 2022 at 03:00, Ty Young <youngty1997 at gmail.com> wrote:
>
>> Getting a symbol from a specific library has been possible for over a
>> month in the preview branch:
>>
>>
>> https://github.com/openjdk/panama-foreign/blob/foreign-preview/src/java.base/share/classes/java/lang/foreign/SymbolLookup.java#L126
>
> guys, someone has just done a copy+paste for these related (practically
> identical)  functions.
>
> @CallerSensitive
> static SymbolLookup libraryLookup(String name, MemorySession session) {
> Reflection.ensureNativeAccess(Reflection.getCallerClass(),
> SymbolLookup.class, "libraryLookup");
> Objects.requireNonNull(name);
> Objects.requireNonNull(session);
> RawNativeLibraries nativeLibraries =
> RawNativeLibraries.newInstance(MethodHandles.lookup());
> NativeLibrary library = nativeLibraries.load(name);
> if (library == null) {
> throw new IllegalArgumentException("Cannot open library: " + name);
> }
> return libraryLookup(nativeLibraries, library, session);
> }
> ..
> @CallerSensitive
> static SymbolLookup libraryLookup(Path path, MemorySession session) {
> Reflection.ensureNativeAccess(Reflection.getCallerClass(),
> SymbolLookup.class, "libraryLookup");
> Objects.requireNonNull(path);
> Objects.requireNonNull(session);
> RawNativeLibraries nativeLibraries =
> RawNativeLibraries.newInstance(MethodHandles.lookup());
> NativeLibrary library = nativeLibraries.load(path);
> if (library == null) {
> throw new IllegalArgumentException("Cannot open library: " + path);
> }
> return libraryLookup(nativeLibraries, library, session);
> }
>
> this really needs to be replaced with a private method:
>
> static private SymbolLookup libraryLookupPathOrString(Object pathOrString,
> MemorySession session) {
>    Reflection.ensureNativeAccess(Reflection.getCallerClass(),
> SymbolLookup.class, "libraryLookup");
>    Objects.requireNonNull(pathOrString);
>    Objects.requireNonNull(session);
>    RawNativeLibraries nativeLibraries =
> RawNativeLibraries.newInstance(MethodHandles.lookup());
>    NativeLibrary library = nativeLibraries.load(pathOrString);
>    if (library == null) {
>      throw new IllegalArgumentException("Cannot open library: " + path);
>    }
>    return libraryLookup(nativeLibraries, library, session);
> }
>
> and shrink the two methods to:
>
> static SymbolLookup libraryLookupPathOrString(String name, MemorySession
> session) {
>    return libraryLookupPathOrString(name,session)
> }
> static SymbolLookup libraryLookupPathOrString(Path path, MemorySession
> session) {
>    return libraryLookupPathOrString(path,session)
> }
>
> my java is rusty, but you get the idea.
> best,
> jack