Fix interaction of garbage collector with JS objects during QML type instantiation

It may happen that during the lengthy process of instantiating a tree of
objects for QML, the garbage collector runs.

For objects created by QML we support different ownership models, for example
in QtQuick visual parents keep their visual children alive, despite perhaps a
lack of QObject parentship. That ownership becomes active once the QML
autoparent function has assigned the correct visual parent, which happens after
object instantiation (after QQmlObjectCreator).

Similarly when a composite type is created, its QObject parent is only set
after all properties have been set. The root QObject is kept alive through a
special boolean, but if the sub-objects aren't children yet, their JS wrapper
might get deleted. For composite types with var properties, that also means
their var properties get deleted, such as the model property of TableView.qml
in the bug report.

In the future we want to support creating QWidget hierarchies with QML, which
also for layouts may rely on a delayed parent assignment for layouts.

To accommodate all this, this patch introduces an array on the JS stack that
keeps track of all JS wrappers for all QObjects created. This array is alive
during object tree creation. Afterwards, the different ownership models take
over, for example the auto parent function assigning a visual parent.

This patch also fixes an off-by-one in the total object count calculation
for composite types, where when instantiating a composite type as a sub-object
we counted the sub composite's object count but forgot the object itself.

Task-number: QTBUG-38835
Task-number: QTBUG-39966
Change-Id: I6104b2434510642081e0c54793ed296adeca7481
Reviewed-by: Lars Knoll <lars.knoll@digia.com>

1 files changed, 21 insertions, 6 deletions

diff --git a/src/qml/qml/qqmlobjectcreator.cpp b/src/qml/qml/qqmlobjectcreator.cpp
index 36c7dfb0e9..1de467b0fa 100644
--- a/src/qml/qml/qqmlobjectcreator.cpp
+++ b/src/qml/qml/qqmlobjectcreator.cpp
@@ -96,6 +96,7 @@ QQmlObjectCreator::QQmlObjectCreator(QQmlContextData *parentContext, QQmlCompile
     sharedState->allCreatedBindings.allocate(compiledData->totalBindingsCount);
     sharedState->allParserStatusCallbacks.allocate(compiledData->totalParserStatusCount);
     sharedState->allCreatedObjects.allocate(compiledData->totalObjectCount);
+    sharedState->allJavaScriptObjects = 0;
     sharedState->creationContext = creationContext;
     sharedState->rootContext = 0;
 
@@ -195,6 +196,13 @@ QObject *QQmlObjectCreator::create(int subComponentIndex, QObject *parent, QQmlI
         sharedState->rootContext->isRootObjectInCreation = true;
     }
 
+    QV4::ExecutionEngine *v4 = QV8Engine::getV4(engine);
+    QV4::Scope scope(v4);
+
+    Q_ASSERT(sharedState->allJavaScriptObjects || topLevelCreator);
+    if (topLevelCreator)
+        sharedState->allJavaScriptObjects = scope.alloc(compiledData->totalObjectCount);
+
     QVector<QQmlContextData::ObjectIdMapping> mapping(objectIndexToId.count());
     for (QHash<int, int>::ConstIterator it = objectIndexToId.constBegin(), end = objectIndexToId.constEnd();
          it != end; ++it) {
@@ -208,8 +216,6 @@ QObject *QQmlObjectCreator::create(int subComponentIndex, QObject *parent, QQmlI
     context->setIdPropertyData(mapping);
 
     if (subComponentIndex == -1) {
-        QV4::ExecutionEngine *v4 = QV8Engine::getV4(engine);
-        QV4::Scope scope(v4);
         QV4::ScopedObject scripts(scope, v4->newArrayObject(compiledData->scripts.count()));
         context->importedScripts = scripts;
         for (int i = 0; i < compiledData->scripts.count(); ++i) {
@@ -230,6 +236,9 @@ QObject *QQmlObjectCreator::create(int subComponentIndex, QObject *parent, QQmlI
         ddata->compiledData->addref();
     }
 
+    if (topLevelCreator)
+        sharedState->allJavaScriptObjects = 0;
+
     phase = CreatingObjectsPhase2;
 
     if (interrupt && interrupt->shouldInterrupt())
@@ -258,8 +267,11 @@ bool QQmlObjectCreator::populateDeferredProperties(QObject *instance)
 
     QV4::ExecutionEngine *v4 = QV8Engine::getV4(engine);
     QV4::Scope valueScope(v4);
-    QV4::ScopedValue scopeObjectProtector(valueScope, declarativeData->jsWrapper.value());
-    Q_UNUSED(scopeObjectProtector);
+
+    Q_ASSERT(topLevelCreator);
+    Q_ASSERT(!sharedState->allJavaScriptObjects);
+    sharedState->allJavaScriptObjects = valueScope.alloc(compiledData->totalObjectCount);
+
     QV4::ScopedObject qmlScope(valueScope, QV4::QmlContextWrapper::qmlScope(QV8Engine::get(engine), context, _scopeObject));
     QV4::Scoped<QV4::QmlBindingWrapper> qmlBindingWrapper(valueScope, new (v4->memoryManager) QV4::QmlBindingWrapper(v4->rootContext, qmlScope));
     QV4::ExecutionContext *qmlContext = qmlBindingWrapper->context();
@@ -1150,9 +1162,12 @@ QObject *QQmlObjectCreator::createInstance(int index, QObject *parent, bool isCo
     qSwap(_scopeObject, scopeObject);
 
     QV4::ExecutionEngine *v4 = QV8Engine::getV4(engine);
+
+    Q_ASSERT(sharedState->allJavaScriptObjects);
+    QV4::ValueRef ref = QV4::ValueRef::fromRawValue(sharedState->allJavaScriptObjects++);
+    ref = QV4::QObjectWrapper::wrap(v4, instance);
+
     QV4::Scope valueScope(v4);
-    QV4::ScopedValue scopeObjectProtector(valueScope, ddata ? ddata->jsWrapper.value() : 0);
-    Q_UNUSED(scopeObjectProtector);
     QV4::ScopedObject qmlScope(valueScope, QV4::QmlContextWrapper::qmlScope(QV8Engine::get(engine), context, _scopeObject));
     QV4::Scoped<QV4::QmlBindingWrapper> qmlBindingWrapper(valueScope, new (v4->memoryManager) QV4::QmlBindingWrapper(v4->rootContext, qmlScope));
     QV4::ExecutionContext *qmlContext = qmlBindingWrapper->context();