Fix GC issues related to QObject connections and ownership

Issue 1: Even if a sender object was only weakly referenced, the
connection's slot function would still get marked. If the slot was a
closure, its scope could hold a reference to the sender object, so by
marking the closure, the sender would get marked, too - even if there
were no other references to the closure outside of the QObject
connection structure. This would cause the sender object to stay
alive, rather than being garbage-collected (i.e., it leaked).

Issue 2: It's possible that a closure used as a slot in a connection
for one QObject holds the only reference to another QObject that has
connections of its own. In that case, if the first object is
explicitly referenced, the second object (and its connections) should
get marked. But depending on the order in which the connections were
marked, the second object might get treated incorrectly.

This commit solves both issues by introducing an iterative scheme for
marking connections. The first pass marks only connections whose
sender object is referenced elsewhere in the JS environment. The
second pass marks connections whose sender object is referenced by
slots of the connections marked in the first pass. And so on, until
no more connections should be marked. At that point, any remaining
unmarked connections are effectively dead (belonging to QObjects that
can be reclaimed by the GC).

Task-number: QTBUG-26590
Change-Id: I50aa66f7fe407a6827b6f2a12e027275a2fb4655
Reviewed-by: Simon Hausmann <simon.hausmann@nokia.com>

1 files changed, 32 insertions, 8 deletions

diff --git a/src/script/api/qscriptengine.cpp b/src/script/api/qscriptengine.cpp
index 8f8c86d..d8eaf3f 100644
--- a/src/script/api/qscriptengine.cpp
+++ b/src/script/api/qscriptengine.cpp
@@ -1307,14 +1307,7 @@ void QScriptEnginePrivate::mark(JSC::MarkStack& markStack)
     }
 
 #ifndef QT_NO_QOBJECT
-    markStack.drain(); // make sure everything is marked before marking qobject data
-    {
-        QHash<QObject*, QScript::QObjectData*>::const_iterator it;
-        for (it = m_qobjectData.constBegin(); it != m_qobjectData.constEnd(); ++it) {
-            QScript::QObjectData *qdata = it.value();
-            qdata->mark(markStack);
-        }
-    }
+    markQObjectData(markStack);
 #endif
 }
 
@@ -1417,6 +1410,37 @@ JSC::JSValue QScriptEnginePrivate::evaluateHelper(JSC::ExecState *exec, intptr_t
 
 #ifndef QT_NO_QOBJECT
 
+void QScriptEnginePrivate::markQObjectData(JSC::MarkStack& markStack)
+{
+    QHash<QObject*, QScript::QObjectData*>::const_iterator it;
+    // 1. Clear connection mark bits for all objects
+    for (it = m_qobjectData.constBegin(); it != m_qobjectData.constEnd(); ++it) {
+        QScript::QObjectData *qdata = it.value();
+        qdata->clearConnectionMarkBits();
+    }
+
+    // 2. Iterate until no more connections are marked
+    int markedCount;
+    do {
+        // Drain the stack to ensure mark bits are set; this is used to determine
+        // whether a connection's sender object is weakly referenced
+        markStack.drain();
+
+        markedCount = 0;
+        for (it = m_qobjectData.constBegin(); it != m_qobjectData.constEnd(); ++it) {
+            QScript::QObjectData *qdata = it.value();
+            markedCount += qdata->markConnections(markStack);
+        }
+    } while (markedCount > 0);
+    markStack.drain(); // One last time before marking wrappers
+
+    // 3. Mark all wrappers
+    for (it = m_qobjectData.constBegin(); it != m_qobjectData.constEnd(); ++it) {
+        QScript::QObjectData *qdata = it.value();
+        qdata->markWrappers(markStack);
+    }
+}
+
 JSC::JSValue QScriptEnginePrivate::newQObject(
     QObject *object, QScriptEngine::ValueOwnership ownership,
     const QScriptEngine::QObjectWrapOptions &options)