Make QQmlIncubatorPrivate refcounted

This fixes possible bugs and crashes where the incubator
could get deleted through GC while constructing the
component.

Change-Id: Ibe0c5d4e172f0b5505ace0c3ea0369169b8b48a5
Reviewed-by: Simon Hausmann <simon.hausmann@digia.com>

diff --git a/src/qml/qml/qqmlcomponent.cpp b/src/qml/qml/qqmlcomponent.cpp
index e0360b6..6d194a6 100644 (file)
--- a/src/qml/qml/qqmlcomponent.cpp
+++ b/src/qml/qml/qqmlcomponent.cpp
@@ -1094,7 +1094,7 @@ void QQmlComponent::create(QQmlIncubator &incubator, QQmlContext *context,
     }
 
     incubator.clear();
-    QQmlIncubatorPrivate *p = incubator.d;
+    QExplicitlySharedDataPointer<QQmlIncubatorPrivate> p(incubator.d);
 
     QQmlEnginePrivate *enginePriv = QQmlEnginePrivate::get(d->engine);
 

diff --git a/src/qml/qml/qqmlengine_p.h b/src/qml/qml/qqmlengine_p.h
index c67e45a..5a2d6c4 100644 (file)
--- a/src/qml/qml/qqmlengine_p.h
+++ b/src/qml/qml/qqmlengine_p.h
@@ -196,9 +196,9 @@ public:
         return uniqueId++;
     }
 
-    // Unfortunate workaround to avoid a circular dependency between 
+    // Unfortunate workaround to avoid a circular dependency between
     // qqmlengine_p.h and qqmlincubator_p.h
-    struct Incubator {
+    struct Incubator : public QSharedData {
         QIntrusiveListNode next;
         // Unfortunate workaround for MSVC
         QIntrusiveListNode nextWaitingFor;

diff --git a/src/qml/qml/qqmlincubator.cpp b/src/qml/qml/qqmlincubator.cpp
index 39c7f29..71adf44 100644 (file)
--- a/src/qml/qml/qqmlincubator.cpp
+++ b/src/qml/qml/qqmlincubator.cpp
@@ -52,7 +52,7 @@
 //     async if nested cases
 void QQmlEnginePrivate::incubate(QQmlIncubator &i, QQmlContextData *forContext)
 {
-    QQmlIncubatorPrivate *p = i.d;
+    QExplicitlySharedDataPointer<QQmlIncubatorPrivate> p(i.d);
 
     QQmlIncubator::IncubationMode mode = i.incubationMode();
 
@@ -63,7 +63,7 @@ void QQmlEnginePrivate::incubate(QQmlIncubator &i, QQmlContextData *forContext)
         mode = QQmlIncubator::Synchronous;
 
         // Need to find the first constructing context and see if it is asynchronous
-        QQmlIncubatorPrivate *parentIncubator = 0;
+        QExplicitlySharedDataPointer<QQmlIncubatorPrivate> parentIncubator;
         QQmlContextData *cctxt = forContext;
         while (cctxt) {
             if (cctxt->activeVMEData) {
@@ -76,7 +76,7 @@ void QQmlEnginePrivate::incubate(QQmlIncubator &i, QQmlContextData *forContext)
         if (parentIncubator && parentIncubator->isAsynchronous) {
             mode = QQmlIncubator::Asynchronous;
             p->waitingOnMe = parentIncubator;
-            parentIncubator->waitingFor.insert(p);
+            parentIncubator->waitingFor.insert(p.data());
         }
     }
 
@@ -85,8 +85,7 @@ void QQmlEnginePrivate::incubate(QQmlIncubator &i, QQmlContextData *forContext)
     inProgressCreations++;
 
     if (mode == QQmlIncubator::Synchronous) {
-        typedef QQmlIncubatorPrivate IP;
-        QRecursionWatcher<IP, &IP::recursion> watcher(p);
+        QRecursionWatcher<QQmlIncubatorPrivate, &QQmlIncubatorPrivate::recursion> watcher(p.data());
 
         p->changeStatus(QQmlIncubator::Loading);
 
@@ -95,14 +94,14 @@ void QQmlEnginePrivate::incubate(QQmlIncubator &i, QQmlContextData *forContext)
             p->incubate(i);
         }
     } else {
-        incubatorList.insert(p);
+        incubatorList.insert(p.data());
         incubatorCount++;
 
         p->vmeGuard.guard(&p->vme);
         p->changeStatus(QQmlIncubator::Loading);
 
         if (incubationController)
-            incubationController->incubatingObjectCountChanged(incubatorCount);
+             incubationController->incubatingObjectCountChanged(incubatorCount);
     }
 }
 
@@ -132,15 +131,15 @@ QQmlIncubationController *QQmlEngine::incubationController() const
     return d->incubationController;
 }
 
-QQmlIncubatorPrivate::QQmlIncubatorPrivate(QQmlIncubator *q, 
-                                                           QQmlIncubator::IncubationMode m)
-: q(q), status(QQmlIncubator::Null), mode(m), isAsynchronous(false), progress(Execute),
-  result(0), compiledData(0), vme(this), waitingOnMe(0)
+QQmlIncubatorPrivate::QQmlIncubatorPrivate(QQmlIncubator *q, QQmlIncubator::IncubationMode m)
+    : q(q), status(QQmlIncubator::Null), mode(m), isAsynchronous(false), progress(Execute),
+      result(0), compiledData(0), vme(this), waitingOnMe(0)
 {
 }
 
 QQmlIncubatorPrivate::~QQmlIncubatorPrivate()
 {
+    clear();
 }
 
 void QQmlIncubatorPrivate::clear()
@@ -154,7 +153,7 @@ void QQmlIncubatorPrivate::clear()
         enginePriv->incubatorCount--;
         QQmlIncubationController *controller = enginePriv->incubationController;
         if (controller)
-            controller->incubatingObjectCountChanged(enginePriv->incubatorCount);
+             controller->incubatingObjectCountChanged(enginePriv->incubatorCount);
     } else if (compiledData) {
         compiledData->release();
         compiledData = 0;
@@ -240,10 +239,7 @@ Return the number of objects currently incubating.
 */
 int QQmlIncubationController::incubatingObjectCount() const
 {
-    if (d)
-        return d->incubatorCount;
-    else 
-        return 0;
+    return d ? d->incubatorCount : 0;
 }
 
 /*!
@@ -261,10 +257,12 @@ void QQmlIncubatorPrivate::incubate(QQmlVME::Interrupt &i)
 {
     if (!compiledData)
         return;
+
     QML_MEMORY_SCOPE_URL(compiledData->url);
 
-    typedef QQmlIncubatorPrivate IP;
-    QRecursionWatcher<IP, &IP::recursion> watcher(this);
+    QExplicitlySharedDataPointer<QQmlIncubatorPrivate> protectThis(this);
+
+    QRecursionWatcher<QQmlIncubatorPrivate, &QQmlIncubatorPrivate::recursion> watcher(this);
 
     QQmlEngine *engine = compiledData->engine;
     QQmlEnginePrivate *enginePriv = QQmlEnginePrivate::get(engine);
@@ -301,7 +299,8 @@ void QQmlIncubatorPrivate::incubate(QQmlVME::Interrupt &i)
             ddata->indestructible = true;
             ddata->explicitIndestructibleSet = true;
             ddata->rootObjectInCreation = false;
-            q->setInitialState(result);
+            if (q)
+                q->setInitialState(result);
         }
 
         if (watcher.hasRecursed())
@@ -337,13 +336,11 @@ void QQmlIncubatorPrivate::incubate(QQmlVME::Interrupt &i)
 
 finishIncubate:
     if (progress == QQmlIncubatorPrivate::Completed && waitingFor.isEmpty()) {
-        typedef QQmlIncubatorPrivate IP;
-
-        QQmlIncubatorPrivate *isWaiting = waitingOnMe;
+        QExplicitlySharedDataPointer<QQmlIncubatorPrivate> isWaiting = waitingOnMe;
         clear();
 
         if (isWaiting) {
-            QRecursionWatcher<IP, &IP::recursion> watcher(isWaiting);
+            QRecursionWatcher<QQmlIncubatorPrivate, &QQmlIncubatorPrivate::recursion> watcher(isWaiting.data());
             changeStatus(calculateStatus());
             if (!watcher.hasRecursed())
                 isWaiting->incubate(i);
@@ -369,14 +366,13 @@ Incubate objects for \a msecs, or until there are no more objects to incubate.
 */
 void QQmlIncubationController::incubateFor(int msecs)
 {
-    if (!d || d->incubatorCount == 0)
+    if (!d || !d->incubatorCount)
         return;
 
     QQmlVME::Interrupt i(msecs * 1000000);
     i.reset();
     do {
-        QQmlIncubatorPrivate *p = (QQmlIncubatorPrivate*)d->incubatorList.first();
-        p->incubate(i);
+        static_cast<QQmlIncubatorPrivate*>(d->incubatorList.first())->incubate(i);
     } while (d && d->incubatorCount != 0 && !i.shouldInterrupt());
 }
 
@@ -389,14 +385,13 @@ the bool pointed to by \a flag to false when it wants incubation to be interrupt
 */
 void QQmlIncubationController::incubateWhile(volatile bool *flag, int msecs)
 {
-    if (!d || d->incubatorCount == 0)
+    if (!d || !d->incubatorCount)
         return;
 
     QQmlVME::Interrupt i(flag, msecs * 1000000);
     i.reset();
     do {
-        QQmlIncubatorPrivate *p = (QQmlIncubatorPrivate*)d->incubatorList.first();
-        p->incubate(i);
+        static_cast<QQmlIncubatorPrivate*>(d->incubatorList.first())->incubate(i);
     } while (d && d->incubatorCount != 0 && !i.shouldInterrupt());
 }
 
@@ -487,16 +482,20 @@ or stutters into the application, should use the AsynchronousIfNested incubation
 Create a new incubator with the specified \a mode
 */
 QQmlIncubator::QQmlIncubator(IncubationMode mode)
-: d(new QQmlIncubatorPrivate(this, mode))
+    : d(new QQmlIncubatorPrivate(this, mode))
 {
+    d->ref.ref();
 }
 
 /*! \internal */
 QQmlIncubator::~QQmlIncubator()
 {
-    clear();
+    d->q = 0;
 
-    delete d; d = 0;
+    if (!d->ref.deref()) {
+        delete d;
+    }
+    d = 0;
 }
 
 /*!
@@ -531,8 +530,7 @@ Ready state, the created object is \b not deleted.
 */
 void QQmlIncubator::clear()
 {
-    typedef QQmlIncubatorPrivate IP;
-    QRecursionWatcher<IP, &IP::recursion> watcher(d);
+    QRecursionWatcher<QQmlIncubatorPrivate, &QQmlIncubatorPrivate::recursion> watcher(d);
 
     Status s = status();
 
@@ -550,16 +548,18 @@ void QQmlIncubator::clear()
     d->clear();
 
     // if we're waiting on any incubators then they should be cleared too.
-    while (d->waitingFor.first())
-        static_cast<QQmlIncubatorPrivate*>(d->waitingFor.first())->q->clear();
+    while (d->waitingFor.first()) {
+        QQmlIncubator * i = static_cast<QQmlIncubatorPrivate*>(d->waitingFor.first())->q;
+        if (i)
+            i->clear();
+    }
 
     d->vme.reset();
     d->vmeGuard.clear();
 
     Q_ASSERT(d->compiledData == 0);
-    Q_ASSERT(d->waitingOnMe == 0);
+    Q_ASSERT(d->waitingOnMe.data() == 0);
     Q_ASSERT(d->waitingFor.isEmpty());
-    Q_ASSERT(!d->nextWaitingFor.isInList());
 
     d->errors.clear();
     d->progress = QQmlIncubatorPrivate::Execute;
@@ -656,8 +656,10 @@ Return the incubated object if the status is Ready, otherwise 0.
 */
 QObject *QQmlIncubator::object() const
 {
-    if (status() != Ready) return 0;
-    else return d->result;
+    if (status() != Ready)
+        return 0;
+    else
+        return d->result;
 }
 
 /*!
@@ -690,15 +692,15 @@ void QQmlIncubatorPrivate::changeStatus(QQmlIncubator::Status s)
         return;
 
     status = s;
-    q->statusChanged(status);
+    if (q)
+        q->statusChanged(status);
 }
 
 QQmlIncubator::Status QQmlIncubatorPrivate::calculateStatus() const
 {
     if (!errors.isEmpty()) 
         return QQmlIncubator::Error;
-    else if (result && progress == QQmlIncubatorPrivate::Completed && 
-             waitingFor.isEmpty()) 
+    else if (result && progress == QQmlIncubatorPrivate::Completed && waitingFor.isEmpty())
         return QQmlIncubator::Ready;
     else if (compiledData)
         return QQmlIncubator::Loading;

diff --git a/src/qml/qml/qqmlincubator_p.h b/src/qml/qml/qqmlincubator_p.h
index 229919f..58e9c93 100644 (file)
--- a/src/qml/qml/qqmlincubator_p.h
+++ b/src/qml/qml/qqmlincubator_p.h
@@ -91,7 +91,7 @@ public:
     QQmlVME vme;
     QQmlVMEGuard vmeGuard;
 
-    QQmlIncubatorPrivate *waitingOnMe;
+    QExplicitlySharedDataPointer<QQmlIncubatorPrivate> waitingOnMe;
     typedef QQmlEnginePrivate::Incubator QIPBase;
     QIntrusiveList<QIPBase, &QIPBase::nextWaitingFor> waitingFor;