diff options
| author | Ulf Hermann <ulf.hermann@qt.io> | 2025-08-07 16:42:22 +0200 |
|---|---|---|
| committer | Ulf Hermann <ulf.hermann@qt.io> | 2025-09-02 09:15:10 +0200 |
| commit | b1c48db754c7019e0472d0baf6d71d2bd93a205b (patch) | |
| tree | 59d174c30931ab8a95bc513d840553e182cc4f07 | |
| parent | 2d016a2653c59f10a57dc1903b817f71d16d0622 (diff) | |
QmlCompiler: Ensure QObjects returned to AOT-compiled code are wrapped
If a QObject is returned from a method call, the QML engine takes
ownership of it and it needs to be deleted by the garbage collector. Our
generated C++ code so far did not actually take ownership of the object
and thereby caused it to leak.
Pick-to: 6.10 6.9 6.8
Fixes: QTBUG-138919
Change-Id: I7bd57b3612bf4b98937756e8a7a7c03aff1c9b32
Reviewed-by: Olivier De Cannière <olivier.decanniere@qt.io>
| -rw-r--r-- | src/qml/qml/qqml.cpp | 7 | ||||
| -rw-r--r-- | src/qml/qml/qqmlprivate.h | 21 | ||||
| -rw-r--r-- | src/qmlcompiler/qqmljscodegenerator.cpp | 11 | ||||
| -rw-r--r-- | src/qmlcompiler/qqmljscodegenerator_p.h | 2 | ||||
| -rw-r--r-- | src/qmlcompiler/qqmljsoptimizations.cpp | 20 | ||||
| -rw-r--r-- | tests/auto/qml/qmlcppcodegen/data/CMakeLists.txt | 2 | ||||
| -rw-r--r-- | tests/auto/qml/qmlcppcodegen/data/callFactory.qml | 10 | ||||
| -rw-r--r-- | tests/auto/qml/qmlcppcodegen/data/cppobj.h | 46 | ||||
| -rw-r--r-- | tests/auto/qml/qmlcppcodegen/tst_qmlcppcodegen.cpp | 14 |
9 files changed, 123 insertions, 10 deletions
diff --git a/src/qml/qml/qqml.cpp b/src/qml/qml/qqml.cpp index e72d6485f6..12a03244ae 100644 --- a/src/qml/qml/qqml.cpp +++ b/src/qml/qml/qqml.cpp @@ -3187,6 +3187,13 @@ void AOTCompiledContext::initCallValueLookup( lookup->call = QV4::Lookup::Call::GetterValueTypeProperty; } +void AOTCompiledContext::setObjectImplicitDestructible(QObject *object) const +{ + Q_ASSERT(object); + QQmlData::get(object, true)->setImplicitDestructible(); + QV4::QObjectWrapper::ensureWrapper(engine->handle(), object); +} + } // namespace QQmlPrivate /*! diff --git a/src/qml/qml/qqmlprivate.h b/src/qml/qml/qqmlprivate.h index c0d06dc32b..9e97993f2f 100644 --- a/src/qml/qml/qqmlprivate.h +++ b/src/qml/qml/qqmlprivate.h @@ -792,6 +792,27 @@ namespace QQmlPrivate bool callValueLookup(uint index, void *target, void **args, int argc) const; void initCallValueLookup( uint index, const QMetaObject *metaObject, int relativeMethodIndex) const; + + void setObjectImplicitDestructible(QObject *object) const; + void setImplicitDestructible(QObject *object) const + { + if (object) + setObjectImplicitDestructible(object); + } + + void setImplicitDestructible(const QVariant &variant) const + { + // This does not cover everything you can possibly wrap into a QVariant. + // However, since we only _promise_ to handle single objects, this is OK. + if (!variant.metaType().flags().testFlag(QMetaType::PointerToQObject)) + return; + + if (QObject *object = *static_cast<QObject *const *>(variant.constData())) + setObjectImplicitDestructible(object); + } + + template<typename Value> + void setImplicitDestructible(const Value &value) const { Q_UNUSED(value); } }; struct AOTCompiledFunction { diff --git a/src/qmlcompiler/qqmljscodegenerator.cpp b/src/qmlcompiler/qqmljscodegenerator.cpp index 520e71ca18..bfe5126b41 100644 --- a/src/qmlcompiler/qqmljscodegenerator.cpp +++ b/src/qmlcompiler/qqmljscodegenerator.cpp @@ -1884,13 +1884,14 @@ QString QQmlJSCodeGenerator::initAndCall( + u"};\n"_s; } -void QQmlJSCodeGenerator::generateMoveOutVar(const QString &outVar) +void QQmlJSCodeGenerator::generateMoveOutVarAfterCall(const QString &outVar) { if (m_state.accumulatorVariableOut.isEmpty() || outVar.isEmpty()) return; - m_body += m_state.accumulatorVariableOut + u" = "_s; - m_body += u"std::move(" + outVar + u");\n"; + m_body += m_state.accumulatorVariableOut + u" = "_s + u"std::move(" + outVar + u");\n"; + m_body += u"aotContext->setImplicitDestructible("_s + + m_state.accumulatorVariableOut + u");\n"_s; } void QQmlJSCodeGenerator::generate_CallValue(int name, int argc, int argv) @@ -2418,7 +2419,7 @@ void QQmlJSCodeGenerator::generate_CallPropertyLookup(int index, int base, int a const QString initialization = u"doInit()"_s; const QString preparation = getLookupPreparation(m_state.accumulatorOut(), outVar, index); generateLookup(lookup, initialization, preparation); - generateMoveOutVar(outVar); + generateMoveOutVarAfterCall(outVar); m_body += u"}\n"_s; @@ -2479,7 +2480,7 @@ void QQmlJSCodeGenerator::generate_CallQmlContextPropertyLookup(int index, int a const QString initialization = u"doInit()"_s; const QString preparation = getLookupPreparation(m_state.accumulatorOut(), outVar, index); generateLookup(lookup, initialization, preparation); - generateMoveOutVar(outVar); + generateMoveOutVarAfterCall(outVar); m_body += u"}\n"_s; } diff --git a/src/qmlcompiler/qqmljscodegenerator_p.h b/src/qmlcompiler/qqmljscodegenerator_p.h index bf44230fd7..65f83017ea 100644 --- a/src/qmlcompiler/qqmljscodegenerator_p.h +++ b/src/qmlcompiler/qqmljscodegenerator_p.h @@ -322,7 +322,7 @@ private: void generateJumpCodeWithTypeConversions(int relativeOffset); void generateUnaryOperation(const QString &cppOperator); void generateInPlaceOperation(const QString &cppOperator); - void generateMoveOutVar(const QString &outVar); + void generateMoveOutVarAfterCall(const QString &outVar); void generateTypeLookup(int index); void generateVariantEqualityComparison( QQmlJSRegisterContent nonStorable, const QString ®isterName, bool invert); diff --git a/src/qmlcompiler/qqmljsoptimizations.cpp b/src/qmlcompiler/qqmljsoptimizations.cpp index d003aa1974..b862500994 100644 --- a/src/qmlcompiler/qqmljsoptimizations.cpp +++ b/src/qmlcompiler/qqmljsoptimizations.cpp @@ -195,10 +195,22 @@ bool QQmlJSOptimizations::eraseDeadStore(const InstructionAnnotations::iterator it->second.changedRegisterIndex = InvalidRegister; it->second.changedRegister = QQmlJSRegisterContent(); } else { - // void the output, rather than deleting it. We still need its variant. - const bool adjusted = m_typeResolver->adjustTrackedType( - it->second.changedRegister, m_typeResolver->voidType()); - Q_ASSERT(adjusted); // Can always convert to void + // We can't do this with certain QObjects because they still need tracking as + // implicitly destructible by the garbage collector. We may be calling a factory + // function and then forgetting the object after all. + // + // However, objects we need to track that way can only be produced through external + // side effects (i.e. function calls). + + const QQmlJSScope::ConstPtr contained = it->second.changedRegister.containedType(); + if (!it->second.hasExternalSideEffects + || (!contained->isReferenceType() + && !m_typeResolver->canHold(contained, m_typeResolver->qObjectType()))) { + // void the output, rather than deleting it. We still need its variant. + const bool adjusted = m_typeResolver->adjustTrackedType( + it->second.changedRegister, m_typeResolver->voidType()); + Q_ASSERT(adjusted); // Can always convert to void + } } m_readerLocations.erase(reader); diff --git a/tests/auto/qml/qmlcppcodegen/data/CMakeLists.txt b/tests/auto/qml/qmlcppcodegen/data/CMakeLists.txt index 4b5dd31c0a..c0da4370fc 100644 --- a/tests/auto/qml/qmlcppcodegen/data/CMakeLists.txt +++ b/tests/auto/qml/qmlcppcodegen/data/CMakeLists.txt @@ -10,6 +10,7 @@ set(cpp_sources collector.h convertQJSPrimitiveValueToIntegral.h cppbaseclass.h + cppobj.h detachedreferences.h druggeljug.h dummyobjekt.h @@ -116,6 +117,7 @@ set(qml_files brokenAs.qml boundComponents.qml callContextPropertyLookupResult.qml + callFactory.qml callObjectLookupOnNull.qml callWithSpread.qml childobject.qml diff --git a/tests/auto/qml/qmlcppcodegen/data/callFactory.qml b/tests/auto/qml/qmlcppcodegen/data/callFactory.qml new file mode 100644 index 0000000000..1b5d9eaec8 --- /dev/null +++ b/tests/auto/qml/qmlcppcodegen/data/callFactory.qml @@ -0,0 +1,10 @@ +pragma Strict +import QtQml +import TestTypes + +QtObject { + Component.onCompleted: { + // Call the factory function and then forget the object + const orphanedObj = CppBridge.singleObj(0) + } +} diff --git a/tests/auto/qml/qmlcppcodegen/data/cppobj.h b/tests/auto/qml/qmlcppcodegen/data/cppobj.h new file mode 100644 index 0000000000..3b64c21e74 --- /dev/null +++ b/tests/auto/qml/qmlcppcodegen/data/cppobj.h @@ -0,0 +1,46 @@ +// Copyright (C) 2025 The Qt Company Ltd. +// SPDX-License-Identifier: LicenseRef-Qt-Commercial OR GPL-3.0-only + +#ifndef CPPOBJ_H +#define CPPOBJ_H + +#include <QtCore/qobject.h> +#include <QtCore/qdebug.h> +#include <QtQmlIntegration/qqmlintegration.h> + +class CppObj : public QObject +{ + Q_OBJECT + QML_ELEMENT + Q_PROPERTY(int identifier MEMBER m_identifier CONSTANT) + +public: + explicit CppObj(int identifier = -1, QObject *parent = nullptr) + : QObject(parent) + , m_identifier(identifier) + { + qDebug() << "Object created:" << m_identifier; + } + + ~CppObj() { qDebug() << "Object destroyed:" << m_identifier; } + +private: + int m_identifier = -1; +}; + +class CppBridge : public QObject +{ + Q_OBJECT + QML_ELEMENT + QML_SINGLETON + +public: + explicit CppBridge(QObject *parent = nullptr) : QObject{parent} {} + + Q_INVOKABLE CppObj *singleObj(int identifier) const + { + return new CppObj(identifier); + } +}; + +#endif // CPPOBJ_H diff --git a/tests/auto/qml/qmlcppcodegen/tst_qmlcppcodegen.cpp b/tests/auto/qml/qmlcppcodegen/tst_qmlcppcodegen.cpp index 243b6b1296..406bcb1da8 100644 --- a/tests/auto/qml/qmlcppcodegen/tst_qmlcppcodegen.cpp +++ b/tests/auto/qml/qmlcppcodegen/tst_qmlcppcodegen.cpp @@ -70,6 +70,7 @@ private slots: void brokenAs(); void boundComponents(); void callContextPropertyLookupResult(); + void callFactoryFunction(); void callObjectLookupOnNull(); void callWithSpread(); void collectGarbageDuringAotCode(); @@ -1072,6 +1073,19 @@ void tst_QmlCppCodegen::callContextPropertyLookupResult() QVERIFY(qvariant_cast<QQmlComponent *>(o->property("c")) != nullptr); } +void tst_QmlCppCodegen::callFactoryFunction() +{ + QQmlEngine engine; + QQmlComponent c(&engine, QUrl(u"qrc:/qt/qml/TestTypes/callFactory.qml"_s)); + QVERIFY2(c.isReady(), qPrintable(c.errorString())); + + QTest::ignoreMessage(QtDebugMsg, "Object created: 0"); + QScopedPointer<QObject> o(c.create()); + QVERIFY(o); + + QTest::ignoreMessage(QtDebugMsg, "Object destroyed: 0"); +} + void tst_QmlCppCodegen::callObjectLookupOnNull() { QQmlEngine engine; |
