gc: Fix stale pointers in WeakValues

WeakValue::set shold normally not mark objects, given that a weak value
is not supposed to keep an object alive.
However, if we are past GCState::HandleQObjectWrappers, nothing will
reset weak values referencing unmarked values, even if sweep collects
the referenced value.
That leads to stale pointers, and then most likely to crashes.
To avoid this, we mark the objects under this special condition.

The test is written in a way that would also allow for resetting the new
weak values instead, but the current implementation treats memory usage
for throughput and doesn't revisit weak values to reset them.

Change-Id: I789f63c1d8609957711c2253d2e76b4bd3f9810a
Reviewed-by: Ulf Hermann <ulf.hermann@qt.io>

1 files changed, 44 insertions, 0 deletions

diff --git a/tests/auto/qml/qv4mm/tst_qv4mm.cpp b/tests/auto/qml/qv4mm/tst_qv4mm.cpp
index 3bf68f7f89..5bcdcd4624 100644
--- a/tests/auto/qml/qv4mm/tst_qv4mm.cpp
+++ b/tests/auto/qml/qv4mm/tst_qv4mm.cpp
@@ -36,6 +36,7 @@ private slots:
     void createObjectsOnDestruction();
     void sharedInternalClassDataMarking();
     void gcTriggeredInOnDestroyed();
+    void weakValuesAssignedAfterThePhaseThatShouldHandleWeakValues();
 };
 
 tst_qv4mm::tst_qv4mm()
@@ -457,6 +458,49 @@ void tst_qv4mm::gcTriggeredInOnDestroyed()
     gc(v4); // run another gc cycle
     QVERIFY(!testObject); // now collcted by gc
 }
+void tst_qv4mm::weakValuesAssignedAfterThePhaseThatShouldHandleWeakValues()
+{
+    QObject testObject;
+    QV4::ExecutionEngine v4;
+
+    QCOMPARE(v4.memoryManager->gcBlocked, QV4::MemoryManager::Unblocked);
+
+
+
+    // let the gc run up to CallDestroyObjects
+    auto sm = v4.memoryManager->gcStateMachine.get();
+    sm->reset();
+    v4.memoryManager->gcBlocked = QV4::MemoryManager::NormalBlocked;
+
+
+    // run just before the sweeping face
+    while (sm->state != QV4::GCState::DoSweep && sm->state != QV4::GCState::Invalid) {
+        QV4::GCStateInfo& stateInfo = sm->stateInfoMap[int(sm->state)];
+        sm->state = stateInfo.execute(sm, sm->stateData);
+    }
+    QCOMPARE(sm->state, QV4::GCState::DoSweep);
+
+    {
+        // simulate code accessing the object wrapper for an object
+        QV4::Scope scope(v4.rootContext());
+        QV4::ScopedValue value(scope);
+        value = QV4::QObjectWrapper::wrap(&v4, &testObject);
+        // let it go out of scope before any stack re-scanning could happen
+    }
+
+    bool gcComplete = v4.memoryManager->tryForceGCCompletion();
+    QVERIFY(gcComplete);
+
+    auto ddata = QQmlData::get(&testObject);
+    QVERIFY(ddata);
+    if (ddata->jsWrapper.isUndefined()) {
+        // it's in principle valid for the wrapper to be reset, though the current
+        // implementation doesn't do it, and it requires some care
+        qWarning("Double-check the handling of weak values and object wrappers in the gc");
+        return;
+    }
+    QVERIFY(ddata->jsWrapper.valueRef()->heapObject()->inUse());
+}
 
 QTEST_MAIN(tst_qv4mm)