JNI: add a static getter for a JNIEnv pointer to QJniEnvironment

This further helps reduce the creation of temporary QJniEnivronment
instances (with allocated d-pointer) for cases where we simply need to
get the JNIEnv for the current thread.

Change-Id: I2eda238124be51c755d8910de9dbc9ca8eb92288
Reviewed-by: Tinja Paavoseppä <tinja.paavoseppa@qt.io>
Reviewed-by: Qt CI Bot <qt_ci_bot@qt-project.org>
Reviewed-by: Assam Boudjelthia <assam.boudjelthia@qt.io>
Reviewed-by: Petri Virkkunen <petri.virkkunen@qt.io>

5 files changed, 65 insertions, 60 deletions

diff --git a/src/corelib/kernel/qjniarray.h b/src/corelib/kernel/qjniarray.h
index b6d4c4ab24..1a990221cd 100644
--- a/src/corelib/kernel/qjniarray.h
+++ b/src/corelib/kernel/qjniarray.h
@@ -278,16 +278,15 @@ template <typename ElementType, typename List, typename NewFn, typename SetFn>
 auto QJniArrayBase::makeArray(List &&list, NewFn &&newArray, SetFn &&setRegion)
 {
     const int length = int(list.size());
-    QJniEnvironment env;
-    JNIEnv *jenv = env.jniEnv();
-    auto localArray = (jenv->*newArray)(length);
-    if (env.checkAndClearExceptions())
+    JNIEnv *env = QJniEnvironment::getJniEnv();
+    auto localArray = (env->*newArray)(length);
+    if (QJniEnvironment::checkAndClearExceptions(env))
         return QJniArray<ElementType>();
 
     // can't use static_cast here because we have signed/unsigned mismatches
     if (length) {
-        (jenv->*setRegion)(localArray, 0, length,
-                           reinterpret_cast<const ElementType *>(std::as_const(list).data()));
+        (env->*setRegion)(localArray, 0, length,
+                          reinterpret_cast<const ElementType *>(std::as_const(list).data()));
     }
     return QJniArray<ElementType>(localArray);
 };
@@ -299,7 +298,7 @@ auto QJniArrayBase::makeObjectArray(List &&list)
     if (list.isEmpty())
         return QJniArray<jobject>();
 
-    QJniEnvironment env;
+    JNIEnv *env = QJniEnvironment::getJniEnv();
     const int length = int(list.size());
 
     // this assumes that all objects in the list have the same class
@@ -309,7 +308,7 @@ auto QJniArrayBase::makeObjectArray(List &&list)
     else
         elementClass = env->GetObjectClass(list.first());
     auto localArray = env->NewObjectArray(length, elementClass, nullptr);
-    if (env.checkAndClearExceptions())
+    if (QJniEnvironment::checkAndClearExceptions(env))
         return QJniArray<jobject>();
     for (int i = 0; i < length; ++i) {
         jobject object;
diff --git a/src/corelib/kernel/qjnienvironment.cpp b/src/corelib/kernel/qjnienvironment.cpp
index 8d4e386a68..5afa1a6484 100644
--- a/src/corelib/kernel/qjnienvironment.cpp
+++ b/src/corelib/kernel/qjnienvironment.cpp
@@ -53,10 +53,20 @@ Q_GLOBAL_STATIC(QThreadStorage<QJniEnvironmentPrivateTLS *>, jniEnvTLS)
 QJniEnvironment::QJniEnvironment()
     : d(new QJniEnvironmentPrivate{})
 {
+    d->jniEnv = getJniEnv();
+}
+
+/*!
+    Returns the JNIEnv pointer for the current thread.
+
+    The current thread will be attached to the Java VM.
+*/
+JNIEnv *QJniEnvironment::getJniEnv()
+{
+    JNIEnv *jniEnv = nullptr;
+
     JavaVM *vm = QtAndroidPrivate::javaVM();
-    const jint ret = vm->GetEnv((void**)&d->jniEnv, JNI_VERSION_1_6);
-    if (ret == JNI_OK) // Already attached
-        return;
+    const jint ret = vm->GetEnv((void**)&jniEnv, JNI_VERSION_1_6);
 
     if (ret == JNI_EDETACHED) { // We need to (re-)attach
         const QByteArray threadName = QThread::currentThread()->objectName().toUtf8();
@@ -64,12 +74,12 @@ QJniEnvironment::QJniEnvironment()
                                   threadName.isEmpty() ? "QtThread" : threadName.constData(),
                                   nullptr
                                 };
-        if (vm->AttachCurrentThread(&d->jniEnv, &args) != JNI_OK)
-            return;
-
-        if (!jniEnvTLS->hasLocalData()) // If we attached the thread we own it.
-            jniEnvTLS->setLocalData(new QJniEnvironmentPrivateTLS);
+        if (vm->AttachCurrentThread(&jniEnv, &args) == JNI_OK) {
+            if (!jniEnvTLS->hasLocalData()) // If we attached the thread we own it.
+                jniEnvTLS->setLocalData(new QJniEnvironmentPrivateTLS);
+        }
     }
+    return jniEnv;
 }
 
 /*!
diff --git a/src/corelib/kernel/qjnienvironment.h b/src/corelib/kernel/qjnienvironment.h
index db41e1d4ab..f0377751f7 100644
--- a/src/corelib/kernel/qjnienvironment.h
+++ b/src/corelib/kernel/qjnienvironment.h
@@ -78,6 +78,8 @@ public:
     bool checkAndClearExceptions(OutputMode outputMode = OutputMode::Verbose);
     static bool checkAndClearExceptions(JNIEnv *env, OutputMode outputMode = OutputMode::Verbose);
 
+    static JNIEnv *getJniEnv();
+
 private:
     Q_DISABLE_COPY_MOVE(QJniEnvironment)
     QScopedPointer<QJniEnvironmentPrivate> d;
diff --git a/src/corelib/kernel/qjniobject.cpp b/src/corelib/kernel/qjniobject.cpp
index dec7163359..5c4114f1ce 100644
--- a/src/corelib/kernel/qjniobject.cpp
+++ b/src/corelib/kernel/qjniobject.cpp
@@ -24,7 +24,7 @@ using namespace Qt::StringLiterals;
     \brief A convenience wrapper around the Java Native Interface (JNI).
 
     The QJniObject class wraps a reference to a Java object, ensuring it isn't
-    gargage-collected and providing access to most \c JNIEnv method calls
+    garbage-collected and providing access to most \c JNIEnv method calls
     (member, static) and fields (setter, getter).  It eliminates much
     boiler-plate that would normally be needed, with direct JNI access, for
     every operation, including exception-handling.
@@ -281,20 +281,17 @@ using namespace Qt::StringLiterals;
 class QJniObjectPrivate
 {
 public:
-    // This is safe and necessary - the JNIEnv is attached to the current thread
-    // and the pointer remains valid for as long as the thread is alive. And if the
-    // QJniObject outlives the thread that it lives in, and gets called for
-    // anything other than destruction, then we have a data race anyway.
-    // And it's necessary, because the QJniEnvironment destructor calls
-    // checkAndClearExceptions, and since we have QJniObjects that get destroyed from
-    // a different thread than the one "owning" it, this triggers JNI assertions.
+    // The JNIEnv is attached to the current thread, and the pointer remains
+    // valid for as long as the thread is alive. If the QJniObject were to
+    // outlive the thread that it lives in, and gets called for anything other
+    // than destruction, then we have a data race anyway.
     QJniObjectPrivate()
-        : m_env(QJniEnvironment().jniEnv())
+        : m_env(QJniEnvironment::getJniEnv())
     {
     }
     ~QJniObjectPrivate() {
         // use the environment of the current thread here
-        QJniEnvironment env;
+        JNIEnv *env = QJniEnvironment::getJniEnv();
         if (m_jobject)
             env->DeleteGlobalRef(m_jobject);
         if (m_jclass && m_own_jclass)
@@ -1035,16 +1032,16 @@ QJniObject QJniObject::callObjectMethod(const char *methodName, const char *sign
 QJniObject QJniObject::callStaticObjectMethod(const char *className, const char *methodName,
                                               const char *signature, ...)
 {
-    QJniEnvironment env;
-    jclass clazz = QJniObject::loadClass(className, env.jniEnv());
+    JNIEnv *env = QJniEnvironment::getJniEnv();
+    jclass clazz = QJniObject::loadClass(className, env);
     if (clazz) {
-        jmethodID id = QJniObject::getCachedMethodID(env.jniEnv(), clazz,
+        jmethodID id = QJniObject::getCachedMethodID(env, clazz,
                                          className,
                                          methodName, signature, true);
         if (id) {
             va_list args;
             va_start(args, signature);
-            QJniObject res = getCleanJniObject(env->CallStaticObjectMethodV(clazz, id, args), env.jniEnv());
+            QJniObject res = getCleanJniObject(env->CallStaticObjectMethodV(clazz, id, args), env);
             va_end(args);
             return res;
         }
@@ -1232,18 +1229,18 @@ QJniObject QJniObject::getStaticObjectField(const char *className,
                                             const char *fieldName,
                                             const char *signature)
 {
-    QJniEnvironment env;
-    jclass clazz = QJniObject::loadClass(className, env.jniEnv());
+    JNIEnv *env = QJniEnvironment::getJniEnv();
+    jclass clazz = QJniObject::loadClass(className, env);
     if (!clazz)
         return QJniObject();
-    jfieldID id = QJniObject::getCachedFieldID(env.jniEnv(), clazz,
+    jfieldID id = QJniObject::getCachedFieldID(env, clazz,
                                    className,
                                    fieldName,
                                    signature, true);
     if (!id)
         return QJniObject();
 
-    return getCleanJniObject(env->GetStaticObjectField(clazz, id), env.jniEnv());
+    return getCleanJniObject(env->GetStaticObjectField(clazz, id), env);
 }
 
 /*!
@@ -1261,12 +1258,9 @@ QJniObject QJniObject::getStaticObjectField(const char *className,
 QJniObject QJniObject::getStaticObjectField(jclass clazz, const char *fieldName,
                                             const char *signature)
 {
-    QJniEnvironment env;
-    jfieldID id = getFieldID(env.jniEnv(), clazz, fieldName, signature, true);
-    if (!id)
-        return QJniObject();
-
-    return getCleanJniObject(env->GetStaticObjectField(clazz, id), env.jniEnv());
+    JNIEnv *env = QJniEnvironment::getJniEnv();
+    jfieldID id = getFieldID(env, clazz, fieldName, signature, true);
+    return getCleanJniObject(env->GetStaticObjectField(clazz, id), env);
 }
 
 /*!
diff --git a/src/corelib/kernel/qjniobject.h b/src/corelib/kernel/qjniobject.h
index b088ae1c63..2c353ad137 100644
--- a/src/corelib/kernel/qjniobject.h
+++ b/src/corelib/kernel/qjniobject.h
@@ -47,7 +47,7 @@ class Q_CORE_EXPORT QJniObject
         JNIEnv *jniEnv() const
         {
             if (!env)
-                env = QJniEnvironment().jniEnv();
+                env = QJniEnvironment::getJniEnv();
             return env;
         }
         bool checkAndClearExceptions()
@@ -176,16 +176,16 @@ public:
     template <typename Ret, typename ...Args>
     static auto callStaticMethod(const char *className, const char *methodName, const char *signature, Args &&...args)
     {
-        QJniEnvironment env;
-        jclass clazz = QJniObject::loadClass(className, env.jniEnv());
+        JNIEnv *env = QJniEnvironment::getJniEnv();
+        jclass clazz = QJniObject::loadClass(className, env);
         return callStaticMethod<Ret>(clazz, methodName, signature, std::forward<Args>(args)...);
     }
 
     template <typename Ret, typename ...Args>
     static auto callStaticMethod(jclass clazz, const char *methodName, const char *signature, Args &&...args)
     {
-        QJniEnvironment env;
-        jmethodID id = clazz ? getMethodID(env.jniEnv(), clazz, methodName, signature, true)
+        JNIEnv *env = QJniEnvironment::getJniEnv();
+        jmethodID id = clazz ? getMethodID(env, clazz, methodName, signature, true)
                              : 0;
         return callStaticMethod<Ret>(clazz, id, std::forward<Args>(args)...);
     }
@@ -228,9 +228,9 @@ public:
     >
     static auto callStaticMethod(const char *className, const char *methodName, Args &&...args)
     {
-        QJniEnvironment env;
-        jclass clazz = QJniObject::loadClass(className, env.jniEnv());
-        const jmethodID id = clazz ? getMethodID(env.jniEnv(), clazz, methodName,
+        JNIEnv *env = QJniEnvironment::getJniEnv();
+        jclass clazz = QJniObject::loadClass(className, env);
+        const jmethodID id = clazz ? getMethodID(env, clazz, methodName,
                                          QtJniTypes::methodSignature<Ret, Args...>().data(), true)
                                    : 0;
         return callStaticMethod<Ret>(clazz, id, std::forward<Args>(args)...);
@@ -253,10 +253,10 @@ public:
     >
     static auto callStaticMethod(const char *methodName, Args &&...args)
     {
-        QJniEnvironment env;
+        JNIEnv *env = QJniEnvironment::getJniEnv();
         const jclass clazz = QJniObject::loadClass(QtJniTypes::Traits<Klass>::className().data(),
-                                                   env.jniEnv());
-        const jmethodID id = clazz ? getMethodID(env.jniEnv(), clazz, methodName,
+                                                   env);
+        const jmethodID id = clazz ? getMethodID(env, clazz, methodName,
                                          QtJniTypes::methodSignature<Ret, Args...>().data(), true)
                                    : 0;
         return callStaticMethod<Ret>(clazz, id, std::forward<Args>(args)...);
@@ -474,17 +474,17 @@ public:
     static void setStaticField(const char *className, const char *fieldName,
                                const char *signature, T value)
     {
-        QJniEnvironment env;
-        jclass clazz = QJniObject::loadClass(className, env.jniEnv());
+        JNIEnv *env = QJniEnvironment::getJniEnv();
+        jclass clazz = QJniObject::loadClass(className, env);
 
         if (!clazz)
             return;
 
-        jfieldID id = getCachedFieldID(env.jniEnv(), clazz, className, fieldName,
+        jfieldID id = getCachedFieldID(env, clazz, className, fieldName,
                                        signature, true);
         if (id) {
-            setStaticFieldForType<T>(env.jniEnv(), clazz, id, value);
-            env.checkAndClearExceptions();
+            setStaticFieldForType<T>(env, clazz, id, value);
+            QJniEnvironment::checkAndClearExceptions(env);
         }
     }
 
@@ -496,12 +496,12 @@ public:
     static void setStaticField(jclass clazz, const char *fieldName,
                                const char *signature, T value)
     {
-        QJniEnvironment env;
-        jfieldID id = getFieldID(env.jniEnv(), clazz, fieldName, signature, true);
+        JNIEnv *env = QJniEnvironment::getJniEnv();
+        jfieldID id = getFieldID(env, clazz, fieldName, signature, true);
 
         if (id) {
-            setStaticFieldForType<T>(env.jniEnv(), clazz, id, value);
-            env.checkAndClearExceptions();
+            setStaticFieldForType<T>(env, clazz, id, value);
+            QJniEnvironment::checkAndClearExceptions(env);
         }
     }