1berry/qtbase
1
0
Fork 0
Code Issues Pull Requests Actions Packages Projects Releases Wiki Activity

JNI: Tolerate threading mismatches and improve diagnostics

Browse Source 
Amend 944200b5a9705a7617f82cdaf5caf8932380aba4. We have code paths
in Qt that result in a global QJniObject being called from multiple
threads (QtActivity instance, possibly the QAndroidSystemLocale),
so we need to tolerate that until we understand better under which
circumstances this should be allowed, and what tools we can use to
control this better.

Don't instantiate QJniEnvironment to check the JNIEnv, but use raw
JNI calls so that we don't implicitly attach a thread to the JVM
when checking. Use categorized logging to emit log output if we
have an environment/threading mismatch, including the name of the
class we are trying to call, and fall-back to the thread-local JNI
environment pointer instead of the one stored in the QJniObject's
private.

Also, give the threads that we attach to the JVM the name of the
QThread instances if we can, resulting in better logcat output on
the device.

This still results in less overhead than the code had prior to
944200b5a9705a7617f82cdaf5caf8932380aba4, as we don't instantiate
multiple temporary QJniEnvironment objects on the stack for each
call.

Change-Id: I98b92edd29162dccfc5c34e6261d4de219b59531
Reviewed-by: Fabian Kosmale <fabian.kosmale@qt.io>
Reviewed-by: Ivan Solovev <ivan.solovev@qt.io>
Reviewed-by: Sami Shalayel <sami.shalayel@qt.io>
This commit is contained in:
Volker Hilsheimer 2023-10-25 17:47:02 +02:00
parent f419a8d67f
commit 84e70976f3
2 changed files with 65 additions and 22 deletions
Show Stats Download Patch File Download Diff File Expand all files Collapse all files

8
src/corelib/kernel/qjnienvironment.cpp
View File

@ -29,8 +29,6 @@ QT_BEGIN_NAMESPACE
It has not been tested for other platforms.
*/
static const char qJniThreadName[] = "QtThread";
class QJniEnvironmentPrivate
{
public:
@ -61,7 +59,11 @@ QJniEnvironment::QJniEnvironment()
return;
if (ret == JNI_EDETACHED) { // We need to (re-)attach
JavaVMAttachArgs args = { JNI_VERSION_1_6, qJniThreadName, nullptr };
const QByteArray threadName = QThread::currentThread()->objectName().toUtf8();
JavaVMAttachArgs args = { JNI_VERSION_1_6,
threadName.isEmpty() ? "QtThread" : threadName.constData(),
nullptr
};
if (vm->AttachCurrentThread(&d->jniEnv, &args) != JNI_OK)
return;

79
src/corelib/kernel/qjniobject.cpp
View File

@ -8,9 +8,13 @@
#include <QtCore/qbytearray.h>
#include <QtCore/qhash.h>
#include <QtCore/qreadwritelock.h>
#include <QtCore/qloggingcategory.h>
QT_BEGIN_NAMESPACE
Q_LOGGING_CATEGORY(lcJniThreadCheck, "qt.core.jni.threadcheck")
using namespace Qt::StringLiterals;
/*!
@ -735,22 +739,70 @@ QJniObject::QJniObject(jobject object)
QJniObject::~QJniObject()
{}
namespace {
QByteArray getClassNameHelper(JNIEnv *env, const QJniObjectPrivate *d)
{
if (env->PushLocalFrame(3) != JNI_OK) // JVM out of memory
return QByteArray();
jmethodID mid = env->GetMethodID(d->m_jclass, "getClass", "()Ljava/lang/Class;");
jobject classObject = env->CallObjectMethod(d->m_jobject, mid);
jclass classObjectClass = env->GetObjectClass(classObject);
mid = env->GetMethodID(classObjectClass, "getName", "()Ljava/lang/String;");
jstring stringObject = static_cast<jstring>(env->CallObjectMethod(classObject, mid));
const jsize length = env->GetStringUTFLength(stringObject);
const char* nameString = env->GetStringUTFChars(stringObject, NULL);
const QByteArray result = QByteArray::fromRawData(nameString, length).replace('.', '/');
env->ReleaseStringUTFChars(stringObject, nameString);
env->PopLocalFrame(nullptr);
return result;
}
}
/*! \internal
While we can synchronize concurrent access to a QJniObject on the C++ side,
we cannot synchronize access across C++ and Java, so any concurrent access to
a QJniObject, except the most basic ref-counting operations (destructor and
assignment) is wrong. All calls must happen from the thread that created the
assignment) is dangerous, and all calls should happen from the thread that
created the Java object.
However, we have code in Qt that does that, so we use a cheap approach to check
for the condition and fall back to the thread-local JNI environment. We do not
re-attach to the thread though - that must have been done when constructing this
QJniObject.
*/
JNIEnv *QJniObject::jniEnv() const noexcept
{
Q_ASSERT_X([this]{
QJniEnvironment currentThreadEnv;
return currentThreadEnv.jniEnv() == d->jniEnv();
}(), __FUNCTION__, "QJniEnvironment mismatch, probably accessing this Java object"
" from the wrong thread!");
return d->jniEnv();
JNIEnv *env = nullptr;
const bool threadCheck = [this, &env]{
JavaVM *vm = QtAndroidPrivate::javaVM();
const jint ret = vm->GetEnv((void**)&env, JNI_VERSION_1_6);
if (ret == JNI_EDETACHED) {
qCCritical(lcJniThreadCheck) << "The JNI Environment has been detached from its Java "
"thread!";
return false;
} else if (ret != JNI_OK) {
qCCritical(lcJniThreadCheck) << "An error occurred while acquiring JNI environment!";
return false;
}
if (env != d->jniEnv()) {
qCCritical(lcJniThreadCheck) << "JNI Environment mismatch - probably accessing this "
"Java object from the wrong thread!";
return false;
}
return true;
}();
if (!threadCheck) {
if (!env) {
qFatal() << "No JNI environment attached to" << QThread::currentThread();
} else {
qCCritical(lcJniThreadCheck) << "JNI threading error when calling object of type"
<< getClassNameHelper(env, d.get()) << "from thread"
<< QThread::currentThread();
}
}
return env;
}
/*!
@ -806,18 +858,7 @@ QByteArray QJniObject::className() const
{
if (d->m_className.isEmpty() && d->m_jclass && d->m_jobject) {
JNIEnv *env = jniEnv();
if (env->PushLocalFrame(3) != JNI_OK) // JVM out of memory
return d->m_className;
jmethodID mid = env->GetMethodID(d->m_jclass, "getClass", "()Ljava/lang/Class;");
jobject classObject = env->CallObjectMethod(d->m_jobject, mid);
jclass classObjectClass = env->GetObjectClass(classObject);
mid = env->GetMethodID(classObjectClass, "getName", "()Ljava/lang/String;");
jstring stringObject = static_cast<jstring>(env->CallObjectMethod(classObject, mid));
const jsize length = env->GetStringUTFLength(stringObject);
const char* nameString = env->GetStringUTFChars(stringObject, NULL);
d->m_className = QByteArray::fromRawData(nameString, length).replace('.', '/');
env->ReleaseStringUTFChars(stringObject, nameString);
env->PopLocalFrame(nullptr);
d->m_className = getClassNameHelper(env, d.get());
}
return d->m_className;
}