QDeclarativeIncubator::clear() and autotests

Change-Id: I2a14c01c7f9412459572e9960cb95a4c24e068aa
Task-number: QTBUG-21151
Reviewed-on: http://codereview.qt-project.org/5911
Reviewed-by: Aaron Kennedy <aaron.kennedy@nokia.com>

src/declarative/qml/qdeclarativeincubator.cpp

@@ -40,6 +40,7 @@
 ****************************************************************************/
 
 #include "qdeclarativeincubator.h"
+#include "qdeclarativecomponent.h"
 #include "qdeclarativeincubator_p.h"
 
 #include <private/qdeclarativecompiler_p.h>
@@ -121,7 +122,6 @@ QDeclarativeIncubatorPrivate::QDeclarativeIncubatorPrivate(QDeclarativeIncubator
 
 QDeclarativeIncubatorPrivate::~QDeclarativeIncubatorPrivate()
 {
-    clear();
 }
 
 void QDeclarativeIncubatorPrivate::clear()
@@ -236,13 +236,26 @@ void QDeclarativeIncubatorPrivate::incubate(QDeclarativeVME::Interrupt &i)
     QDeclarativeEngine *engine = component->engine;
     QDeclarativeEnginePrivate *enginePriv = QDeclarativeEnginePrivate::get(engine);
 
+    bool guardOk = vmeGuard.isOK();
+    vmeGuard.clear();
+
+    if (!guardOk) {
+        QDeclarativeError error;
+        error.setUrl(component->url);
+        error.setDescription(QDeclarativeComponent::tr("Object destroyed during incubation"));
+        errors << error;
+        progress = QDeclarativeIncubatorPrivate::Completed;
+
+        goto finishIncubate;
+    }
+
     if (progress == QDeclarativeIncubatorPrivate::Execute) {
         enginePriv->referenceScarceResources();
         result = vme.execute(&errors, i);
         enginePriv->dereferenceScarceResources();
 
         if (errors.isEmpty() && result == 0) 
-            return; // Interrupted
+            goto finishIncubate;
 
         if (result) {
             QDeclarativeData *ddata = QDeclarativeData::get(result);
@@ -288,6 +301,8 @@ void QDeclarativeIncubatorPrivate::incubate(QDeclarativeVME::Interrupt &i)
                 enginePriv->erroredBindings->removeError();
             }
         }
+    } else {
+        vmeGuard.guard(&vme);
     }
 }
 
@@ -419,6 +434,8 @@ QDeclarativeIncubator::QDeclarativeIncubator(IncubationMode mode)
 /*! \internal */
 QDeclarativeIncubator::~QDeclarativeIncubator()
 {
+    clear();
+
     delete d; d = 0;
 }
 
@@ -456,12 +473,14 @@ void QDeclarativeIncubator::clear()
 {
     Status s = status();
 
-    if (s == Loading)
-        qFatal("QDeclarativeIncubator::clear(): Clear not implemented for loading incubator");
-
     if (s == Null)
         return;
 
+    d->clear();
+
+    d->vme.reset();
+    d->vmeGuard.clear();
+
     Q_ASSERT(d->component == 0);
     Q_ASSERT(d->waitingOnMe == 0);
     Q_ASSERT(d->waitingFor.isEmpty());

src/declarative/qml/qdeclarativeincubator_p.h

@@ -79,6 +79,7 @@ class QDeclarativeIncubatorPrivate : public QDeclarativeEnginePrivate::Incubator
     QObject *result;
     QDeclarativeCompiledData *component;
     QDeclarativeVME vme;
+    QDeclarativeVMEGuard vmeGuard;
 
     typedef QDeclarativeIncubatorPrivate QIP;
     QIP *waitingOnMe;

src/declarative/qml/qdeclarativevme.cpp

@@ -130,6 +130,7 @@ bool QDeclarativeVME::initDeferred(QObject *object)
     int start = data->deferredIdx;
 
     State initState;
+    initState.flags = State::Deferred;
     initState.context = ctxt;
     initState.compiledData = comp;
     initState.instructionStream = comp->bytecode.constData() + start;
@@ -195,7 +196,6 @@ static void removeBindingOnProperty(QObject *o, int index)
 // XXX we probably need some form of "work count" here to prevent us checking this 
 // for every instruction.
 #define QML_BEGIN_INSTR_COMMON(I) { \
-    if (interrupt.shouldInterrupt()) return 0; \
     const QDeclarativeInstructionMeta<(int)QDeclarativeInstruction::I>::DataType &instr = QDeclarativeInstructionMeta<(int)QDeclarativeInstruction::I>::data(*genericInstr); \
     INSTRUCTIONSTREAM += QDeclarativeInstructionMeta<(int)QDeclarativeInstruction::I>::Size; \
     Q_UNUSED(instr);
@@ -211,6 +211,7 @@ static void removeBindingOnProperty(QObject *o, int index)
 
 #  define QML_END_INSTR(I) } \
     genericInstr = reinterpret_cast<const QDeclarativeInstruction *>(INSTRUCTIONSTREAM); \
+    if (interrupt.shouldInterrupt()) return 0; \
     goto *genericInstr->common.code;
 
 #else
@@ -219,7 +220,9 @@ static void removeBindingOnProperty(QObject *o, int index)
     QML_BEGIN_INSTR_COMMON(I)
 
 #  define QML_NEXT_INSTR(I) break;
-#  define QML_END_INSTR(I) } break;
+#  define QML_END_INSTR(I) \
+    if (interrupt.shouldInterrupt()) return 0; \
+    } break;
 #endif
 
 #define CLEAN_PROPERTY(o, index) if (fastHasBinding(o, index)) removeBindingOnProperty(o, index)
@@ -1099,23 +1102,10 @@ QObject *QDeclarativeVME::run(QList<QDeclarativeError> *errors,
 #endif
 
 exceptionExit:
-    if (!objects.isEmpty()) 
-        delete objects.at(0); // XXX What about failures in deferred creation?
-
-    // XXX does context get leaked in this case?
-
+    Q_ASSERT(!states.isEmpty());
     Q_ASSERT(!errors->isEmpty());
 
-    // Remove the QDeclarativeParserStatus and QDeclarativeAbstractBinding back pointers
-    blank(parserStatus);
-    blank(bindValues);
-
-    objects.deallocate();
-    lists.deallocate();
-    states.clear();
-    bindValues.deallocate();
-    parserStatus.deallocate();
-    finalizeCallbacks.clear();
+    reset();
 
     return 0;
 
@@ -1131,6 +1121,35 @@ QObject *QDeclarativeVME::run(QList<QDeclarativeError> *errors,
     return rv;
 }
 
+void QDeclarativeVME::reset()
+{
+    Q_ASSERT(!states.isEmpty() || objects.isEmpty());
+
+    if (!objects.isEmpty() && !(states.at(0).flags & State::Deferred))
+        delete objects.at(0); 
+
+    if (!rootContext.isNull()) 
+        rootContext->activeVME = 0;
+
+    // Remove the QDeclarativeParserStatus and QDeclarativeAbstractBinding back pointers
+    blank(parserStatus);
+    blank(bindValues);
+
+    while (componentAttached) {
+        QDeclarativeComponentAttached *a = componentAttached;
+        a->rem();
+    }
+
+    engine = 0;
+    objects.deallocate();
+    lists.deallocate();
+    bindValues.deallocate();
+    parserStatus.deallocate();
+    finalizeCallbacks.clear();
+    states.clear();
+    rootContext = 0;
+}
+
 // Must be called with a handle scope and context
 void QDeclarativeScriptData::initialize(QDeclarativeEngine *engine)
 {
@@ -1242,6 +1261,16 @@ void **QDeclarativeVME::instructionJumpTable()
 
 bool QDeclarativeVME::complete(const Interrupt &interrupt) 
 {
+    Q_ASSERT(engine ||
+             (bindValues.isEmpty() &&
+              parserStatus.isEmpty() &&
+              componentAttached == 0 &&
+              rootContext.isNull() &&
+              finalizeCallbacks.isEmpty()));
+
+    if (!engine)
+        return true;
+
     ActiveVMERestorer restore(this, QDeclarativeEnginePrivate::get(engine));
 
     while (!bindValues.isEmpty()) {
@@ -1284,8 +1313,7 @@ bool QDeclarativeVME::complete(const Interrupt &interrupt)
             return false;
     }
 
-    // XXX (what if its deleted?)
-    if (rootContext) 
+    if (!rootContext.isNull()) 
         rootContext->activeVME = 0;
 
     for (int ii = 0; ii < finalizeCallbacks.count(); ++ii) {
@@ -1298,6 +1326,8 @@ bool QDeclarativeVME::complete(const Interrupt &interrupt)
     }
     finalizeCallbacks.clear();
 
+    reset();
+
     return true;
 }
 
@@ -1317,4 +1347,55 @@ void QDeclarativeVME::blank(QFiniteStack<QDeclarativeParserStatus *> &pss)
     }
 }
 
+QDeclarativeVMEGuard::QDeclarativeVMEGuard()
+: m_objectCount(0), m_objects(0), m_contextCount(0), m_contexts(0)
+{
+}
+
+QDeclarativeVMEGuard::~QDeclarativeVMEGuard()
+{
+    clear();
+}
+
+void QDeclarativeVMEGuard::guard(QDeclarativeVME *vme)
+{
+    clear();
+
+    m_objectCount = vme->objects.count();
+    m_objects = new QDeclarativeGuard<QObject>[m_objectCount];
+    for (int ii = 0; ii < m_objectCount; ++ii)
+        m_objects[ii] = vme->objects[ii];
+
+    m_contextCount = (vme->rootContext.isNull())?0:1 + vme->states.count();
+    m_contexts = new QDeclarativeGuardedContextData[m_contextCount];
+    for (int ii = 0; ii < vme->states.count(); ++ii) 
+        m_contexts[ii] = vme->states.at(ii).context;
+    if (!vme->rootContext.isNull())
+        m_contexts[m_contextCount - 1] = vme->rootContext.contextData();
+}
+
+void QDeclarativeVMEGuard::clear()
+{
+    delete [] m_objects;
+    delete [] m_contexts;
+
+    m_objectCount = 0;
+    m_objects = 0;
+    m_contextCount = 0;
+    m_contexts = 0;
+}
+
+bool QDeclarativeVMEGuard::isOK() const
+{
+    for (int ii = 0; ii < m_objectCount; ++ii)
+        if (m_objects[ii].isNull())
+            return false;
+
+    for (int ii = 0; ii < m_contextCount; ++ii)
+        if (m_contexts[ii].isNull())
+            return false;
+
+    return true;
+}
+
 QT_END_NAMESPACE

src/declarative/qml/qdeclarativevme_p.h

@@ -120,11 +120,14 @@ class QDeclarativeVME
 
     void init(QDeclarativeContextData *, QDeclarativeCompiledData *, int start);
     bool initDeferred(QObject *);
+    void reset();
 
     QObject *execute(QList<QDeclarativeError> *errors, const Interrupt & = Interrupt());
     bool complete(const Interrupt & = Interrupt());
 
 private:
+    friend class QDeclarativeVMEGuard;
+
     QObject *run(QList<QDeclarativeError> *errors, const Interrupt &
 #ifdef QML_THREADED_VME_INTERPRETER
                  , void ***storeJumpTable = 0
@@ -138,15 +141,19 @@ class QDeclarativeVME
 #endif
 
     QDeclarativeEngine *engine;
+
     QFiniteStack<QObject *> objects;
     QFiniteStack<QDeclarativeVMETypes::List> lists;
 
     QFiniteStack<QDeclarativeAbstractBinding *> bindValues;
     QFiniteStack<QDeclarativeParserStatus *> parserStatus;
-    QDeclarativeContextData *rootContext;
+    QDeclarativeGuardedContextData rootContext;
 
     struct State {
-        State() : context(0), compiledData(0), instructionStream(0) {}
+        enum Flag { Deferred = 0x00000001 };
+
+        State() : flags(0), context(0), compiledData(0), instructionStream(0) {}
+        quint32 flags;
         QDeclarativeContextData *context;
         QDeclarativeCompiledData *compiledData;
         const char *instructionStream;
@@ -159,6 +166,27 @@ class QDeclarativeVME
     static void blank(QFiniteStack<QDeclarativeAbstractBinding *> &);
 };
 
+// Used to check that a QDeclarativeVME that is interrupted mid-execution
+// is still valid.  Checks all the objects and contexts have not been 
+// deleted.
+class QDeclarativeVMEGuard
+{
+public:
+    QDeclarativeVMEGuard();
+    ~QDeclarativeVMEGuard();
+
+    void guard(QDeclarativeVME *);
+    void clear();
+
+    bool isOK() const;
+
+private:
+    int m_objectCount;
+    QDeclarativeGuard<QObject> *m_objects;
+    int m_contextCount;
+    QDeclarativeGuardedContextData *m_contexts;
+};
+
 QDeclarativeVME::Interrupt::Interrupt()
 : mode(None)
 {

tests/auto/declarative/qdeclarativeecmascript/tst_qdeclarativeecmascript.cpp

@@ -389,7 +389,7 @@ void tst_qdeclarativeecmascript::methods()
 void tst_qdeclarativeecmascript::bindingLoop()
 {
     QDeclarativeComponent component(&engine, TEST_FILE("bindingLoop.qml"));
-    QString warning = component.url().toString() + ":9:9: QML MyQmlObject: Binding loop detected for property \"stringProperty\"";
+    QString warning = component.url().toString() + ":5:9: QML MyQmlObject: Binding loop detected for property \"stringProperty\"";
     QTest::ignoreMessage(QtWarningMsg, warning.toLatin1().constData());
     QObject *object = component.create();
     QVERIFY(object != 0);

tests/auto/declarative/qdeclarativeincubator/data/clear.qml

@@ -0,0 +1,8 @@
+import QtQuick 2.0
+import Qt.test 1.0
+
+Item {
+    SelfRegistering {
+        value: 11
+    }
+}

tests/auto/declarative/qdeclarativeincubator/data/objectDeleted.errors.txt

@@ -0,0 +1 @@
+-1:-1:Object destroyed during incubation

tests/auto/declarative/qdeclarativeincubator/data/objectDeleted.qml

@@ -0,0 +1,8 @@
+import QtQuick 2.0
+import Qt.test 1.0
+
+Item {
+    SelfRegistering {
+        value: 11
+    }
+}

tests/auto/declarative/qdeclarativeincubator/qdeclarativeincubator.pro

@@ -0,0 +1,13 @@
+load(qttest_p4)
+contains(QT_CONFIG,declarative): QT += declarative network widgets
+macx:CONFIG -= app_bundle
+
+SOURCES += tst_qdeclarativeincubator.cpp \
+           testtypes.cpp
+HEADERS += testtypes.h
+
+DEFINES += SRCDIR=\\\"$$PWD\\\"
+
+CONFIG += parallel_test
+
+QT += core-private gui-private v8-private declarative-private