[foreign] RFR: add support for callbacks in struct fields/globals

[Maurizio Cimadamore]

Thanks I already had found the first issue (at least the first) and was 
working on a followup patch.

I believe the second has to do with the fact that pointers to global 
variables have no associated scope. Good catch.

Maurizio


On 04/07/18 09:32, Sundararajan Athijegannathan wrote:
> The patch builds fine on Mac. Tests are passing as well. But I tried a 
> simple global function pointer example. Found two issues:
>
> * jextract change needed to implement java.nicl.types.Callback for 
> generated functional interfaces. Without this, binder threw exception 
> (as expected)
>
> diff -r 46db2ebe439d 
> src/jdk.jextract/share/classes/com/sun/tools/jextract/AsmCodeFactory.java
> --- 
> a/src/jdk.jextract/share/classes/com/sun/tools/jextract/AsmCodeFactory.java 
> Tue Jul 03 19:42:25 2018 +0530
> +++ 
> b/src/jdk.jextract/share/classes/com/sun/tools/jextract/AsmCodeFactory.java 
> Wed Jul 04 13:56:15 2018 +0530
> @@ -319,7 +319,7 @@
>          global_cw.visitInnerClass(name, internal_name, intf, 
> ACC_PUBLIC | ACC_STATIC | ACC_ABSTRACT | ACC_INTERFACE);
>          ClassWriter cw = new ClassWriter(ClassWriter.COMPUTE_FRAMES);
>          cw.visit(V1_8, ACC_PUBLIC | ACC_ABSTRACT | ACC_INTERFACE,
> -                name, null, "java/lang/Object", null);
> +                name, null, "java/lang/Object", new String[] { 
> "java/nicl/types/Callback" });
>          if (dcl != null) {
>              annotateNativeLocation(cw, dcl);
>          }
>
> * After making that change, binder went fine - but I still got NPE at 
> calling the functional interface method.
>
>     static long checkPointer(Pointer<?> ptr) throws Throwable {
>         Scope s = ((BoundedPointer<?>)ptr).scope();
>         if (s != null) s.checkAlive();  // <-- scope is null for 
> global function pointer
>         return ptr.addr();
>     }
>
>
> With those changes, I got global function pointer type variable working.
>
> PS. Attached the simple header/C code + Java main that I used to test.
>
> -Sundar
>
> On 03/07/18, 7:28 PM, Maurizio Cimadamore wrote:
>> Hi,
>> this patch adds binder support for function pointers in struct 
>> fields/global variables.
>>
>> This adds a new kind of reference abstraction which handles 
>> marshalling/unmarshalling of function pointers.
>>
>> When a function pointer is retrieved from a  struct/global var, we 
>> need to create a synthetic implementation for a target functional 
>> interface whose method implementation will do a native call to some 
>> native function. Note: in some cases, the native function being 
>> called could be a binder-generated native stub - we could detect this 
>> and save the object creation and the indirect allocation, but I have 
>> not done this as part of this patch.
>>
>> When a function pointer is set, we need to obtain an address from a 
>> functional interface instance; now, if such instance is 
>> binder-generated, we have a way to retrieve such address (by calling 
>> Callback::resource). Otherwise, the instance truly comes from Java 
>> code, so we need to allocate a stub and set its address onto the 
>> struct field.
>>
>> I had to do some reshuffling of the code generation logic - there's 
>> now a new code generator (namely CallbackImplGenerator) which uses 
>> some of the features also used by HeaderImplGenerator, so I moved 
>> these shared stuff in the common superclass BinderImplGenerator.
>>
>> Also, I've tweaked NativeInvoker NOT to eagerly bind the underlying 
>> handle to a function address. That's because when we work with header 
>> symbols, we always know the address statically, and we can bind to 
>> it; but when we work with callbacks, the address is stored in the 
>> callback implementation, so it's a dynamic property. As a result, 
>> it's better not to bind the address parameter of the method handle, 
>> and have the caller pass it explicitly - which works well in both 
>> cases (and saves one bound MH creation).
>>
>> I've also added some methods to BoundedPointer in order to create an 
>> unbounded pointer with given scope: we want the pointer set on the 
>> callback implementation to have the same scope/lifecycle as the 
>> struct field it corresponds to - this allows us to throw an exception 
>> when we attempt to call a callback implementation coming from a 
>> struct field whose scope has expired.
>>
>> The added test checks some of the assertions mentioned above.
>>
>> Webrev:
>> http://cr.openjdk.java.net/~mcimadamore/panama/callback_in_structs/
>>
>>
>> Maurizio
>>
>>
>>