aboutsummaryrefslogtreecommitdiffstats
diff options
context:
space:
mode:
authorUlf Hermann <ulf.hermann@qt.io>2025-08-07 16:42:22 +0200
committerUlf Hermann <ulf.hermann@qt.io>2025-09-02 09:15:10 +0200
commitb1c48db754c7019e0472d0baf6d71d2bd93a205b (patch)
tree59d174c30931ab8a95bc513d840553e182cc4f07
parent2d016a2653c59f10a57dc1903b817f71d16d0622 (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.cpp7
-rw-r--r--src/qml/qml/qqmlprivate.h21
-rw-r--r--src/qmlcompiler/qqmljscodegenerator.cpp11
-rw-r--r--src/qmlcompiler/qqmljscodegenerator_p.h2
-rw-r--r--src/qmlcompiler/qqmljsoptimizations.cpp20
-rw-r--r--tests/auto/qml/qmlcppcodegen/data/CMakeLists.txt2
-rw-r--r--tests/auto/qml/qmlcppcodegen/data/callFactory.qml10
-rw-r--r--tests/auto/qml/qmlcppcodegen/data/cppobj.h46
-rw-r--r--tests/auto/qml/qmlcppcodegen/tst_qmlcppcodegen.cpp14
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 &registerName, 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;