Remove cast to void** as the formal parameter type should be JNIEnv**

[eakoli]

A recent changed altered the call to pass the p_env parameter as a void**, which causes compilation errors as the format prototype of AttachCurrentProcess is

 jint AttachCurrentThread(JNIEnv** p_env, void* thr_args)

[a4z]

I need to find out why the reinterprate cast has been added explicit here

755d165

I have forgotten the reason, but I think there was one

[a4z]

remember now why this cast exists

Building CXX object CMakeFiles/djinni_support_lib.dir/djinni/jni/djinni_support.cpp.o
FAILED: CMakeFiles/djinni_support_lib.dir/djinni/jni/djinni_support.cpp.o 
/Applications/Xcode.app/Contents/Developer/Toolchains/XcodeDefault.xctoolchain/usr/bin/c++ -DEXPERIMENTAL_AUTO_CPP_THREAD_ATTACH -I/Users/a4z/work/djinni/support-lib -I/Users/a4z/work/djinni/support-lib/djinni/jni -I/Users/a4z/.asdf/installs/java/zulu-11.41.23/zulu-11.jdk/Contents/Home/include -I/Users/a4z/.asdf/installs/java/zulu-11.41.23/zulu-11.jdk/Contents/Home/include/darwin -g -isysroot /Applications/Xcode.app/Contents/Developer/Platforms/MacOSX.platform/Developer/SDKs/MacOSX11.3.sdk -fPIC -std=c++17 -MD -MT CMakeFiles/djinni_support_lib.dir/djinni/jni/djinni_support.cpp.o -MF CMakeFiles/djinni_support_lib.dir/djinni/jni/djinni_support.cpp.o.d -o CMakeFiles/djinni_support_lib.dir/djinni/jni/djinni_support.cpp.o -c /Users/a4z/work/djinni/support-lib/djinni/jni/djinni_support.cpp
/Users/a4z/work/djinni/support-lib/djinni/jni/djinni_support.cpp:78:52: error: cannot initialize a parameter of type 'void **' with an rvalue of type 'JNIEnv **' (aka 'JNIEnv_ **')
        get_res = g_cachedJVM->AttachCurrentThread(&env, nullptr);
                                                   ^~~~
/Users/a4z/.asdf/installs/java/zulu-11.41.23/zulu-11.jdk/Contents/Home/include/jni.h:1924:37: note: passing argument to parameter 'penv' here
    jint AttachCurrentThread(void **penv, void *args) {
                                    ^
1 error generated.

and indeed, the Zulu JDK seems to take a void**

Now it starts to become complicated, does that mean Zulu JDK is not usable?

because it is true thatthe oracle docs say jint AttachCurrentThread(JavaVM *vm, void **p_env, void *thr_args);

So I am willy to pull in the PR, since it seems to be correct, but that also means ZULU JDK via conan is not usable .... (edited after adding a gh action for that pr, see next comment)

anyone an idea what's the problem in the zulu jdk ?

[a4z]

also in a gh action, if we add the flag, would fail

[  2%] Generating Djinni Java bindings from /home/runner/work/djinni-support-lib/djinni-support-lib/test-suite/djinni/all.djinni
Parsing...
/home/runner/work/djinni-support-lib/djinni-support-lib/djinni/jni/djinni_support.cpp: In function ‘JNIEnv* djinni::jniGetThreadEnv()’:
/home/runner/work/djinni-support-lib/djinni-support-lib/djinni/jni/djinni_support.cpp:78:52: error: invalid conversion from ‘JNIEnv**’ {aka ‘JNIEnv_**’} to ‘void**’ [-fpermissive]
   78 |         get_res = g_cachedJVM->AttachCurrentThread(&env, nullptr);
      |                                                    ^~~~
      |                                                    |
      |                                                    JNIEnv** {aka JNIEnv_**}
In file included from /home/runner/work/djinni-support-lib/djinni-support-lib/djinni/jni/djinni_support.hpp:29,
                 from /home/runner/work/djinni-support-lib/djinni-support-lib/djinni/jni/djinni_support.cpp:18:
/usr/lib/jvm/adoptopenjdk-11-hotspot-amd64/include/jni.h:1924:37: note:   initializing argument 1 of ‘jint JavaVM_::AttachCurrentThread(void**, void*)’
 1924 |     jint AttachCurrentThread(void **penv, void *args) {
      |                              ~~~~~~~^~~~
make[2]: *** [CMakeFiles/djinni_support_lib.dir/build.make:76: CMakeFiles/djinni_support_lib.dir/djinni/jni/djinni_support.cpp.o] Error 1
Resolving...

pls see https://github.com/a4z/djinni-support-lib/runs/3212494909?check_suite_focus=true

[jothepro]

Ouff, that sucks... 🤨

My guess it that this may be an incompatibility between OpenJDK and the Android NDK?

• OpenJDK: https://github.com/openjdk/jdk/blob/master/src/java.base/share/native/include/jni.h#L1938
• Android NDK: https://github.com/ricardoquesada/android-ndk/blob/master/usr/include/jni.h#L1090

[a4z]

omg, this must have been the relevant change to make google win the law suite against oracle (beside the different code formatting, of course) :-)

I think we might be able to fix this with some #ifdef __ANDROID__ ,
but this also means we need an NDK build in the CI,
which we should have anyway, it's just, NDK download is huge, and will dramatically slow down the pipeline ....

[a4z]

Please adopt change as suggested.

Android JNI has a different interface as Oracle JNI, and we need to address that.

Thanks!

[a4z]

Hej @eakoli ,please check your repo, I added a PR that does the required changes so it works on both, Android and Oracle JNI, plus github action checks for both cofigs

NetsoftHoldings#1

please merge that , so we can pull in the changes, thanks!

[a4z]

Thanks! Awesome, I think this PR lead to some improvements. Thanks a lot NetSoftHoldings team!

Since I am on co author of this PR, please @jothepro and/or @freitass review the changes.

[freitass]

I appreciate how this preserves the compatibility with other platforms. Thanks everyone!