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

[Samuel Audet]

I'll reply to the renamed thread, sorry about that.

"re-discuss", well, AFAIK, it's never been discussed publicly once 
anywhere. One day, someone came up with the idea that interfaces should 
be used for everything, and that was the end of the "discussion". 
Although I know we're not supposed to have discussions here, there are 
no other places to have them, so I'm trying here anyway until you guys 
come up with with forums for that. :) The "spec" list is quite low 
volume and moderated, not exactly an inviting place for discussion...

Samuel

On 09/24/2018 09:00 PM, Maurizio Cimadamore wrote:
> 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 
>>>>
>>>
>>
>