diff --git a/tests/tst_ModelQuery.cpp b/tests/tst_ModelQuery.cpp index bf6e9d2..e440ba3 100644 --- a/tests/tst_ModelQuery.cpp +++ b/tests/tst_ModelQuery.cpp @@ -1,6 +1,11 @@ #include +#include +#include +#include #include +#include +#include #include #include @@ -46,6 +51,63 @@ private slots: QCOMPARE(utils.indexOf(model, "balance", 4), 3); QCOMPARE(utils.indexOf(model, "balance", "4"), 3); } + + void testGetDoesNotHandSubmodelToJsGc() + { + QQmlEngine engine; + + ListModelWrapper model(engine, R"([ + { name: "A", balances: [ { chainId: 1 }, { chainId: 2 } ] } + ])"); + + ModelQuery utils; + + // The submodel wrapper is cached, so the direct call returns the same + // object the JS call below returns. + QPointer subModel = utils.get(model, 0, "balances").value(); + QVERIFY(subModel); + + // Fetch through the JS engine: a QVariant(QObject*) returned from a + // Q_INVOKABLE is handed to the JS GC unless ownership is pinned. + engine.rootContext()->setContextProperty("modelQuery", &utils); + engine.rootContext()->setContextProperty("sourceModel", model.model()); + QQmlExpression expression(engine.rootContext(), nullptr, + QStringLiteral("modelQuery.get(sourceModel, 0, 'balances')")); + QVERIFY(!expression.evaluate().isNull()); + + QCOMPARE(QQmlEngine::objectOwnership(subModel), QQmlEngine::CppOwnership); + + // The source model owns the submodel; a GC pass must not delete it. + engine.collectGarbage(); + QCoreApplication::sendPostedEvents(nullptr, QEvent::DeferredDelete); + QVERIFY(subModel); + } + + void testGetRespectsExplicitJsOwnershipAndWarns() + { + QQmlEngine engine; + + ListModelWrapper model(engine, R"([ + { name: "A", balances: [ { chainId: 1 } ] } + ])"); + + ModelQuery utils; + + QObject *subModel = utils.get(model, 0, "balances").value(); + QVERIFY(subModel); + QQmlEngine::setObjectOwnership(subModel, QQmlEngine::JavaScriptOwnership); + + engine.rootContext()->setContextProperty("modelQuery", &utils); + engine.rootContext()->setContextProperty("sourceModel", model.model()); + QQmlExpression expression(engine.rootContext(), nullptr, + QStringLiteral("modelQuery.get(sourceModel, 0, 'balances')")); + + QTest::ignoreMessage(QtWarningMsg, + QRegularExpression(QStringLiteral("ModelQuery::get:.*JS-owned"))); + QVERIFY(!expression.evaluate().isNull()); + + QCOMPARE(QQmlEngine::objectOwnership(subModel), QQmlEngine::JavaScriptOwnership); + } }; QTEST_MAIN(TestModelQuery) diff --git a/toolkit/src/modelquery.cpp b/toolkit/src/modelquery.cpp index a6c48f9..f69f919 100644 --- a/toolkit/src/modelquery.cpp +++ b/toolkit/src/modelquery.cpp @@ -2,6 +2,7 @@ #include #include +#include namespace qtmt { @@ -88,8 +89,26 @@ QVariantMap ModelQuery::get(QAbstractItemModel *model, int row) const QVariant ModelQuery::get(QAbstractItemModel *model, int row, const QString &roleName) const { - if (auto role = roleByName(model, roleName); role != -1) - return model->data(model->index(row, 0), role); + if (auto role = roleByName(model, roleName); role != -1) { + QVariant value = model->data(model->index(row, 0), role); + // A QObject* returned to JS from a Q_INVOKABLE (even wrapped in a + // QVariant) is made destructible by the JS GC unless ownership is + // explicitly pinned. Role values (e.g. nested list models) stay owned + // by the source model — pin them so the GC cannot delete them. + // Objects already under JS ownership are left alone: pinning those + // would leak them, but the GC may then delete them while the model + // still refers to them, so surface it. + if (QObject *obj = value.value()) { + if (QQmlEngine::objectOwnership(obj) == QQmlEngine::JavaScriptOwnership) { + qWarning() << "ModelQuery::get: role" << roleName + << "returned JS-owned object" << obj + << "- not pinning; the JS GC may delete it while the model still uses it"; + } else { + QQmlEngine::setObjectOwnership(obj, QQmlEngine::CppOwnership); + } + } + return value; + } return {}; }