Skip to content
This repository has been archived by the owner on Mar 26, 2020. It is now read-only.

Implement basic automatic attachment/detachment of c++ threads to JVM. #405

Merged
merged 2 commits into from
Dec 30, 2018
Merged
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
12 changes: 11 additions & 1 deletion support-lib/jni/djinni_support.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -72,7 +72,17 @@ void jniShutdown() {
JNIEnv * jniGetThreadEnv() {
assert(g_cachedJVM);
JNIEnv * env = nullptr;
const jint get_res = g_cachedJVM->GetEnv(reinterpret_cast<void**>(&env), JNI_VERSION_1_6);
jint get_res = g_cachedJVM->GetEnv(reinterpret_cast<void**>(&env), JNI_VERSION_1_6);
#ifdef EXPERIMENTAL_AUTO_CPP_THREAD_ATTACH
if (get_res == JNI_EDETACHED) {
get_res = g_cachedJVM->AttachCurrentThread(&env, nullptr);

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I ran into a problem compiling the pull request using Oracle's JDK. The code compiles if I use Android's jni.h, but if I use jni.h that comes with Oracle's JDK, it fails because the AttachCurrentThread parameters are different. Android expects JNIEnv** whereas the JDK expects void**. I got around it with ifdefs, though I'm still in the process of testing the change at runtime.

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Also ran into a runtime issue with Android on NDK 16b, but NDK 18b works. Desktop Java (1.8) works as well (if you account for the difference in attach current thread parameters).

Copy link
Contributor Author

@mjmacleod mjmacleod Nov 26, 2018

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Would it be sufficient to introduce a compile time error for ndk <18?
It seems to me that as an experimental/optional feature thats off by default it should be fine for it to only work on new ndks going forward.

thread_local struct DetachOnExit {
~DetachOnExit() {
g_cachedJVM->DetachCurrentThread();
}
} detachOnExit;
}
#endif
if (get_res != 0 || !env) {
// :(
std::abort();
Expand Down