diff options
author | Ulf Hermann <ulf.hermann@qt.io> | 2022-03-23 11:36:59 +0100 |
---|---|---|
committer | Qt Cherry-pick Bot <cherrypick_bot@qt-project.org> | 2022-03-24 18:38:17 +0000 |
commit | c49fbc7a86c5d25e516091ee2aa933ab32e2606b (patch) | |
tree | 30a04e997261842f8b3704b231474d07d6129244 | |
parent | ab22b8bbd3496466585092458c5038329636c4e1 (diff) |
V4 ArrayIterator: Protect retrieved value from GC
When constructing the iterator return object, the garbage collector may
run, and drop the element value we want to return.
Fixes: QTBUG-101700
Change-Id: I60c9b0b9fbb9e784fa089a8b5bb274d02ef7fc1f
Reviewed-by: Maximilian Goldstein <max.goldstein@qt.io>
Reviewed-by: Andrei Golubev <andrei.golubev@qt.io>
(cherry picked from commit 185760fa44f5b62f1ed3f10a458f4bc38072768f)
Reviewed-by: Qt Cherry-pick Bot <cherrypick_bot@qt-project.org>
-rw-r--r-- | src/qml/jsruntime/qv4arrayiterator.cpp | 6 | ||||
-rw-r--r-- | tests/auto/qml/qjsengine/tst_qjsengine.cpp | 71 |
2 files changed, 74 insertions, 3 deletions
diff --git a/src/qml/jsruntime/qv4arrayiterator.cpp b/src/qml/jsruntime/qv4arrayiterator.cpp index 199b1a728a..51387edf6e 100644 --- a/src/qml/jsruntime/qv4arrayiterator.cpp +++ b/src/qml/jsruntime/qv4arrayiterator.cpp @@ -86,18 +86,18 @@ ReturnedValue ArrayIteratorPrototype::method_next(const FunctionObject *b, const return IteratorPrototype::createIterResultObject(scope.engine, Value::fromInt32(index), false); } - ReturnedValue elementValue = a->get(index); + QV4::ScopedValue elementValue(scope, a->get(index)); CHECK_EXCEPTION(); if (itemKind == ValueIteratorKind) { - return IteratorPrototype::createIterResultObject(scope.engine, Value::fromReturnedValue(elementValue), false); + return IteratorPrototype::createIterResultObject(scope.engine, elementValue, false); } else { Q_ASSERT(itemKind == KeyValueIteratorKind); ScopedArrayObject resultArray(scope, scope.engine->newArrayObject()); resultArray->arrayReserve(2); resultArray->arrayPut(0, Value::fromInt32(index)); - resultArray->arrayPut(1, Value::fromReturnedValue(elementValue)); + resultArray->arrayPut(1, elementValue); resultArray->setArrayLengthUnchecked(2); return IteratorPrototype::createIterResultObject(scope.engine, resultArray, false); diff --git a/tests/auto/qml/qjsengine/tst_qjsengine.cpp b/tests/auto/qml/qjsengine/tst_qjsengine.cpp index e9e22d4a93..85ec400cea 100644 --- a/tests/auto/qml/qjsengine/tst_qjsengine.cpp +++ b/tests/auto/qml/qjsengine/tst_qjsengine.cpp @@ -281,6 +281,7 @@ private slots: void uiLanguage(); void urlObject(); void thisInConstructor(); + void forOfAndGc(); public: Q_INVOKABLE QJSValue throwingCppMethod1(); @@ -5501,6 +5502,76 @@ void tst_QJSEngine::thisInConstructor() QVERIFY(result.isObject()); } +void tst_QJSEngine::forOfAndGc() +{ + // We want to guard against the iterator of a for..of loop leaving the result unprotected from + // garbage collection. It should be possible to construct a pure JS test case, but due to the + // vaguaries of garbage collection it's hard to reliably trigger the crash. This test is the + // best I could come up with. + + QQmlEngine engine; + QQmlComponent c(&engine); + c.setData(R"( + import QtQml + + QtObject { + id: counter + property int count: 0 + + property DelegateModel model: DelegateModel { + id: filesModel + + model: ListModel { + Component.onCompleted: { + for (let idx = 0; idx < 50; idx++) + append({"i" : idx}) + } + } + + groups: [ + DelegateModelGroup { + name: "selected" + } + ] + + function getSelected() { + for (let i = 0; i < items.count; ++i) { + var item = items.get(i) + for (let el of item.groups) { + if (el === "selected") + ++counter.count + } + } + } + + property bool bSelect: true + function selectAll() { + for (let i = 0; i < items.count; ++i) { + if (bSelect && !items.get(i).inSelected) + items.addGroups(i, 1, ["selected"]) + else + items.removeGroups(i, 1, ["selected"]) + getSelected() + } + bSelect = !bSelect + } + } + + property Timer timer: Timer { + running: true + interval: 1 + repeat: true + onTriggered: filesModel.selectAll() + } + } + )", QUrl()); + + QVERIFY2(c.isReady(), qPrintable(c.errorString())); + QScopedPointer<QObject> o(c.create()); + + QTRY_VERIFY(o->property("count").toInt() > 32768); +} + QTEST_MAIN(tst_QJSEngine) #include "tst_qjsengine.moc" |