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

[Maurizio Cimadamore]

Can you please expand a bit on this?

But also, warning: this is a RFR thread and I don't like it being side 
tracked to re-discuss higher level goals such as modeling headers with 
interfaces. If you want to raise this concern (seems so, but not 100% 
sure), as otherwise stated, please raise that in the appropriate forum 
(panama-spec-experts). Let's keep this list for discussion on the 
prototype, feedback on real usage, bugs, etc.

Maurizio

On 24/09/18 12:38, Samuel Audet wrote:
> FWIW, I think the factory method pattern should be reconsidered 
> entirely. In C/C++, when we want to call say getpid(), we don't start 
> loading stuff up before calling getpid(), we call getpid()! Why not do 
> the same from Java? From a usability point of view, not loading stuff 
> manually works fine for JavaCPP...
>
> Now, I know you're going to start taking about interfaces and what 
> not. You said that you had plans to introduce an entirely new array 
> type just to make it more friendly with vector instructions and native 
> libraries. Why not start thinking about an "interface" that would be 
> friendly to native libraries as well? Why stop at arrays?
>
> Samuel
>
> On 09/24/2018 08:10 PM, Maurizio Cimadamore wrote:
>> 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 
>>>
>>
>