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

Android: Fix synchronization issue of QtAbstractItemModel and its proxy

Browse Source 
Java methods of QtAbstractItemModel may get called from Qt GUI thread or
Java main thread. Situation is the same with internal proxy methods,
where they may directly or indirectly called from these two thread.
At the same time, a chain of calls may happen from these threads, so
some functions may get called indirectly from a single caller thread.

In this change, we are adding a map of QObject* to QRecursiveMutux
where the QObject is a QAbstractItemModel, either the proxy instance
or a reference to an underlaying QAbstractItemModel when the model
is implemented in C++.

Each call to any function locks the QRecursiveMutux instance associated
with the QAbstractItemModel instance. A connection from
QObject::destroyed to a lamda, helps to remove the map entry when
the model is destroyed.

Pick-to: 6.8
Fixes: QTBUG-127467
Change-Id: I18dcdc207e201b3883526648b4e672e5868ed8ab
Reviewed-by: Assam Boudjelthia <assam.boudjelthia@qt.io>
(cherry picked from commit bf76387507dde71063ebe484bce1e7d1b8e09bc2)
Reviewed-by: Qt Cherry-pick Bot <cherrypick_bot@qt-project.org>
This commit is contained in:
Soheil Armin 2024-11-01 11:59:07 +02:00 committed by Qt Cherry-pick Bot
parent be31cbfce8
commit 2b14b70b43
2 changed files with 40 additions and 27 deletions
Show Stats Download Patch File Download Diff File Expand all files Collapse all files

19
src/corelib/platform/android/qandroiditemmodelproxy.cpp
View File

@ -11,9 +11,13 @@ QT_BEGIN_NAMESPACE
using namespace QtJniTypes;
std::map<const QAbstractItemModel *, QRecursiveMutex> QAndroidItemModelProxy::s_mutexes =
std::map<const QAbstractItemModel *, QRecursiveMutex>{};
jint QAndroidItemModelProxy::columnCount(const QModelIndex &parent) const
{
Q_ASSERT(jInstance.isValid());
const QMutexLocker<QRecursiveMutex> lock = getMutexLocker(this);
auto parentIndex = QAndroidModelIndexProxy::jInstance(parent);
return jInstance.callMethod<jint>("columnCount", parentIndex);
}
@ -21,6 +25,7 @@ jint QAndroidItemModelProxy::columnCount(const QModelIndex &parent) const
bool QAndroidItemModelProxy::canFetchMore(const QModelIndex &parent) const
{
Q_ASSERT(jInstance.isValid());
const QMutexLocker<QRecursiveMutex> lock = getMutexLocker(this);
auto parentIndex = QAndroidModelIndexProxy::jInstance(parent);
return jInstance.callMethod<jboolean>("canFetchMore", parentIndex);
}
@ -33,6 +38,7 @@ bool QAndroidItemModelProxy::canFetchMoreDefault(const QModelIndex &parent) cons
QVariant QAndroidItemModelProxy::data(const QModelIndex &index, int role) const
{
Q_ASSERT(jInstance.isValid());
const QMutexLocker<QRecursiveMutex> lock = getMutexLocker(this);
auto jIndex = QAndroidModelIndexProxy::jInstance(index);
QJniObject jData = jInstance.callMethod<jobject>("data", jIndex, role);
return QAndroidTypeConverter::toQVariant(jData);
@ -41,6 +47,7 @@ QVariant QAndroidItemModelProxy::data(const QModelIndex &index, int role) const
QModelIndex QAndroidItemModelProxy::index(int row, int column, const QModelIndex &parent) const
{
Q_ASSERT(jInstance.isValid());
const QMutexLocker<QRecursiveMutex> lock = getMutexLocker(this);
JQtModelIndex jIndex = jInstance.callMethod<JQtModelIndex>(
"index", row, column, QAndroidModelIndexProxy::jInstance(parent));
return QAndroidModelIndexProxy::qInstance(jIndex);
@ -49,6 +56,7 @@ QModelIndex QAndroidItemModelProxy::index(int row, int column, const QModelIndex
QModelIndex QAndroidItemModelProxy::parent(const QModelIndex &index) const
{
Q_ASSERT(jInstance.isValid());
const QMutexLocker<QRecursiveMutex> lock = getMutexLocker(this);
auto jIndex = QAndroidModelIndexProxy::jInstance(index);
return QAndroidModelIndexProxy::qInstance(
@ -57,6 +65,7 @@ QModelIndex QAndroidItemModelProxy::parent(const QModelIndex &index) const
int QAndroidItemModelProxy::rowCount(const QModelIndex &parent) const
{
Q_ASSERT(jInstance.isValid());
const QMutexLocker<QRecursiveMutex> lock = getMutexLocker(this);
auto parentIndex = QAndroidModelIndexProxy::jInstance(parent);
return jInstance.callMethod<int>("rowCount", parentIndex);
@ -65,6 +74,7 @@ int QAndroidItemModelProxy::rowCount(const QModelIndex &parent) const
QHash<int, QByteArray> QAndroidItemModelProxy::roleNames() const
{
Q_ASSERT(jInstance.isValid());
const QMutexLocker<QRecursiveMutex> lock = getMutexLocker(this);
QHash<int, QByteArray> roleNames;
HashMap hashMap = jInstance.callMethod<HashMap>("roleNames");
@ -88,6 +98,7 @@ QHash<int, QByteArray> QAndroidItemModelProxy::defaultRoleNames() const
void QAndroidItemModelProxy::fetchMore(const QModelIndex &parent)
{
Q_ASSERT(jInstance.isValid());
const QMutexLocker<QRecursiveMutex> lock = getMutexLocker(this);
auto parentIndex = QAndroidModelIndexProxy::jInstance(parent);
jInstance.callMethod<void>("fetchMore", parentIndex);
}
@ -100,6 +111,7 @@ void QAndroidItemModelProxy::fetchMoreDefault(const QModelIndex &parent)
bool QAndroidItemModelProxy::hasChildren(const QModelIndex &parent) const
{
Q_ASSERT(jInstance.isValid());
const QMutexLocker<QRecursiveMutex> lock = getMutexLocker(this);
auto parentIndex = QAndroidModelIndexProxy::jInstance(parent);
return jInstance.callMethod<jboolean>("hasChildren", parentIndex);
}
@ -112,6 +124,7 @@ bool QAndroidItemModelProxy::hasChildrenDefault(const QModelIndex &parent) const
QModelIndex QAndroidItemModelProxy::sibling(int row, int column, const QModelIndex &parent) const
{
Q_ASSERT(jInstance.isValid());
const QMutexLocker<QRecursiveMutex> lock = getMutexLocker(this);
return QAndroidModelIndexProxy::qInstance(jInstance.callMethod<jobject>(
"sibling", row, column, QAndroidModelIndexProxy::jInstance(parent)));
}
@ -124,6 +137,7 @@ QModelIndex QAndroidItemModelProxy::siblingDefault(int row, int column, const QM
bool QAndroidItemModelProxy::setData(const QModelIndex &index, const QVariant &value, int role)
{
Q_ASSERT(jInstance.isValid());
const QMutexLocker<QRecursiveMutex> lock = getMutexLocker(this);
auto jIndex = QAndroidModelIndexProxy::jInstance(index);
auto jValue = QAndroidTypeConverter::toJavaObject(value, QJniEnvironment::getJniEnv());
return jInstance.callMethod<jboolean>("setData", jIndex, jValue, role);
@ -157,8 +171,10 @@ QAndroidItemModelProxy::createNativeProxy(QJniObject itemModel)
itemModel.callMethod<void>("setNativeReference", reinterpret_cast<jlong>(nativeProxy));
connect(nativeProxy, &QAndroidItemModelProxy::destroyed, nativeProxy, [](QObject *obj) {
auto proxy = qobject_cast<QAndroidItemModelProxy *>(obj);
if (proxy)
if (proxy) {
const QMutexLocker<QRecursiveMutex> lock = getMutexLocker(proxy);
proxy->jInstance.callMethod<void>("detachFromNative");
}
});
connect(nativeProxy, &QAndroidItemModelProxy::dataChanged, nativeProxy,
@ -167,6 +183,7 @@ QAndroidItemModelProxy::createNativeProxy(QJniObject itemModel)
auto proxy = qobject_cast<QAndroidItemModelProxy *>(nativeProxy);
if (proxy) {
QJniObject jInstance = proxy->jInstance;
const QMutexLocker<QRecursiveMutex> lock = getMutexLocker(proxy);
jInstance.callMethod<void>("handleDataChanged",
QAndroidModelIndexProxy::jInstance(topLeft),
QAndroidModelIndexProxy::jInstance(bottomRight),

48
src/corelib/platform/android/qandroiditemmodelproxy_p.h
View File

@ -22,6 +22,7 @@
#include <QtCore/qjnienvironment.h>
#include <QtCore/qjnitypes.h>
#include <QtCore/qthread.h>
#include <QtCore/qmutex.h>
QT_BEGIN_NAMESPACE
@ -67,7 +68,8 @@ public:
Q_ASSERT(jvmObject);
auto model = qobject_cast<QAndroidItemModelProxy *>(nativeInstance(jvmObject));
Q_ASSERT(model);
return safeCall(model, std::forward<Func>(func), std::forward<Args>(args)...);
const QMutexLocker<QRecursiveMutex> lock = getMutexLocker(model);
return std::invoke(std::forward<Func>(func), model, std::forward<Args>(args)...);
}
template <typename Func, typename... Args>
@ -76,7 +78,8 @@ public:
Q_ASSERT(jvmObject);
auto model = nativeInstance(jvmObject);
Q_ASSERT(model);
return safeCall(model, std::forward<Func>(func), std::forward<Args>(args)...);
const QMutexLocker<QRecursiveMutex> lock = getMutexLocker(model);
return std::invoke(std::forward<Func>(func), model, std::forward<Args>(args)...);
}
template <typename Func1, typename Func2, typename... Args>
@ -86,33 +89,13 @@ public:
Q_ASSERT(jvmObject);
auto nativeModel = nativeInstance(jvmObject);
auto nativeProxyModel = qobject_cast<QAndroidItemModelProxy *>(nativeModel);
const QMutexLocker<QRecursiveMutex> lock = getMutexLocker(nativeModel);
if (nativeProxyModel)
return safeCall(nativeProxyModel, std::forward<Func1>(defaultFunc),
std::forward<Args>(args)...);
return std::invoke(std::forward<Func1>(defaultFunc), nativeProxyModel,
std::forward<Args>(args)...);
else
return safeCall(nativeModel, std::forward<Func2>(func), std::forward<Args>(args)...);
}
template <typename Object, typename Func, typename... Args>
static auto safeCall(Object *object, Func &&func, Args &&...args)
{
using ReturnType = decltype(std::invoke(std::forward<Func>(func), object,
std::forward<Args>(args)...));
if constexpr (std::is_void_v<ReturnType>) {
QMetaObject::invokeMethod(object, std::forward<Func>(func), Qt::AutoConnection,
std::forward<Args>(args)...);
} else {
ReturnType returnValue;
const auto connectionType = object->thread() == QThread::currentThread()
? Qt::DirectConnection
: Qt::BlockingQueuedConnection;
QMetaObject::invokeMethod(object, std::forward<Func>(func), connectionType,
qReturnArg(returnValue), std::forward<Args>(args)...);
return returnValue;
}
return std::invoke(std::forward<Func2>(func), nativeModel, std::forward<Args>(args)...);
}
static jint jni_columnCount(JNIEnv *env, jobject object, JQtModelIndex parent);
@ -216,6 +199,19 @@ public:
static bool registerProxyNatives(QJniEnvironment &env);
private:
static std::map<const QAbstractItemModel *, QRecursiveMutex> s_mutexes;
Q_REQUIRED_RESULT static QMutexLocker<QRecursiveMutex>
getMutexLocker(const QAbstractItemModel *model)
{
auto [iter, inserted] = s_mutexes.try_emplace(model);
if (inserted)
QObject::connect(model, &QAbstractItemModel::destroyed, model, [](QObject *model) {
s_mutexes.erase(reinterpret_cast<QAbstractItemModel *>(model));
});
return QMutexLocker(&iter->second);
}
QJniObject jInstance;
friend class QAndroidModelIndexProxy;
};