Methods named "invoke" for Kotlin callbacks using jna.Callback might be safer if called "callback"

[gmulhearn-anonyome]

hi, this is the first time i've ran into this, and only occurred with the introduction of UniFfiForeignExecutorCallback. When running debug android tests, i get this exception when UniFFI instance initializes:

java.lang.IllegalArgumentException: Callback must implement a single public method, or one public method named 'callback'
at com.sun.jna.CallbackReference.getCallbackMethod(CallbackReference.java:418)
at com.sun.jna.CallbackReference.getCallbackMethod(CallbackReference.java:388)
at com.sun.jna.CallbackReference.<init>(CallbackReference.java:283)
at com.sun.jna.CallbackReference.getFunctionPointer(CallbackReference.java:506)
at com.sun.jna.CallbackReference.getFunctionPointer(CallbackReference.java:483)
at com.sun.jna.Function.convertArgument(Function.java:558)
at com.sun.jna.Function.invoke(Function.java:345)
at com.sun.jna.Library$Handler.invoke(Library.java:265)
at java.lang.reflect.Proxy.invoke(Proxy.java:1006)
at $Proxy7.uniffi_foreign_executor_callback_set(Unknown Source)
at com.package.FfiConverterForeignExecutor.register$package_debug(UniffiWrapper.kt:2981)
at com.package._UniFFILib$Companion$INSTANCE$2.invoke(UniffiWrapper.kt:396)
at com.package._UniFFILib$Companion$INSTANCE$2.invoke(UniffiWrapper.kt:391)

I added some break lines in CallbackReference, essentially it has the following behaviour:

1. if there is only 1 public method (i.e. UniFfiForeignExecutorCallback.invoke) then register that as the callback handler
2. if there is more than 1 pub method, than find the public method called "callback" and use that,
3. if "callback" is not found, throw the exception seen above ^

I added some breaklines, and found that when running debug tests, jacoco is injecting another method into the UniFfiForeignExecutorCallback object: $jacocoInit https://github.com/jacoco/jacoco/issues/946 .

this occurs in at least [email protected] and [email protected] from my testing.

as that jacoco issue suggests, jna should really be ignoring synethic methods so that this error does not occur, however i wonder if UniFFI could improve by calling it's Callback methods "callback" instead? I've tried changing the generated file to callback and had success.

thnx

[gmulhearn-anonyome]

Opened a PR for what i think the change should be.. works and compiles locally for my use case, but will see if fixtures all pass etc

[gmulhearn-anonyome]

Hmmm, additionally, it seems like my use-case only solves when the method for callback in UniFfiForeignExecutorCallback is not internal:

object UniFfiForeignExecutorCallback : com.sun.jna.Callback {
internal fun callback(handle: USize, delayMs: Int, rustTask: UniFfiRustTaskCallback?, rustTaskData: Pointer?) {

->

object UniFfiForeignExecutorCallback : com.sun.jna.Callback { 
fun callback(handle: USize, delayMs: Int, rustTask: UniFfiRustTaskCallback?, rustTaskData: Pointer?) {

is there any problem with doing this? other jna.Callbacks in the templates seem to have their method as public?

[gmulhearn-anonyome]

hm, my latest commit with that change causes kotlin compile issues, as now UniFfiForeignExecutorCallback.callback is exposing internal data via it's parameters:

/home/circleci/project/target/tmp/uniffi-fixture-foreign-executor-48f7d97ead8ab37d/uniffi/fixture_foreign_executor/fixture_foreign_executor.kt:793:47: error: 'public' function exposes its 'internal' parameter type UniFfiRustTaskCallback
    fun callback(handle: USize, delayMs: Int, rustTask: UniFfiRustTaskCallback?, rustTaskData: Pointer?) {

https://app.circleci.com/pipelines/github/mozilla/uniffi-rs/3374/workflows/a4120840-5a0e-4279-8122-03723f8b36c4/jobs/13831

My gut is to make UniFfiForeignExecutorCallback have internal visibility, which works for my use case (and is actually preferred in my use case), but i don't know enough but other use cases (e.g. projects with multiple uniffi sources?) to know if it's ok to make UniFfiForeignExecutorCallback have internal visibility...