From f9408cc81c7d3cb3fa212005fb30cd8318ebf247 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Christian=20Str=C3=B8mme?= Date: Fri, 21 Nov 2014 11:33:09 +0100 Subject: Android: protect global jni cache. This fixes a issue that has been neglected for a while, namely, that the access to the global jni caches where not sufficiently protected for concurrent usage. This change also fixes an issue with the thread-name storage. Task-number: QTBUG-42755 Change-Id: I22f95ae7f44d1f6a13e289e52b050d98ccb9fb28 Reviewed-by: Eskil Abrahamsen Blomfeldt --- src/corelib/kernel/qjni.cpp | 83 +++++++++++++++++++++++++++++++-------------- 1 file changed, 58 insertions(+), 25 deletions(-) (limited to 'src') diff --git a/src/corelib/kernel/qjni.cpp b/src/corelib/kernel/qjni.cpp index b179323fdc..9d74fd69de 100644 --- a/src/corelib/kernel/qjni.cpp +++ b/src/corelib/kernel/qjni.cpp @@ -37,6 +37,7 @@ #include #include #include +#include QT_BEGIN_NAMESPACE @@ -45,11 +46,6 @@ static inline QString keyBase() return QStringLiteral("%1%2%3"); } -static inline QByteArray threadBaseName() -{ - return QByteArrayLiteral("QtThread-"); -} - static QString qt_convertJString(jstring string) { QJNIEnvironmentPrivate env; @@ -74,6 +70,7 @@ static inline bool exceptionCheckAndClear(JNIEnv *env) typedef QHash JClassHash; Q_GLOBAL_STATIC(JClassHash, cachedClasses) +Q_GLOBAL_STATIC(QReadWriteLock, cachedClassesLock) static QString toDotEncodedClassName(const char *className) { @@ -82,8 +79,9 @@ static QString toDotEncodedClassName(const char *className) static jclass getCachedClass(const QString &classDotEnc, bool *isCached = 0) { - QHash::iterator it = cachedClasses->find(classDotEnc); - const bool found = (it != cachedClasses->end()); + QReadLocker locker(cachedClassesLock); + const QHash::const_iterator &it = cachedClasses->constFind(classDotEnc); + const bool found = (it != cachedClasses->constEnd()); if (isCached != 0) *isCached = found; @@ -102,6 +100,12 @@ static jclass loadClassDotEnc(const QString &classDotEnc, JNIEnv *env) if (!classLoader.isValid()) return 0; + QWriteLocker locker(cachedClassesLock); + // did we lose the race? + const QHash::const_iterator &it = cachedClasses->constFind(classDotEnc); + if (it != cachedClasses->constEnd()) + return it.value(); + QJNIObjectPrivate stringName = QJNIObjectPrivate::fromString(classDotEnc); QJNIObjectPrivate classObject = classLoader.callObjectMethod("loadClass", "(Ljava/lang/String;)Ljava/lang/Class;", @@ -121,6 +125,7 @@ inline static jclass loadClass(const char *className, JNIEnv *env) typedef QHash JMethodIDHash; Q_GLOBAL_STATIC(JMethodIDHash, cachedMethodID) +Q_GLOBAL_STATIC(QReadWriteLock, cachedMethodIDLock) static jmethodID getCachedMethodID(JNIEnv *env, jclass clazz, @@ -128,11 +133,24 @@ static jmethodID getCachedMethodID(JNIEnv *env, const char *sig, bool isStatic = false) { - jmethodID id = 0; // TODO: We need to use something else then the ref. from clazz to avoid collisions. - QString key = keyBase().arg(size_t(clazz)).arg(QLatin1String(name)).arg(QLatin1String(sig)); - QHash::iterator it = cachedMethodID->find(key); - if (it == cachedMethodID->end()) { + const QString key = keyBase().arg(size_t(clazz)).arg(QLatin1String(name)).arg(QLatin1String(sig)); + QHash::const_iterator it; + + { + QReadLocker locker(cachedMethodIDLock); + it = cachedMethodID->constFind(key); + if (it != cachedMethodID->constEnd()) + return it.value(); + } + + { + QWriteLocker locker(cachedMethodIDLock); + it = cachedMethodID->constFind(key); + if (it != cachedMethodID->constEnd()) + return it.value(); + + jmethodID id = 0; if (isStatic) id = env->GetStaticMethodID(clazz, name, sig); else @@ -142,14 +160,13 @@ static jmethodID getCachedMethodID(JNIEnv *env, id = 0; cachedMethodID->insert(key, id); - } else { - id = it.value(); + return id; } - return id; } typedef QHash JFieldIDHash; Q_GLOBAL_STATIC(JFieldIDHash, cachedFieldID) +Q_GLOBAL_STATIC(QReadWriteLock, cachedFieldIDLock) static jfieldID getCachedFieldID(JNIEnv *env, jclass clazz, @@ -157,10 +174,23 @@ static jfieldID getCachedFieldID(JNIEnv *env, const char *sig, bool isStatic = false) { - jfieldID id = 0; - QString key = keyBase().arg(size_t(clazz)).arg(QLatin1String(name)).arg(QLatin1String(sig)); - QHash::iterator it = cachedFieldID->find(key); - if (it == cachedFieldID->end()) { + const QString key = keyBase().arg(size_t(clazz)).arg(QLatin1String(name)).arg(QLatin1String(sig)); + QHash::const_iterator it; + + { + QReadLocker locker(cachedFieldIDLock); + it = cachedFieldID->constFind(key); + if (it != cachedFieldID->constEnd()) + return it.value(); + } + + { + QWriteLocker locker(cachedFieldIDLock); + it = cachedFieldID->constFind(key); + if (it != cachedFieldID->constEnd()) + return it.value(); + + jfieldID id = 0; if (isStatic) id = env->GetStaticFieldID(clazz, name, sig); else @@ -170,10 +200,8 @@ static jfieldID getCachedFieldID(JNIEnv *env, id = 0; cachedFieldID->insert(key, id); - } else { - id = it.value(); + return id; } - return id; } class QJNIEnvironmentPrivateTLS @@ -187,14 +215,14 @@ public: Q_GLOBAL_STATIC(QThreadStorage, jniEnvTLS) +static const char qJniThreadName[] = "QtThread"; + QJNIEnvironmentPrivate::QJNIEnvironmentPrivate() : jniEnv(0) { JavaVM *vm = QtAndroidPrivate::javaVM(); if (vm->GetEnv((void**)&jniEnv, JNI_VERSION_1_6) == JNI_EDETACHED) { - const qulonglong id = reinterpret_cast(QThread::currentThreadId()); - const QByteArray threadName = threadBaseName() + QByteArray::number(id); - JavaVMAttachArgs args = { JNI_VERSION_1_6, threadName, Q_NULLPTR }; + JavaVMAttachArgs args = { JNI_VERSION_1_6, qJniThreadName, Q_NULLPTR }; if (vm->AttachCurrentThread(&jniEnv, &args) != JNI_OK) return; } @@ -223,6 +251,12 @@ jclass QJNIEnvironmentPrivate::findClass(const char *className, JNIEnv *env) return clazz; if (env != 0) { // We got an env. pointer (We expect this to be the right env. and call FindClass()) + QWriteLocker locker(cachedClassesLock); + const QHash::const_iterator &it = cachedClasses->constFind(classDotEnc); + // Did we lose the race? + if (it != cachedClasses->constEnd()) + return it.value(); + jclass fclazz = env->FindClass(className); if (!exceptionCheckAndClear(env)) { clazz = static_cast(env->NewGlobalRef(fclazz)); @@ -400,7 +434,6 @@ QJNIObjectPrivate::QJNIObjectPrivate(jobject obj) d->m_jclass = static_cast(env->NewGlobalRef(objectClass)); env->DeleteLocalRef(objectClass); } - template <> void QJNIObjectPrivate::callMethodV(const char *methodName, const char *sig, va_list args) const { -- cgit v1.2.3