Fix garbage collection issue with script-owned objects with connections

This reinstates the pre-4.6 behavior: A script-owned C++ object
that's not referenced anymore should be garbage collected, even
if it has connections. In order to achieve this, the "weak"
reference to the C++ object's wrapper must be invalidated.

Task-number: QTBUG-6366
Reviewed-by: Simon Hausmann

2 files changed, 25 insertions, 23 deletions

diff --git a/src/script/api/qscriptengine.cpp b/src/script/api/qscriptengine.cpp
index dc0e0d0..b6aa872 100644
--- a/src/script/api/qscriptengine.cpp
+++ b/src/script/api/qscriptengine.cpp
@@ -1101,16 +1101,6 @@ void QScriptEnginePrivate::mark(JSC::MarkStack& markStack)
         }
     }
 
-#ifndef QT_NO_QOBJECT
-    {
-        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);
-        }
-    }
-#endif
-
     {
         QHash<int, QScriptTypeInfo*>::const_iterator it;
         for (it = m_typeInfos.constBegin(); it != m_typeInfos.constEnd(); ++it) {
@@ -1134,6 +1124,17 @@ void QScriptEnginePrivate::mark(JSC::MarkStack& markStack)
             context = context->parentContext();
         }
     }
+
+#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);
+        }
+    }
+#endif
 }
 
 bool QScriptEnginePrivate::isCollecting() const
diff --git a/src/script/bridge/qscriptqobject.cpp b/src/script/bridge/qscriptqobject.cpp
index 559fcd3..63ba9ec 100644
--- a/src/script/bridge/qscriptqobject.cpp
+++ b/src/script/bridge/qscriptqobject.cpp
@@ -83,22 +83,23 @@ struct QObjectConnection
 
     void mark(JSC::MarkStack& markStack)
     {
-        // ### need to find out if senderWrapper is marked
         if (senderWrapper) {
-            // see if the sender should be marked or not
+            // see if the sender should be marked or not;
+            // if the C++ object is owned by script, we don't want
+            // it to stay alive due to a script connection.
             Q_ASSERT(senderWrapper.inherits(&QScriptObject::info));
             QScriptObject *scriptObject = static_cast<QScriptObject*>(JSC::asObject(senderWrapper));
-            QScriptObjectDelegate *delegate = scriptObject->delegate();
-            Q_ASSERT(delegate && (delegate->type() == QScriptObjectDelegate::QtObject));
-            QObjectDelegate *inst = static_cast<QObjectDelegate*>(delegate);
-            if ((inst->ownership() == QScriptEngine::ScriptOwnership)
-                || ((inst->ownership() == QScriptEngine::AutoOwnership)
-                    && inst->value() && !inst->value()->parent())) {
-                // #### don't mark if not marked otherwise
-                //senderWrapper = JSC::JSValue();
-                markStack.append(senderWrapper);
-            } else {
-                markStack.append(senderWrapper);
+            if (!JSC::Heap::isCellMarked(scriptObject)) {
+                QScriptObjectDelegate *delegate = scriptObject->delegate();
+                Q_ASSERT(delegate && (delegate->type() == QScriptObjectDelegate::QtObject));
+                QObjectDelegate *inst = static_cast<QObjectDelegate*>(delegate);
+                if ((inst->ownership() == QScriptEngine::ScriptOwnership)
+                    || ((inst->ownership() == QScriptEngine::AutoOwnership)
+                        && inst->value() && !inst->value()->parent())) {
+                    senderWrapper = JSC::JSValue();
+                } else {
+                    markStack.append(senderWrapper);
+                }
             }
         }
         if (receiver)