JNI_OnLoad/JNI_OnUnload - default implementation not optional

[mutagene]

In djinni_jni_main.hpp it is written:

Don't include it if your library does require a custom JNI_OnLoad implementation with custom initialization logic.

Unfortunately the djinni-support-lib has not made the inclusion of this default implementation optional.

[a4z]

I think you are right, the idea was to provide just an header, and let you include the header if you need a pre-written JNI_OnLoad

And everything was fine until this change , https://github.com/cross-language-cpp/djinni-support-lib/pull/26/files

@jothepro , do you remember if there was a reason to add that file
https://github.com/jothepro/djinni-support-lib/blob/3e0d40e25f972a23363459ca083ed405f62fee0d/CMakeLists.txt#L24
without the options it had before
https://github.com/jothepro/djinni-support-lib/blob/568593d4f9e0b16ddf60a1e5922742796eeda9f9/CMakeLists.txt#L88

I fear I can not recall all details we discused back then ...

[jothepro]

I recall that the following was my thought-process leading to these changes:

There is 2 ways how you can use the djinni-support-lib in your project: As static or as shared lib.
Every shared lib has to provide a JNI_OnLoad implementation.

If you use the support-lib as shared library, it has to provide it's own JNI initialization mechanism for it to be loaded properly, which is provided by djinni_jni_main.cpp.
If you link the support-lib as static lib, it is also built with djinni_jni_main.cpp. But the symbols exported from these default implementations for JNI_OnLoad and JNI_OnUnload are not used by the linking library, so they get discarded. That's the reason why there is no need to explicitly exclude them by a switch, the linker will automatically do this for you.

If you choose to provide your own implementations of JNI_OnLoad and JNI_OnUnload, you can do this by setting --jni-generate-main false in the generator, which will disable the generation of a djinni_jni_main.cpp file alongside with the generated gluecode.

That's how it is still possible to choose a custom loading implementation in your library. Only in case you use the support-lib as shared library, the loading of the support-lib itself always depends on the default implementation, while the library depending on the support-lib can choose it's own loader.

---

I struggle to describe the whole situation in short words, that's why I tried to visualize it in the PR: cross-language-cpp/djinni-generator#49 (comment)

The main flaw of this concept may be that it's difficult to grasp. If you have an idea on how this could be made clearer in the docs or the sources, I'd be super grateful! If there is something that's not clear to you after reading this answer and the linked thread, pls keep asking!

BTW I am not sure if I ever tested using the support-lib as shared library. @a4z pointed out here that this may be an edge case that's not common.
If one could prove that it doesn't make sense to use the support-lib as standalone shared library, djinni_jni_main.cpp could be removed to avoid confusion. 🤔

[a4z]

Thanks @jothepro , I think you described the situation very well.

I think we can ask CMake if it is generating a dynamic library, and if than do that

djinni-support-lib/CMakeLists.txt

Line 24 in 42c05ca

"djinni/jni/djinni_jni_main.cpp"

otherwise, do nothing.

So if we need to query DJINNI_LIBRARY_TYPE and if not set cmakes DJINNI_LIBRARY_TYPE, and add the file only if we build a dynamic library

[a4z]

If one could prove that it doesn't make sense to use the support-lib as standalone shared library, ...

All you need to do is add the support lib into the android package and load 2 shared libs instead of only 1, so technically we should not close the door

but we maybe need better docs ....

[a4z]

Added a PR , #51, to address this issue

@mutagene , out of interest , do you use a custom JNI_OnLoad function, and if , may I ask what you are doing in it?

[mutagene]

Thanks for the follow-up. We were using a custom JNI_OnUnload function with a fork we had made of the dropbox repo a long time ago. The motivations of the custom unload function was that we had some async operations being performed and (apparently, judging from the comments - it was a while ago now) we were concerned about jniShutdown being called before the async operations completed.

[a4z]

This is an interesting use case, especially since I would expect that the unload of the library happens when the program exits, what should be at at point in time async operations are done.
Exception, loading and unloading happens manually.

Anyhow, we removed the file from the static lib to make the indention of the djinni_main in the support lib more clear.
And we might improve documentation in future.