path: root/src/corelib/kernel/qjnihelpers.cpp
diff options
authorMarc Mutz <>2017-02-14 17:04:55 +0100
committerMarc Mutz <>2017-02-15 11:23:28 +0000
commit39820cf8c38f527e178913192d428085af4327b4 (patch)
treeabe90d316ebe2e2855951f97c4979fc2384a7281 /src/corelib/kernel/qjnihelpers.cpp
parenta05116e6f1f87027eb90a24da4895577f596f3c9 (diff)
QJNIHelper: fix a potential race in sendRequestPermissionsResult()
The code obtained an iterator into a QHash under mutex protection, then dropped the lock, dereferenced the iterator several times and only retook the lock to erase the element from the QHash. This is very smelly. QHash provides no official iterator validity guarantees, and the container isn't const, either (which would imply thread-safety). In particular, the dereference into the container outside the critical section is cause for concerns. Simplify the code, removing any doubts about its race-freedom, by taking the payload item out of the hash before dropping the lock, and using only the local strong reference in the remainder of the function. The only other references to g_pendingPermissionRequests are insertions with unique-by-construction keys in QtAndroidPrivate's requestPermissions(), so there was no reason to keep the item in the hash for the whole duration of the sendRequestPermissionsResult() call. Change-Id: I39fe0803b13b3046d1f0fd9c8e96c531406d57da Reviewed-by: Olivier Goffart (Woboq GmbH) <> Reviewed-by: BogDan Vatra <>
Diffstat (limited to 'src/corelib/kernel/qjnihelpers.cpp')
1 files changed, 4 insertions, 5 deletions
diff --git a/src/corelib/kernel/qjnihelpers.cpp b/src/corelib/kernel/qjnihelpers.cpp
index 1ad6dfd44f..028fb1256e 100644
--- a/src/corelib/kernel/qjnihelpers.cpp
+++ b/src/corelib/kernel/qjnihelpers.cpp
@@ -119,9 +119,11 @@ static void sendRequestPermissionsResult(JNIEnv *env, jobject /*obj*/, jint requ
// show an error or something ?
+ auto request = std::move(*it);
+ g_pendingPermissionRequests->erase(it);
- Qt::ConnectionType connection = QThread::currentThread() == it.value()->thread() ? Qt::DirectConnection : Qt::BlockingQueuedConnection;
+ Qt::ConnectionType connection = QThread::currentThread() == request->thread() ? Qt::DirectConnection : Qt::BlockingQueuedConnection;
QtAndroidPrivate::PermissionsHash hash;
const int size = env->GetArrayLength(permissions);
std::unique_ptr<jint[]> results(new jint[size]);
@@ -133,10 +135,7 @@ static void sendRequestPermissionsResult(JNIEnv *env, jobject /*obj*/, jint requ
hash[permission] = value;
- QMetaObject::invokeMethod(it.value().data(), "sendResult", connection, Q_ARG(QtAndroidPrivate::PermissionsHash, hash));
- locker.relock();
- g_pendingPermissionRequests->erase(it);
+ QMetaObject::invokeMethod(, "sendResult", connection, Q_ARG(QtAndroidPrivate::PermissionsHash, hash));
static jboolean dispatchGenericMotionEvent(JNIEnv *, jclass, jobject event)