2 files changed, 64 insertions, 0 deletions

diff --git a/src/qml/jsruntime/qv4internalclass_p.h b/src/qml/jsruntime/qv4internalclass_p.h
index d14579b4ce..68c0fe57a0 100644
--- a/src/qml/jsruntime/qv4internalclass_p.h
+++ b/src/qml/jsruntime/qv4internalclass_p.h
@@ -133,6 +133,8 @@ struct SharedInternalClassDataPrivate<PropertyAttributes> {
 
     void grow();
 
+    void markIfNecessary(const PropertyAttributes &) {}
+
     uint alloc() const { return m_alloc; }
     uint size() const { return m_size; }
     void setSize(uint s) { m_size = s; }
@@ -180,6 +182,9 @@ struct SharedInternalClassDataPrivate<PropertyKey> {
     SharedInternalClassDataPrivate(const SharedInternalClassDataPrivate &other, uint pos, PropertyKey value);
     ~SharedInternalClassDataPrivate() {}
 
+    template<typename StringOrSymbol = Heap::StringOrSymbol>
+    void markIfNecessary(const PropertyKey &value);
+
     void grow();
     uint alloc() const;
     uint size() const;
@@ -196,6 +201,17 @@ private:
     Heap::MemberData *data;
 };
 
+template<typename StringOrSymbol>
+void QV4::SharedInternalClassDataPrivate<PropertyKey>::markIfNecessary(const PropertyKey &value)
+{
+    QV4::WriteBarrier::markCustom(engine, [&](QV4::MarkStack *stack) {
+        if constexpr (QV4::WriteBarrier::isInsertionBarrier) {
+            if (auto s = value.asStringOrSymbol<StringOrSymbol>())
+                s->mark(stack);
+        }
+    });
+}
+
 template <typename T>
 struct SharedInternalClassData {
     using Private = SharedInternalClassDataPrivate<T>;
@@ -223,6 +239,7 @@ struct SharedInternalClassData {
     }
 
     void add(uint pos, T value) {
+        d->markIfNecessary(value);
         if (pos < d->size()) {
             Q_ASSERT(d->refcount > 1);
             // need to detach
@@ -244,6 +261,7 @@ struct SharedInternalClassData {
 
     void set(uint pos, T value) {
         Q_ASSERT(pos < d->size());
+        d->markIfNecessary(value);
         if (d->refcount > 1) {
             // need to detach
             Private *dd = new Private(*d);
diff --git a/tests/auto/qml/qv4mm/tst_qv4mm.cpp b/tests/auto/qml/qv4mm/tst_qv4mm.cpp
index dec49324d5..806eb00f0f 100644
--- a/tests/auto/qml/qv4mm/tst_qv4mm.cpp
+++ b/tests/auto/qml/qv4mm/tst_qv4mm.cpp
@@ -10,6 +10,7 @@
 #include <private/qv4qobjectwrapper_p.h>
 #include <private/qjsvalue_p.h>
 #include <private/qqmlengine_p.h>
+#include <private/qv4identifiertable_p.h>
 
 #include <QtQuickTestUtils/private/qmlutils_p.h>
 
@@ -30,6 +31,7 @@ private slots:
     void accessParentOnDestruction();
     void cleanInternalClasses();
     void createObjectsOnDestruction();
+    void sharedInternalClassDataMarking();
 };
 
 tst_qv4mm::tst_qv4mm()
@@ -307,6 +309,50 @@ void tst_qv4mm::createObjectsOnDestruction()
     QCOMPARE(obj->property("ok").toBool(), true);
 }
 
+void tst_qv4mm::sharedInternalClassDataMarking()
+{
+    QV4::ExecutionEngine engine;
+    QV4::Scope scope(engine.rootContext());
+    QV4::ScopedObject object(scope, engine.newObject());
+    QVERIFY(!engine.memoryManager->gcBlocked);
+    // no scoped classes, as that would defeat the point of the test
+    // we block the gc instead so that the allocation can't trigger the gc
+    engine.memoryManager->gcBlocked = true;
+    QV4::Heap::String *s = engine.newString(QString::fromLatin1("test"));
+    QV4::PropertyKey id = engine.identifierTable->asPropertyKeyImpl(s);
+    engine.memoryManager->gcBlocked = false;
+    QVERIFY(!id.asStringOrSymbol()->isMarked());
+
+    auto sm = engine.memoryManager->gcStateMachine.get();
+    sm->reset();
+    while (sm->state != QV4::GCState::MarkGlobalObject) {
+        QV4::GCStateInfo& stateInfo = sm->stateInfoMap[int(sm->state)];
+        sm->state = stateInfo.execute(sm, sm->stateData);
+    }
+
+    // simulate partial marking caused by drain due mark stack running out of space
+    // and running out of time during drain phase for complete marking
+    // the last part is necessary for us to find not-already marked name/value pair to put into
+    // the object
+
+    QVERIFY(engine.memoryManager->markStack()->isEmpty());
+    QVERIFY(!id.asStringOrSymbol()->isMarked());
+    {
+
+        // for simplcity's sake we create a new PropertyKey - if gc were actually ongoing that would
+        // already mark it. In practice we would need to retrieve an existing one from an unmarked
+        // object, and then make that object unreachable afterwards.
+        object->put(id, QV4::Value::fromUInt32(42));
+        engine.memoryManager->markStack()->drain();
+        QVERIFY(id.asStringOrSymbol()->isMarked());
+    }
+    gc(engine);
+    // sanity check that we still can lookup the value
+    QV4::ScopedString s2(scope, engine.newString(QString::fromLatin1("test")));
+    auto val = QV4::Value::fromReturnedValue(object->get(s2->toPropertyKey()));
+    QCOMPARE(val.toUInt32(), 42u);
+}
+
 QTEST_MAIN(tst_qv4mm)
 
 #include "tst_qv4mm.moc"