[foreign] RFR 8211060: Library.getDefault() does not work on Windows

[Jorn Vernee]

I also think the approach of removing the default library is more 
robust, since a symbol can be loaded from several libraries at once, but 
when using the default library you don't know which one you're gonna 
get. I can imagine a big project using several decencies which try to 
load the same symbol from different libraries through the default 
library and things breaking when the wrong function address is loaded.

On the other hand, library names for the standard library are platform 
specific. e.g. the standard C library dll is named msvcrt on windows, so 
writing platform agnostic code that just depends on the portable 
standard library, like the tests we have, would be more difficult.

Maybe that problem could be solved by shipping pre-computed library 
interfaces for the standard library with the JDK, where the value of 
NativeHeader#libraries would be platform specific. That would also 
decrease the time-to-helloworld for foreign, and I think that at least 
the standard C library is small enough that that won't be problematic.

Jorn

Maurizio Cimadamore schreef op 2018-09-24 13:10:
> Hi Jorn,
> thanks for the patch. As mentioned last time we have two options here:
> one is to follow the approach forward in your patch; another would be
> to ditch Library.getDefault() entirely and adopt a more explicit
> approach - that is to always require 'implicit' libraries to be
> mentioned - either by jextract artifacts or by API points.
> 
> A note on the latter - when you do:
> 
> Libraries.bindRaw(lookup, Foo.class)
> 
> the code lookup the @NativeHeader annotation on Foo.class and tries to
> extract required library names from there. Currently, we do not add
> library names for standard libraries such as "c" or "m" (math), which
> is what led us down the (slippery?) slope of having a
> Library.getDefault somewhere.
> 
> Another option would be to have jextract to always generate the names
> of the libraries an artifact depends on, and then the API should throw
> an exception if a @NativeHeader is found with no libraries. More
> specifically, jextract should always add "c" to the set of used
> libraries in the @NativeHeader annotation (other libraries can be
> explicitly supplied using the -l flag).
> 
> Note that I'm not saying "the binder should always add in 'clib'" for
> you, because that's kind of a lame assumption: it works obviously well
> for C but it doesn't make a lot of sense if you want to use Panama for
> other purposes/languages. Which seems to suggest that, as far as the
> binder is concerned, library dependencies should always be explicit.
> 
> Thoughts?
> 
> Maurizio
> 
> 
> 
> On 24/09/18 11:52, Jorn Vernee wrote:
>> Hello,
>> 
>> I'd like to contribute a patch that adds support for the default 
>> library on windows.
>> 
>> Bug: https://bugs.openjdk.java.net/browse/JDK-8211060
>> Diff: 
>> https://gist.github.com/JornVernee/7d45780df082cbfb27417b437c7b13a8
>> 
>> As mentioned before [1], this fixes 2 bugs:
>> 
>> 1.) When no library was loaded ClassLoader#NativeLibrary#getFromClass
>> threw an NPE (at least on windows). That is fixed by returning
>> defaultLibrary.fromClass when the nativeLibraryContext is empty.
>> 
>> 2.) The default library search was not working on windows. It was 
>> using
>> a default handle, which works on unix (dlsym(RTLD_DEFAULT)), but not 
>> on
>> windows (see https://stackoverflow.com/q/23437007). I have changed the
>> implementation from using a default handle to using a new native
>> function findEntryInProcess, which does the right thing for Windows
>> (iterate over all loaded modules), and does the same thing it used to
>> for unix. findEntry is now a Java method, and the original findEntry 
>> is
>> renamed to findEntry0. The NativeLibrary implementation of findEntry
>> forwards to findEntry0, and the anonymous class of the default library
>> overrides to forward to findEntryInProcess.
>> 
>> Please test this patch on unix as well, since I don't have the ability 
>> to do so.
>> 
>> Jorn
>> 
>> [1] : 
>> http://mail.openjdk.java.net/pipermail/panama-dev/2018-September/002644.html