diff options
author | Simon Hausmann <simon.hausmann@digia.com> | 2013-05-23 22:13:42 +0200 |
---|---|---|
committer | Lars Knoll <lars.knoll@digia.com> | 2013-05-24 09:51:51 +0200 |
commit | fa2d572d5d202b05ed1908ea1119a1995960ce1f (patch) | |
tree | a53782ee1b94ac0856e6336b2386e717f4102d4e /src | |
parent | ee90b4efa521287e96b7ce23a2bc1d56e1b526fd (diff) |
Fix QObject ownership
Implement the JS vs. C++ ownership policy. QV4::WeakValue is a weak
reference that's now used instead of PersistentValue in QQmlData.
Whether or not to delete the QObject when the JS object is garbage
collected is decided in the ~QObjectWrapper destructor (conveniently).
Fixes four ownership tests.
Change-Id: Iedeb498f510295b5e656d0bb3b324084efa98f0f
Reviewed-by: Lars Knoll <lars.knoll@digia.com>
Diffstat (limited to 'src')
-rw-r--r-- | src/qml/qml/qqmldata_p.h | 2 | ||||
-rw-r--r-- | src/qml/qml/v4/qv4mm.cpp | 24 | ||||
-rw-r--r-- | src/qml/qml/v4/qv4mm_p.h | 1 | ||||
-rw-r--r-- | src/qml/qml/v4/qv4value.cpp | 87 | ||||
-rw-r--r-- | src/qml/qml/v4/qv4value_p.h | 37 | ||||
-rw-r--r-- | src/qml/qml/v8/qv8engine.cpp | 8 | ||||
-rw-r--r-- | src/qml/qml/v8/qv8engine_p.h | 2 | ||||
-rw-r--r-- | src/qml/qml/v8/qv8qobjectwrapper.cpp | 14 |
8 files changed, 155 insertions, 20 deletions
diff --git a/src/qml/qml/qqmldata_p.h b/src/qml/qml/qqmldata_p.h index 096e358a73..98407e53b5 100644 --- a/src/qml/qml/qqmldata_p.h +++ b/src/qml/qml/qqmldata_p.h @@ -179,7 +179,7 @@ public: unsigned int deferredIdx; quint32 v8objectid; - QV4::PersistentValue v8object; + QV4::WeakValue v8object; QQmlPropertyCache *propertyCache; diff --git a/src/qml/qml/v4/qv4mm.cpp b/src/qml/qml/v4/qv4mm.cpp index 9c0e4ad1e0..cb24c3c134 100644 --- a/src/qml/qml/v4/qv4mm.cpp +++ b/src/qml/qml/v4/qv4mm.cpp @@ -131,6 +131,7 @@ MemoryManager::MemoryManager() : m_d(new Data(true)) , m_contextList(0) , m_persistentValues(0) + , m_weakValues(0) { setEnableGC(true); #ifdef V4_USE_VALGRIND @@ -242,18 +243,16 @@ void MemoryManager::mark() it.key()->mark(); PersistentValuePrivate *persistent = m_persistentValues; - PersistentValuePrivate **last = &m_persistentValues; while (persistent) { if (!persistent->refcount) { - *last = persistent->next; PersistentValuePrivate *n = persistent->next; + persistent->removeFromList(); delete persistent; persistent = n; continue; } if (Managed *m = persistent->value.asManaged()) m->mark(); - last = &persistent->next; persistent = persistent->next; } @@ -289,6 +288,25 @@ void MemoryManager::mark() std::size_t MemoryManager::sweep() { + PersistentValuePrivate *weak = m_weakValues; + while (weak) { + if (!weak->refcount) { + PersistentValuePrivate *n = weak->next; + weak->removeFromList(); + delete weak; + weak = n; + continue; + } + if (Managed *m = weak->value.asManaged()) { + if (!m->markBit) { + weak->value = Value::emptyValue(); + weak->removeFromList(); + continue; + } + } + weak = weak->next; + } + std::size_t freedCount = 0; for (QVector<Data::Chunk>::iterator i = m_d->heapChunks.begin(), ei = m_d->heapChunks.end(); i != ei; ++i) diff --git a/src/qml/qml/v4/qv4mm_p.h b/src/qml/qml/v4/qv4mm_p.h index fe858d8f7f..cb56c8e158 100644 --- a/src/qml/qml/v4/qv4mm_p.h +++ b/src/qml/qml/v4/qv4mm_p.h @@ -137,6 +137,7 @@ protected: ExecutionContext *m_contextList; public: PersistentValuePrivate *m_persistentValues; + PersistentValuePrivate *m_weakValues; }; inline ExecutionContext *MemoryManager::allocContext(uint size) diff --git a/src/qml/qml/v4/qv4value.cpp b/src/qml/qml/v4/qv4value.cpp index 6e69d05744..a86e886c6c 100644 --- a/src/qml/qml/v4/qv4value.cpp +++ b/src/qml/qml/v4/qv4value.cpp @@ -284,7 +284,69 @@ PersistentValue::~PersistentValue() d->deref(); } -PersistentValuePrivate::PersistentValuePrivate(const Value &v) +WeakValue::WeakValue(const Value &val) + : d(new PersistentValuePrivate(val, /*weak*/true)) +{ +} + +WeakValue::WeakValue(const WeakValue &other) + : d(other.d) +{ + if (d) + d->ref(); +} + +WeakValue &WeakValue::operator=(const WeakValue &other) +{ + if (d == other.d) + return *this; + + // the memory manager cleans up those with a refcount of 0 + + if (d) + d->deref(); + d = other.d; + if (d) + d->ref(); +} + +WeakValue &WeakValue::operator =(const Value &other) +{ + if (!d) { + d = new PersistentValuePrivate(other, /*weak*/true); + return *this; + } + d->value = other; + Managed *m = d->value.asManaged(); + if (!d->prev) { + if (m) { + ExecutionEngine *engine = m->engine(); + if (engine) { + d->prev = &engine->memoryManager->m_weakValues; + d->next = engine->memoryManager->m_weakValues; + *d->prev = d; + if (d->next) + d->next->prev = &d->next; + } + } + } else if (!m) { + if (d->next) + d->next->prev = d->prev; + *d->prev = d->next; + d->prev = 0; + d->next = 0; + } +} + + +WeakValue::~WeakValue() +{ + if (d) + d->deref(); +} + + +PersistentValuePrivate::PersistentValuePrivate(const Value &v, bool weak) : value(v) , refcount(1) , prev(0) @@ -296,24 +358,33 @@ PersistentValuePrivate::PersistentValuePrivate(const Value &v) ExecutionEngine *engine = m->engine(); if (engine) { - prev = &engine->memoryManager->m_persistentValues; - next = engine->memoryManager->m_persistentValues; + PersistentValuePrivate **listRoot = weak ? &engine->memoryManager->m_weakValues : &engine->memoryManager->m_persistentValues; + + prev = listRoot; + next = *listRoot; *prev = this; if (next) next->prev = &this->next; } } +void PersistentValuePrivate::removeFromList() +{ + if (prev) { + if (next) + next->prev = prev; + *prev = next; + prev = 0; + } +} + void PersistentValuePrivate::deref() { // if engine is not 0, they are registered with the memory manager // and will get cleaned up in the next gc run if (!--refcount) { - if (prev) { - if (next) - next->prev = prev; - *prev = next; - } + removeFromList(); delete this; } } + diff --git a/src/qml/qml/v4/qv4value_p.h b/src/qml/qml/v4/qv4value_p.h index ea1f1f86d3..673fa7c91d 100644 --- a/src/qml/qml/v4/qv4value_p.h +++ b/src/qml/qml/v4/qv4value_p.h @@ -563,12 +563,13 @@ inline Value Managed::call(ExecutionContext *context, const Value &thisObject, V struct PersistentValuePrivate { - PersistentValuePrivate(const Value &v); + PersistentValuePrivate(const Value &v, bool weak = false); Value value; - int refcount; + uint refcount; PersistentValuePrivate **prev; PersistentValuePrivate *next; + void removeFromList(); void ref() { ++refcount; } void deref(); }; @@ -605,6 +606,38 @@ private: PersistentValuePrivate *d; }; +class Q_QML_EXPORT WeakValue +{ +public: + WeakValue() : d(0) {} + WeakValue(const Value &val); + WeakValue(const WeakValue &other); + WeakValue &operator=(const WeakValue &other); + WeakValue &operator=(const Value &other); + ~WeakValue(); + + Value value() const { + return d ? d->value : Value::emptyValue(); + } + + ExecutionEngine *engine() { + if (!d) + return 0; + Managed *m = d->value.asManaged(); + return m ? m->engine() : 0; + } + + operator Value() const { return value(); } + + bool isEmpty() const { return !d || d->value.isEmpty(); } + void clear() { + *this = WeakValue(); + } + +private: + PersistentValuePrivate *d; +}; + } // namespace QV4 QT_END_NAMESPACE diff --git a/src/qml/qml/v8/qv8engine.cpp b/src/qml/qml/v8/qv8engine.cpp index e0a7766886..8244d02600 100644 --- a/src/qml/qml/v8/qv8engine.cpp +++ b/src/qml/qml/v8/qv8engine.cpp @@ -722,7 +722,7 @@ void QV8Engine::setExtensionData(int index, Deletable *data) } -QV4::PersistentValue *QV8Engine::findOwnerAndStrength(QObject *object, bool *shouldBeStrong) +QV4::WeakValue *QV8Engine::findOwnerAndStrength(QObject *object, bool *shouldBeStrong) { QQmlData *data = QQmlData::get(object); if (data && data->rootObjectInCreation) { // When the object is still being created it may not show up to the GC. @@ -765,7 +765,7 @@ void QV8Engine::addRelationshipForGC(QObject *object, const QV4::PersistentValue return; bool handleShouldBeStrong = false; - QV4::PersistentValue *implicitOwner = findOwnerAndStrength(object, &handleShouldBeStrong); + QV4::WeakValue *implicitOwner = findOwnerAndStrength(object, &handleShouldBeStrong); if (handleShouldBeStrong) { // ### FIXME // v8::V8::AddImplicitReferences(m_strongReferencer, &handle, 1); @@ -782,8 +782,8 @@ void QV8Engine::addRelationshipForGC(QObject *object, QObject *other) return; bool handleShouldBeStrong = false; - QV4::PersistentValue *implicitOwner = findOwnerAndStrength(object, &handleShouldBeStrong); - QV4::PersistentValue handle = QQmlData::get(other, true)->v8object; + QV4::WeakValue *implicitOwner = findOwnerAndStrength(object, &handleShouldBeStrong); + QV4::WeakValue handle = QQmlData::get(other, true)->v8object; if (handle.isEmpty()) // no JS data to keep alive. return; // ### FIXME diff --git a/src/qml/qml/v8/qv8engine_p.h b/src/qml/qml/v8/qv8engine_p.h index 254445d0b0..24096b7f68 100644 --- a/src/qml/qml/v8/qv8engine_p.h +++ b/src/qml/qml/v8/qv8engine_p.h @@ -437,7 +437,7 @@ private: QVariantMap variantMapFromJS(QV4::Object *object, V8ObjectSet &visitedObjects); QVariant variantFromJS(const QV4::Value &value, V8ObjectSet &visitedObjects); - static QV4::PersistentValue *findOwnerAndStrength(QObject *object, bool *shouldBeStrong); + static QV4::WeakValue *findOwnerAndStrength(QObject *object, bool *shouldBeStrong); Q_DISABLE_COPY(QV8Engine) }; diff --git a/src/qml/qml/v8/qv8qobjectwrapper.cpp b/src/qml/qml/v8/qv8qobjectwrapper.cpp index 2303df3116..73d396d262 100644 --- a/src/qml/qml/v8/qv8qobjectwrapper.cpp +++ b/src/qml/qml/v8/qv8qobjectwrapper.cpp @@ -90,6 +90,18 @@ QObjectWrapper::QObjectWrapper(ExecutionEngine *engine, QObject *object) QObjectWrapper::~QObjectWrapper() { + if (!object) + return; + QQmlData *ddata = QQmlData::get(object, false); + if (!ddata) + return; + if (!object->parent() && !ddata->indestructible) { + // This object is notionally destroyed now + if (ddata->ownContext && ddata->context) + ddata->context->emitDestruction(); + ddata->isQueuedForDeletion = true; + object->deleteLater(); + } } QV4::Value QObjectWrapper::method_toString(QV4::SimpleCallContext *ctx) @@ -270,7 +282,7 @@ public: delete this; } - QV4::PersistentValue v8object; + QV4::WeakValue v8object; QV8QObjectWrapper *wrapper; }; |