summaryrefslogtreecommitdiffstats
diff options
context:
space:
mode:
authorTom Scheler <dd-tom@web.de>2019-05-19 18:16:34 +0200
committerTom Scheler <dd-tom@web.de>2019-07-02 08:58:27 +0000
commit29c61f83c98c269c1c862a668c0a91a1027ee2d2 (patch)
treee60f4a1ba64036d85e67fce51db24ed73ad5fa9d
parentcca5d1ec2f2c1f038c7c933b6c57d89888fc683b (diff)
Fix read access violation when using KeyNavigation attached property
Setting the KeyNavigation.up property of an item to another item will implicitly set the reverse (KeyNavigation.down) property on that other item pointing back to the item. Once the item is destroyed, you will have an invalid pointer stored in the other item pointing to the destroyed item. Using QPointer<> instead of raw pointers fixes that issue, because they will become null on QObject's destruction. Added QQuickItem test that verifies the issue is solved. Fixes: QTBUG-75399 Change-Id: Ibb3e976c4eb9fcd81604bcc2eb757257d3653930 Reviewed-by: Shawn Rutledge <shawn.rutledge@qt.io> Reviewed-by: Ulf Hermann <ulf.hermann@qt.io>
-rw-r--r--src/quick/items/qquickitem_p.h13
-rw-r--r--tests/auto/quick/qquickitem2/data/keynavigationtest_implicitDestroy.qml13
-rw-r--r--tests/auto/quick/qquickitem2/tst_qquickitem.cpp24
3 files changed, 44 insertions, 6 deletions
diff --git a/src/quick/items/qquickitem_p.h b/src/quick/items/qquickitem_p.h
index 771228914b..f618bcf1c3 100644
--- a/src/quick/items/qquickitem_p.h
+++ b/src/quick/items/qquickitem_p.h
@@ -75,6 +75,7 @@
#include <QtCore/qlist.h>
#include <QtCore/qdebug.h>
#include <QtCore/qelapsedtimer.h>
+#include <QtCore/qpointer.h>
#if QT_CONFIG(quick_shadereffect)
#include <QtQuick/private/qquickshadereffectsource_p.h>
@@ -682,12 +683,12 @@ public:
: leftSet(false), rightSet(false), upSet(false), downSet(false),
tabSet(false), backtabSet(false) {}
- QQuickItem *left = nullptr;
- QQuickItem *right = nullptr;
- QQuickItem *up = nullptr;
- QQuickItem *down = nullptr;
- QQuickItem *tab = nullptr;
- QQuickItem *backtab = nullptr;
+ QPointer<QQuickItem> left;
+ QPointer<QQuickItem> right;
+ QPointer<QQuickItem> up;
+ QPointer<QQuickItem> down;
+ QPointer<QQuickItem> tab;
+ QPointer<QQuickItem> backtab;
bool leftSet : 1;
bool rightSet : 1;
bool upSet : 1;
diff --git a/tests/auto/quick/qquickitem2/data/keynavigationtest_implicitDestroy.qml b/tests/auto/quick/qquickitem2/data/keynavigationtest_implicitDestroy.qml
new file mode 100644
index 0000000000..54d20273da
--- /dev/null
+++ b/tests/auto/quick/qquickitem2/data/keynavigationtest_implicitDestroy.qml
@@ -0,0 +1,13 @@
+import QtQuick 2.12
+
+Item {
+ id: root
+
+ function createImplicitKeyNavigation() {
+ var item = Qt.createQmlObject("import QtQuick 2.0; Item { }", root);
+ item.KeyNavigation.up = root
+ item.destroy();
+
+ forceActiveFocus();
+ }
+}
diff --git a/tests/auto/quick/qquickitem2/tst_qquickitem.cpp b/tests/auto/quick/qquickitem2/tst_qquickitem.cpp
index 7107f4d995..399535cfa6 100644
--- a/tests/auto/quick/qquickitem2/tst_qquickitem.cpp
+++ b/tests/auto/quick/qquickitem2/tst_qquickitem.cpp
@@ -86,6 +86,7 @@ private slots:
void keyNavigation_RightToLeft();
void keyNavigation_skipNotVisible();
void keyNavigation_implicitSetting();
+ void keyNavigation_implicitDestroy();
void keyNavigation_focusReason();
void keyNavigation_loop();
void layoutMirroring();
@@ -2164,6 +2165,29 @@ void tst_QQuickItem::keyNavigation_implicitSetting()
delete window;
}
+// QTBUG-75399
+void tst_QQuickItem::keyNavigation_implicitDestroy()
+{
+ QQuickView view;
+ view.setSource(testFileUrl("keynavigationtest_implicitDestroy.qml"));
+ view.show();
+
+ QVERIFY(QTest::qWaitForWindowActive(&view));
+
+ QQuickItem *root = view.rootObject();
+ QVERIFY(QMetaObject::invokeMethod(root, "createImplicitKeyNavigation"));
+
+ // process events is necessary to trigger upcoming memory access violation
+ QTest::qWait(0);
+
+ QVERIFY(root->hasActiveFocus());
+
+ QKeyEvent keyPress = QKeyEvent(QEvent::KeyPress, Qt::Key_Down, Qt::NoModifier, "", false, 1);
+ QGuiApplication::sendEvent(&view, &keyPress); // <-- access violation happens here
+ // this should fail the test, even if the access violation does not occur
+ QVERIFY(!keyPress.isAccepted());
+}
+
void tst_QQuickItem::keyNavigation_focusReason()
{
QQuickView *window = new QQuickView(nullptr);