From 59b83b9990d2dd222d0e4ab202f6f5ae53088dc2 Mon Sep 17 00:00:00 2001 From: Juha Vuolle Date: Thu, 6 May 2021 18:39:48 +0300 Subject: Fix calling of the overloaded Scxml statemachine methods This commit reverts the commit 2951c124d3ff58abe1f84175e512ba26977f13d8 which marked the dynamic statemachines' metabojects as fully dynamic. The marking was done to prevent stale accesses via QML property cache. However it introduced another problem with the resolution of the QScxmlStateMachine's overloaded invokable methods. The result was the inability to call all but one of the functions. To address the original issues, instead of the dynamic marking, this commit invalidates the related qml property cache during the dynamic metaobject's destruction. Task-number: QTBUG-89521 Task-number: QTBUG-92367 Change-Id: I5d206558bb052b27b2764a80c4bcbe6410c07b22 Reviewed-by: Qt CI Bot Reviewed-by: Ulf Hermann --- src/scxml/qscxmlcompiler.cpp | 56 ++++++++++++++++++----------------- tests/auto/auto.pro | 3 +- tests/auto/qmltest/qmltest.pro | 14 +++++++++ tests/auto/qmltest/statemachine.scxml | 45 ++++++++++++++++++++++++++++ tests/auto/qmltest/tst_dynamic.qml | 52 ++++++++++++++++++++++++++++++++ tests/auto/qmltest/tst_scxmlqml.cpp | 30 +++++++++++++++++++ tests/auto/qmltest/tst_scxmlqml.qrc | 5 ++++ 7 files changed, 177 insertions(+), 28 deletions(-) create mode 100644 tests/auto/qmltest/qmltest.pro create mode 100644 tests/auto/qmltest/statemachine.scxml create mode 100644 tests/auto/qmltest/tst_dynamic.qml create mode 100644 tests/auto/qmltest/tst_scxmlqml.cpp create mode 100644 tests/auto/qmltest/tst_scxmlqml.qrc diff --git a/src/scxml/qscxmlcompiler.cpp b/src/scxml/qscxmlcompiler.cpp index 9bf3a92..0353fdc 100644 --- a/src/scxml/qscxmlcompiler.cpp +++ b/src/scxml/qscxmlcompiler.cpp @@ -54,6 +54,14 @@ #include "qscxmltabledata_p.h" #include +#if QT_CONFIG(scxml_ecmascriptdatamodel) +// In Qt5 the ecmascript datamodel brings in QML dependency. +// We use that to include the QML headers needed for invalidating +// property cache, even though strictly speaking these are distinct +// functionalities. +#include +#include +#endif #endif // BUILD_QSCXMLC #include @@ -485,31 +493,9 @@ private: class DynamicStateMachinePrivate : public QScxmlStateMachinePrivate { - struct DynamicMetaObject : public QAbstractDynamicMetaObject - { - QAbstractDynamicMetaObject *toDynamicMetaObject(QObject *) override - { - return this; - } - - int metaCall(QObject *o, QMetaObject::Call c, int id, void **a) override - { - return o->qt_metacall(c, id, a); - } - }; - public: DynamicStateMachinePrivate() : - QScxmlStateMachinePrivate(&QScxmlStateMachine::staticMetaObject) - { - metaObject = new DynamicMetaObject; - } - - void setDynamicMetaObject(const QMetaObject *m) { - // Prevent the QML engine from creating a property cache for this thing. - static_cast(metaObject)->d = m->d; - m_metaObject = m; - } + QScxmlStateMachinePrivate(&QScxmlStateMachine::staticMetaObject) {} }; class DynamicStateMachine: public QScxmlStateMachine, public QScxmlInternal::GeneratedTableData @@ -566,7 +552,7 @@ private: b.setClassName("DynamicStateMachine"); b.setSuperClass(&QScxmlStateMachine::staticMetaObject); b.setStaticMetacallFunction(qt_static_metacall); - d->setDynamicMetaObject(b.toMetaObject()); + d->m_metaObject = b.toMetaObject(); } void initDynamicParts(const MetaDataInfo &info) @@ -575,7 +561,7 @@ private: // Release the temporary QMetaObject. Q_ASSERT(d->m_metaObject != &QScxmlStateMachine::staticMetaObject); free(const_cast(d->m_metaObject)); - d->setDynamicMetaObject(&QScxmlStateMachine::staticMetaObject); + d->m_metaObject = &QScxmlStateMachine::staticMetaObject; // Build the real one. QMetaObjectBuilder b; @@ -601,7 +587,7 @@ private: } // And we're done - d->setDynamicMetaObject(b.toMetaObject()); + d->m_metaObject = b.toMetaObject(); } public: @@ -609,8 +595,24 @@ public: { Q_D(DynamicStateMachine); if (d->m_metaObject != &QScxmlStateMachine::staticMetaObject) { +#if QT_CONFIG(scxml_ecmascriptdatamodel) + // Invalidate the QML property cache as we delete the dynamic + // metaobject, otherwise stale string accesses might occur. + // Important! This invalidation is a workaround and brittle at + // at best; while string cache will be cleared, the cache itself + // will not. Instead we rely on that the (only) user for the cache + // is gone as well. This workaround is specific to Qt5, in Qt6 + // we are able to fix the issue more properly by marking the + // metaobject dynamic => QML property caching will not be done. + // + // All further interaction with this property cache must be + // avoided. + QQmlData *ddata = QQmlData::get(this, false); + if (ddata && ddata->propertyCache) + ddata->propertyCache->invalidate(d->m_metaObject); +#endif free(const_cast(d->m_metaObject)); - d->setDynamicMetaObject(&QScxmlStateMachine::staticMetaObject); + d->m_metaObject = &QScxmlStateMachine::staticMetaObject; } } diff --git a/tests/auto/auto.pro b/tests/auto/auto.pro index 60576d5..acce986 100644 --- a/tests/auto/auto.pro +++ b/tests/auto/auto.pro @@ -5,4 +5,5 @@ SUBDIRS = cmake\ parser\ scion\ statemachine \ - statemachineinfo + statemachineinfo \ + qmltest diff --git a/tests/auto/qmltest/qmltest.pro b/tests/auto/qmltest/qmltest.pro new file mode 100644 index 0000000..c9649bf --- /dev/null +++ b/tests/auto/qmltest/qmltest.pro @@ -0,0 +1,14 @@ +TEMPLATE = app +TARGET = tst_scxmlqml +CONFIG += qmltestcase + +SOURCES += \ + $$PWD/tst_scxmlqml.cpp + +RESOURCES += tst_scxmlqml.qrc + +OTHER_FILES += \ + $$PWD/*.qml + +TESTDATA += \ + $$PWD/tst_* diff --git a/tests/auto/qmltest/statemachine.scxml b/tests/auto/qmltest/statemachine.scxml new file mode 100644 index 0000000..68c9768 --- /dev/null +++ b/tests/auto/qmltest/statemachine.scxml @@ -0,0 +1,45 @@ + + + + + + + + + + + + + + diff --git a/tests/auto/qmltest/tst_dynamic.qml b/tests/auto/qmltest/tst_dynamic.qml new file mode 100644 index 0000000..f8c411d --- /dev/null +++ b/tests/auto/qmltest/tst_dynamic.qml @@ -0,0 +1,52 @@ +/**************************************************************************** +** +** Copyright (C) 2021 The Qt Company Ltd. +** Contact: https://www.qt.io/licensing/ +** +** This file is part of the test suite of the Qt Toolkit. +** +** $QT_BEGIN_LICENSE:GPL-EXCEPT$ +** Commercial License Usage +** Licensees holding valid commercial Qt licenses may use this file in +** accordance with the commercial license agreement provided with the +** Software or, alternatively, in accordance with the terms contained in +** a written agreement between you and The Qt Company. For licensing terms +** and conditions see https://www.qt.io/terms-conditions. For further +** information use the contact form at https://www.qt.io/contact-us. +** +** GNU General Public License Usage +** Alternatively, this file may be used under the terms of the GNU +** General Public License version 3 as published by the Free Software +** Foundation with exceptions as appearing in the file LICENSE.GPL3-EXCEPT +** included in the packaging of this file. Please review the following +** information to ensure the GNU General Public License requirements will +** be met: https://www.gnu.org/licenses/gpl-3.0.html. +** +** $QT_END_LICENSE$ +** +****************************************************************************/ + +import QtTest 1.15 +import QtScxml 5.15 + +TestCase { + id: testCase + + StateMachineLoader { + id: loader + source: "qrc:///statemachine.scxml" + } + + function test_overloaded_calls_with_dynamic_statemachine() + { + // This test calls "submitEvent" invokable function which has 3 + // overloads, differentiated both by parameter types and amounts. + // Test verifies that the overloads are callable while using + // a dynamic statemachine which has a dynamic metaobject under the hood + tryVerify(() => loader.stateMachine.activeStateNames()[0] === "red", 200) + loader.stateMachine.submitEvent("step") + tryVerify(() => loader.stateMachine.activeStateNames()[0] === "yellow", 200) + loader.stateMachine.submitEvent("step", "somedata") + tryVerify(() => loader.stateMachine.activeStateNames()[0] === "green", 200) + } +} diff --git a/tests/auto/qmltest/tst_scxmlqml.cpp b/tests/auto/qmltest/tst_scxmlqml.cpp new file mode 100644 index 0000000..a32c12a --- /dev/null +++ b/tests/auto/qmltest/tst_scxmlqml.cpp @@ -0,0 +1,30 @@ +/**************************************************************************** +** +** Copyright (C) 2021 The Qt Company Ltd. +** Contact: https://www.qt.io/licensing/ +** +** This file is part of the test suite of the Qt Toolkit. +** +** $QT_BEGIN_LICENSE:GPL-EXCEPT$ +** Commercial License Usage +** Licensees holding valid commercial Qt licenses may use this file in +** accordance with the commercial license agreement provided with the +** Software or, alternatively, in accordance with the terms contained in +** a written agreement between you and The Qt Company. For licensing terms +** and conditions see https://www.qt.io/terms-conditions. For further +** information use the contact form at https://www.qt.io/contact-us. +** +** GNU General Public License Usage +** Alternatively, this file may be used under the terms of the GNU +** General Public License version 3 as published by the Free Software +** Foundation with exceptions as appearing in the file LICENSE.GPL3-EXCEPT +** included in the packaging of this file. Please review the following +** information to ensure the GNU General Public License requirements will +** be met: https://www.gnu.org/licenses/gpl-3.0.html. +** +** $QT_END_LICENSE$ +** +****************************************************************************/ + +#include +QUICK_TEST_MAIN(tst_scxmlqml) diff --git a/tests/auto/qmltest/tst_scxmlqml.qrc b/tests/auto/qmltest/tst_scxmlqml.qrc new file mode 100644 index 0000000..241bd0b --- /dev/null +++ b/tests/auto/qmltest/tst_scxmlqml.qrc @@ -0,0 +1,5 @@ + + + statemachine.scxml + + -- cgit v1.2.3 From d955df6e78a8bb033adf4842c1bb205634de13b9 Mon Sep 17 00:00:00 2001 From: Tarja Sundqvist Date: Thu, 20 May 2021 14:48:56 +0300 Subject: Bump version Change-Id: I2d44c92890f429548ca93905e2364610694016af --- .qmake.conf | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/.qmake.conf b/.qmake.conf index 44eb13b..4d5b540 100644 --- a/.qmake.conf +++ b/.qmake.conf @@ -4,4 +4,4 @@ CONFIG += warning_clean DEFINES += QT_NO_FOREACH QT_NO_JAVA_STYLE_ITERATORS QT_NO_LINKED_LIST -MODULE_VERSION = 5.15.4 +MODULE_VERSION = 5.15.5 -- cgit v1.2.3