AuroraMiddleware/qt5base-lts
1
0
Fork 0
Code Issues Pull Requests Packages Projects Releases Wiki Activity

QJNIHelper: fix a potential race in sendRequestPermissionsResult()

Browse Source 
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) <ogoffart@woboq.com>
Reviewed-by: BogDan Vatra <bogdan@kdab.com>
This commit is contained in:
Marc Mutz 2017-02-14 17:04:55 +01:00
parent a05116e6f1
commit 39820cf8c3
1 changed files with 4 additions and 5 deletions
Show Stats Download Patch File Download Diff File Expand all files Collapse all files

9
src/corelib/kernel/qjnihelpers.cpp
View File

@ -119,9 +119,11 @@ static void sendRequestPermissionsResult(JNIEnv *env, jobject /*obj*/, jint requ
// show an error or something ?
return;
}
auto request = std::move(*it);
g_pendingPermissionRequests->erase(it);
locker.unlock();
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
QtAndroidPrivate::PermissionsResult::Denied;
hash[permission] = value;
}
QMetaObject::invokeMethod(it.value().data(), "sendResult", connection, Q_ARG(QtAndroidPrivate::PermissionsHash, hash));
locker.relock();
g_pendingPermissionRequests->erase(it);
QMetaObject::invokeMethod(request.data(), "sendResult", connection, Q_ARG(QtAndroidPrivate::PermissionsHash, hash));
}
static jboolean dispatchGenericMotionEvent(JNIEnv *, jclass, jobject event)