From 9dc7a22b212c18215942b9a4bfa17bd16dd5151b Mon Sep 17 00:00:00 2001 From: Ulf Hermann Date: Thu, 19 Aug 2021 15:08:41 +0200 Subject: [PATCH] Provide a method to clear the singletons from a QQmlEngine This is handy for cases where you want to clearComponentCache(). Singletons may hold on to old types, and using them after clearComponentCache() can produce surprising results. Task-number: QTBUG-95788 Change-Id: I390ff78cf9be9c034573ae848c8ccefa6d6a8817 Reviewed-by: Fabian Kosmale Reviewed-by: Andrei Golubev --- src/qml/qml/qqml.cpp | 53 ++++-- src/qml/qml/qqml.h | 2 +- src/qml/qml/qqmlengine.cpp | 55 +++--- src/qml/qml/qqmlengine.h | 1 + src/qml/qml/qqmlengine_p.h | 19 +- src/qml/qml/qqmlprivate.h | 16 +- .../auto/qml/qqmlengine/data/QmlSingleton.qml | 6 + tests/auto/qml/qqmlengine/tst_qqmlengine.cpp | 167 ++++++++++++++---- 8 files changed, 253 insertions(+), 66 deletions(-) create mode 100644 tests/auto/qml/qqmlengine/data/QmlSingleton.qml diff --git a/src/qml/qml/qqml.cpp b/src/qml/qml/qqml.cpp index 866ba99211..c23293c708 100644 --- a/src/qml/qml/qqml.cpp +++ b/src/qml/qml/qqml.cpp @@ -310,30 +310,63 @@ int qmlTypeId(const char *uri, int versionMajor, int versionMinor, const char *q return QQmlMetaType::typeId(uri, QTypeRevision::fromVersion(versionMajor, versionMinor), qmlName); } +static bool checkSingletonInstance(QQmlEngine *engine, QObject *instance) +{ + if (!instance) { + QQmlError error; + error.setDescription(QStringLiteral("The registered singleton has already been deleted. " + "Ensure that it outlives the engine.")); + QQmlEnginePrivate::get(engine)->warning(engine, error); + return false; + } + + if (engine->thread() != instance->thread()) { + QQmlError error; + error.setDescription(QStringLiteral("Registered object must live in the same thread " + "as the engine it was registered with")); + QQmlEnginePrivate::get(engine)->warning(engine, error); + return false; + } + + return true; +} + // From qqmlprivate.h +#if QT_DEPRECATED_SINCE(6, 3) QObject *QQmlPrivate::SingletonFunctor::operator()(QQmlEngine *qeng, QJSEngine *) { - if (!m_object) { + if (!checkSingletonInstance(qeng, m_object)) + return nullptr; + + if (alreadyCalled) { QQmlError error; - error.setDescription(QLatin1String("The registered singleton has already been deleted. Ensure that it outlives the engine.")); + error.setDescription(QStringLiteral("Singleton registered by registerSingletonInstance " + "must only be accessed from one engine")); QQmlEnginePrivate::get(qeng)->warning(qeng, error); return nullptr; } - if (qeng->thread() != m_object->thread()) { - QQmlError error; - error.setDescription(QLatin1String("Registered object must live in the same thread as the engine it was registered with")); - QQmlEnginePrivate::get(qeng)->warning(qeng, error); + alreadyCalled = true; + QJSEngine::setObjectOwnership(m_object, QQmlEngine::CppOwnership); + return m_object; +}; +#endif + +QObject *QQmlPrivate::SingletonInstanceFunctor::operator()(QQmlEngine *qeng, QJSEngine *) +{ + if (!checkSingletonInstance(qeng, m_object)) return nullptr; - } - if (alreadyCalled) { + + if (!m_engine) { + m_engine = qeng; + QJSEngine::setObjectOwnership(m_object, QQmlEngine::CppOwnership); + } else if (m_engine != qeng) { QQmlError error; error.setDescription(QLatin1String("Singleton registered by registerSingletonInstance must only be accessed from one engine")); QQmlEnginePrivate::get(qeng)->warning(qeng, error); return nullptr; } - alreadyCalled = true; - qeng->setObjectOwnership(m_object, QQmlEngine::CppOwnership); + return m_object; }; diff --git a/src/qml/qml/qqml.h b/src/qml/qml/qqml.h index b635c2289e..472aa74af7 100644 --- a/src/qml/qml/qqml.h +++ b/src/qml/qml/qqml.h @@ -659,7 +659,7 @@ inline auto qmlRegisterSingletonInstance(const char *uri, int versionMajor, int const char *typeName, T *cppObject) -> typename std::enable_if::value, int>::type #endif { - QQmlPrivate::SingletonFunctor registrationFunctor; + QQmlPrivate::SingletonInstanceFunctor registrationFunctor; registrationFunctor.m_object = cppObject; return qmlRegisterSingletonType(uri, versionMajor, versionMinor, typeName, registrationFunctor); } diff --git a/src/qml/qml/qqmlengine.cpp b/src/qml/qml/qqmlengine.cpp index 13950b6356..ee1ac7bcc9 100644 --- a/src/qml/qml/qqmlengine.cpp +++ b/src/qml/qml/qqmlengine.cpp @@ -809,9 +809,7 @@ QQmlEngine::~QQmlEngine() // we do this here and not in the private dtor since otherwise a crash can // occur (if we are the QObject parent of the QObject singleton instance) // XXX TODO: performance -- store list of singleton types separately? - const QList singletonTypes = QQmlMetaType::qmlSingletonTypes(); - for (const QQmlType &currType : singletonTypes) - d->destroySingletonInstance(currType); + d->singletonInstances.clear(); delete d->rootContext; d->rootContext = nullptr; @@ -863,7 +861,7 @@ QQmlEngine::~QQmlEngine() As a general rule of thumb, make sure that no objects created from QML components are alive when you clear the component cache. - \sa trimComponentCache() + \sa trimComponentCache(), clearSingletons() */ void QQmlEngine::clearComponentCache() { @@ -891,6 +889,27 @@ void QQmlEngine::trimComponentCache() d->typeLoader.trimCache(); } +/*! + Clears all singletons the engine owns. + + This function drops all singleton instances, deleting any QObjects owned by + the engine among them. This is useful to make sure that no QML-created objects + are left before calling clearComponentCache(). + + QML properties holding QObject-based singleton instances become null if the + engine owns the singleton or retain their value if the engine doesn't own it. + The singletons are not automatically re-created by accessing existing + QML-created objects. Only when new components are instantiated, the singletons + are re-created. + + \sa clearComponentCache() + */ +void QQmlEngine::clearSingletons() +{ + Q_D(QQmlEngine); + d->singletonInstances.clear(); +} + /*! Returns the engine's root context. @@ -2094,10 +2113,19 @@ QJSValue QQmlEnginePrivate::singletonInstance(const QQmlType &type) // if this object can use a property cache, create it now QQmlData::ensurePropertyCache(q, o); + + // even though the object is defined in C++, qmlContext(obj) and qmlEngine(obj) + // should behave identically to QML singleton types. You can, however, manually + // assign a context; and clearSingletons() retains the contexts, in which case + // we don't want to see warnings about the object already having a context. + QQmlData *data = QQmlData::get(o, true); + if (!data->context) { + auto contextData = QQmlContextData::get(new QQmlContext(q->rootContext(), q)); + data->context = contextData.data(); + contextData->addOwnedObject(data); + } } - // even though the object is defined in C++, qmlContext(obj) and qmlEngine(obj) - // should behave identically to QML singleton types. - q->setContextForObject(o, new QQmlContext(q->rootContext(), q)); + value = q->newQObject(o); singletonInstances.convertAndInsert(v4engine(), type, &value); } else if (!siinfo->url.isEmpty()) { @@ -2117,19 +2145,6 @@ QJSValue QQmlEnginePrivate::singletonInstance(const QQmlType &type) return value; } -void QQmlEnginePrivate::destroySingletonInstance(const QQmlType &type) -{ - Q_ASSERT(type.isSingleton() || type.isCompositeSingleton()); - - QObject* o = singletonInstances.take(type).toQObject(); - if (o) { - QQmlData *ddata = QQmlData::get(o, false); - if (type.singletonInstanceInfo()->url.isEmpty() && ddata && ddata->indestructible && ddata->explicitIndestructibleSet) - return; - delete o; - } -} - bool QQmlEnginePrivate::isTypeLoaded(const QUrl &url) const { return typeLoader.isTypeLoaded(url); diff --git a/src/qml/qml/qqmlengine.h b/src/qml/qml/qqmlengine.h index 64e4a82b03..f38609a410 100644 --- a/src/qml/qml/qqmlengine.h +++ b/src/qml/qml/qqmlengine.h @@ -102,6 +102,7 @@ public: void clearComponentCache(); void trimComponentCache(); + void clearSingletons(); QStringList importPathList() const; void setImportPathList(const QStringList &paths); diff --git a/src/qml/qml/qqmlengine_p.h b/src/qml/qml/qqmlengine_p.h index 8da2ef9e77..8c1f5c225c 100644 --- a/src/qml/qml/qqmlengine_p.h +++ b/src/qml/qml/qqmlengine_p.h @@ -243,7 +243,6 @@ public: template T singletonInstance(const QQmlType &type); - void destroySingletonInstance(const QQmlType &type); void sendQuit(); void sendExit(int retCode = 0); @@ -303,6 +302,24 @@ private: insert(type, *value); } + void clear() { + for (auto it = constBegin(), end = constEnd(); it != end; ++it) { + QObject *instance = it.value().toQObject(); + if (!instance) + continue; + + if (it.key().singletonInstanceInfo()->url.isEmpty()) { + const QQmlData *ddata = QQmlData::get(instance, false); + if (ddata && ddata->indestructible && ddata->explicitIndestructibleSet) + continue; + } + + delete instance; + } + + QHash::clear(); + } + using QHash::value; using QHash::take; }; diff --git a/src/qml/qml/qqmlprivate.h b/src/qml/qml/qqmlprivate.h index aecf417245..4cd185aca4 100644 --- a/src/qml/qml/qqmlprivate.h +++ b/src/qml/qml/qqmlprivate.h @@ -721,12 +721,26 @@ namespace QQmlPrivate int Q_QML_EXPORT qmlregister(RegistrationType, void *); void Q_QML_EXPORT qmlunregister(RegistrationType, quintptr); + +#if QT_DEPRECATED_SINCE(6, 3) struct Q_QML_EXPORT SingletonFunctor + { + QT_DEPRECATED QObject *operator()(QQmlEngine *, QJSEngine *); + QPointer m_object; + bool alreadyCalled = false; + }; +#endif + + struct Q_QML_EXPORT SingletonInstanceFunctor { QObject *operator()(QQmlEngine *, QJSEngine *); QPointer m_object; - bool alreadyCalled = false; + + // Not a QPointer, so that you cannot assign it to a different + // engine when the first one is deleted. + // That would mess up the QML contexts. + QQmlEngine *m_engine = nullptr; }; static int indexOfOwnClassInfo(const QMetaObject *metaObject, const char *key, int startOffset = -1) diff --git a/tests/auto/qml/qqmlengine/data/QmlSingleton.qml b/tests/auto/qml/qqmlengine/data/QmlSingleton.qml new file mode 100644 index 0000000000..23f180cb67 --- /dev/null +++ b/tests/auto/qml/qqmlengine/data/QmlSingleton.qml @@ -0,0 +1,6 @@ +pragma Singleton +import QtQml + +QtObject { + objectName: "theSingleton" +} diff --git a/tests/auto/qml/qqmlengine/tst_qqmlengine.cpp b/tests/auto/qml/qqmlengine/tst_qqmlengine.cpp index e1dff63e03..87654deb44 100644 --- a/tests/auto/qml/qqmlengine/tst_qqmlengine.cpp +++ b/tests/auto/qml/qqmlengine/tst_qqmlengine.cpp @@ -65,6 +65,7 @@ private slots: void clearComponentCache(); void trimComponentCache(); void trimComponentCache_data(); + void clearSingletons(); void repeatedCompilation(); void failedCompilation(); void failedCompilation_data(); @@ -103,6 +104,40 @@ private: QTemporaryDir m_tempDir; }; +class ObjectCaller : public QObject +{ + Q_OBJECT + +signals: + void doubleReply(const double a); +}; + +class CppSingleton : public QObject { + Q_OBJECT +public: + CppSingleton() {} + + static QObject *create(QQmlEngine *qmlEngine, QJSEngine *jsEngine) + { + Q_UNUSED(qmlEngine); + Q_UNUSED(jsEngine); + return new CppSingleton(); + } +}; + +class JsSingleton : public QObject { + Q_OBJECT +public: + JsSingleton() {} + + static QJSValue create(QQmlEngine *qmlEngine, QJSEngine *jsEngine) + { + Q_UNUSED(qmlEngine); + QJSValue value = jsEngine->newQObject(new JsSingleton()); + return value; + } +}; + void tst_qqmlengine::initTestCase() { QVERIFY2(m_tempDir.isValid(), qPrintable(m_tempDir.errorString())); @@ -510,6 +545,105 @@ void tst_qqmlengine::trimComponentCache_data() QTest::newRow("ScriptComponent") << "testScriptComponent.qml"; } +static QJSValue createValueSingleton(QQmlEngine *, QJSEngine *) { return 13u; } + +void tst_qqmlengine::clearSingletons() +{ + ObjectCaller objectCaller1; + ObjectCaller objectCaller2; + const int cppInstance = qmlRegisterSingletonInstance( + "ClearSingletons", 1, 0, "CppInstance", &objectCaller1); + + QQmlPrivate::SingletonFunctor deprecatedSingletonFunctor; + deprecatedSingletonFunctor.m_object = &objectCaller2; + const int deprecatedCppInstance = qmlRegisterSingletonType( + "ClearSingletons", 1, 0, "DeprecatedCppInstance", deprecatedSingletonFunctor); + + const int cppFactory = qmlRegisterSingletonType( + "ClearSingletons", 1, 0, "CppFactory", &CppSingleton::create); + const int jsValue = qmlRegisterSingletonType( + "ClearSingletons", 1, 0, "JsValue", &createValueSingleton); + const int jsObject = qmlRegisterSingletonType( + "ClearSingletons", 1, 0, "JsObject", &JsSingleton::create); + const int qmlObject = qmlRegisterSingletonType( + testFileUrl("QmlSingleton.qml"), "ClearSingletons", 1, 0, "QmlSingleton"); + + QQmlEngine engine; + + QCOMPARE(engine.singletonInstance(cppInstance), &objectCaller1); + QCOMPARE(engine.singletonInstance(deprecatedCppInstance), &objectCaller2); + const CppSingleton *oldCppSingleton = engine.singletonInstance(cppFactory); + QVERIFY(oldCppSingleton != nullptr); + QCOMPARE(engine.singletonInstance(jsValue).toUInt(), 13u); + const JsSingleton *oldJsSingleton = engine.singletonInstance(jsObject); + QVERIFY(oldJsSingleton != nullptr); + const QObject *oldQmlSingleton = engine.singletonInstance(qmlObject); + QVERIFY(oldQmlSingleton != nullptr); + + QQmlComponent c(&engine); + c.setData("import ClearSingletons\n" + "import QtQml\n" + "QtObject {\n" + " property QtObject a: CppInstance\n" + " property QtObject f: DeprecatedCppInstance\n" + " property QtObject b: CppFactory\n" + " property int c: JsValue\n" + " property QtObject d: JsObject\n" + " property QtObject e: QmlSingleton\n" + "}", QUrl()); + QVERIFY2(c.isReady(), qPrintable(c.errorString())); + QScopedPointer singletonUser(c.create()); + QCOMPARE(qvariant_cast(singletonUser->property("a")), &objectCaller1); + QCOMPARE(qvariant_cast(singletonUser->property("f")), &objectCaller2); + QCOMPARE(qvariant_cast(singletonUser->property("b")), oldCppSingleton); + QCOMPARE(singletonUser->property("c").toUInt(), 13u); + QCOMPARE(qvariant_cast(singletonUser->property("d")), oldJsSingleton); + QCOMPARE(qvariant_cast(singletonUser->property("e")), oldQmlSingleton); + + engine.clearSingletons(); + + QCOMPARE(engine.singletonInstance(cppInstance), &objectCaller1); + + // Singleton instances created with previous versions of qmlRegisterSingletonInstance() + // are lost and cannot be accessed anymore. We can only call their synthesized factory + // functions once. This is not a big problem as you have to recompile in order to use + // clearSingletons() anyway. + QTest::ignoreMessage( + QtWarningMsg, + ": Singleton registered by registerSingletonInstance " + "must only be accessed from one engine"); + QTest::ignoreMessage( + QtCriticalMsg, + ": qmlRegisterSingletonType(): \"DeprecatedCppInstance\" is not " + "available because the callback function returns a null pointer."); + QCOMPARE(engine.singletonInstance(deprecatedCppInstance), nullptr); + + const CppSingleton *newCppSingleton = engine.singletonInstance(cppFactory); + QVERIFY(newCppSingleton != nullptr); + QVERIFY(newCppSingleton != oldCppSingleton); + QCOMPARE(engine.singletonInstance(jsValue).toUInt(), 13u); + const JsSingleton *newJsSingleton = engine.singletonInstance(jsObject); + QVERIFY(newJsSingleton != nullptr); + QVERIFY(newJsSingleton != oldJsSingleton); + const QObject *newQmlSingleton = engine.singletonInstance(qmlObject); + QVERIFY(newQmlSingleton != nullptr); + QVERIFY(newQmlSingleton != oldQmlSingleton); + + // Holding on to an old singleton instance is OK. We don't delete those. + QCOMPARE(qvariant_cast(singletonUser->property("a")), &objectCaller1); + QCOMPARE(qvariant_cast(singletonUser->property("f")), &objectCaller2); + + // Deleting the singletons created by factories has cleared their references in QML. + // We don't renew them as the point of clearing the singletons is not having any + // singletons left afterwards. + QCOMPARE(qvariant_cast(singletonUser->property("b")), nullptr); + QCOMPARE(qvariant_cast(singletonUser->property("d")), nullptr); + QCOMPARE(qvariant_cast(singletonUser->property("e")), nullptr); + + // Value types are unaffected as they are copied. + QCOMPARE(singletonUser->property("c").toUInt(), 13u); +} + void tst_qqmlengine::repeatedCompilation() { QQmlEngine engine; @@ -947,13 +1081,6 @@ void tst_qqmlengine::qrcUrls() } } -class ObjectCaller : public QObject -{ - Q_OBJECT -signals: - void doubleReply(const double a); -}; - void tst_qqmlengine::cppSignalAndEval() { ObjectCaller objectCaller; @@ -979,32 +1106,6 @@ void tst_qqmlengine::cppSignalAndEval() QCOMPARE(object->property("r"), 1.1234); } -class CppSingleton : public QObject { - Q_OBJECT -public: - CppSingleton() {} - - static QObject *create(QQmlEngine *qmlEngine, QJSEngine *jsEngine) - { - Q_UNUSED(qmlEngine); - Q_UNUSED(jsEngine); - return new CppSingleton(); - } -}; - -class JsSingleton : public QObject { - Q_OBJECT -public: - JsSingleton() {} - - static QJSValue create(QQmlEngine *qmlEngine, QJSEngine *jsEngine) - { - Q_UNUSED(qmlEngine); - QJSValue value = jsEngine->newQObject(new JsSingleton()); - return value; - } -}; - class SomeQObjectClass : public QObject { Q_OBJECT public: