From: Simon Hausmann Date: Tue, 13 Dec 2011 16:18:45 +0000 (+0100) Subject: V8: Remove extra V8::Context allocated for expressing strong references X-Git-Tag: qt-v5.0.0-alpha1~828 X-Git-Url: http://review.tizen.org/git/?a=commitdiff_plain;h=d70cca804e88110eae32f92baff7cee957a6a531;p=profile%2Fivi%2Fqtdeclarative.git V8: Remove extra V8::Context allocated for expressing strong references Moved the Referencer code into QV8Engine and re-used the available v8 context there. That also makes things a bit cleaner in the sense that now references from one object to another are guaranteed to be within the same context. Previously some strong references would be across contexts that do not actually share a security token. Change-Id: I717b27a4d96323feb570023d4d84f2b2176d1a84 Reviewed-by: Aaron Kennedy Reviewed-by: Chris Adams --- diff --git a/src/declarative/qml/qdeclarativevmemetaobject.cpp b/src/declarative/qml/qdeclarativevmemetaobject.cpp index 0ff6ad5..a37be92 100644 --- a/src/declarative/qml/qdeclarativevmemetaobject.cpp +++ b/src/declarative/qml/qdeclarativevmemetaobject.cpp @@ -1027,13 +1027,14 @@ void QDeclarativeVMEMetaObject::VarPropertiesWeakReferenceCallback(v8::Persisten vmemo->varProperties.Clear(); } -void QDeclarativeVMEMetaObject::GcPrologueCallback(QV8GCCallback::Referencer *r, QV8GCCallback::Node *node) +void QDeclarativeVMEMetaObject::GcPrologueCallback(QV8GCCallback::Node *node) { QDeclarativeVMEMetaObject *vmemo = static_cast(node); Q_ASSERT(vmemo); - if (!vmemo->varPropertiesInitialized || vmemo->varProperties.IsEmpty()) + if (!vmemo->varPropertiesInitialized || vmemo->varProperties.IsEmpty() || !vmemo->ctxt || !vmemo->ctxt->engine) return; - r->addRelationship(vmemo->object, vmemo->varProperties); + QDeclarativeEnginePrivate *ep = QDeclarativeEnginePrivate::get(vmemo->ctxt->engine); + ep->v8engine()->addRelationshipForGC(vmemo->object, vmemo->varProperties); } bool QDeclarativeVMEMetaObject::aliasTarget(int index, QObject **target, int *coreIndex, int *valueTypeIndex) const diff --git a/src/declarative/qml/qdeclarativevmemetaobject_p.h b/src/declarative/qml/qdeclarativevmemetaobject_p.h index 22cfbb1..2a4fd08 100644 --- a/src/declarative/qml/qdeclarativevmemetaobject_p.h +++ b/src/declarative/qml/qdeclarativevmemetaobject_p.h @@ -179,7 +179,7 @@ private: int firstVarPropertyIndex; bool varPropertiesInitialized; static void VarPropertiesWeakReferenceCallback(v8::Persistent object, void* parameter); - static void GcPrologueCallback(QV8GCCallback::Referencer *r, QV8GCCallback::Node *node); + static void GcPrologueCallback(QV8GCCallback::Node *node); inline void allocateVarPropertiesArray(); inline void ensureVarPropertiesAllocated(); diff --git a/src/declarative/qml/v8/qv8engine.cpp b/src/declarative/qml/v8/qv8engine.cpp index 7c3dc1d..d89326a 100644 --- a/src/declarative/qml/v8/qv8engine.cpp +++ b/src/declarative/qml/v8/qv8engine.cpp @@ -141,6 +141,7 @@ QV8Engine::QV8Engine(QJSEngine* qq, QJSEngine::ContextOwnership ownership) v8::V8::SetUserObjectComparisonCallbackFunction(ObjectComparisonCallback); QV8GCCallback::registerGcPrologueCallback(); + m_strongReferencer = qPersistentNew(v8::Object::New()); m_stringWrapper.init(); m_contextWrapper.init(this); @@ -177,6 +178,8 @@ QV8Engine::~QV8Engine() invalidateAllValues(); clearExceptions(); + qPersistentDispose(m_strongReferencer); + m_sequenceWrapper.destroy(); m_valueTypeWrapper.destroy(); m_variantWrapper.destroy(); @@ -751,6 +754,37 @@ double QV8Engine::qtDateTimeToJsDate(const QDateTime &dt) return QV8DateConverter::JSC::gregorianDateTimeToMS(tm, time.msec()); } +v8::Persistent *QV8Engine::findOwnerAndStrength(QObject *object, bool *shouldBeStrong) +{ + QObject *parent = object->parent(); + if (!parent) { + // if the object has JS ownership, the object's v8object owns the lifetime of the persistent value. + if (QDeclarativeEngine::objectOwnership(object) == QDeclarativeEngine::JavaScriptOwnership) { + *shouldBeStrong = false; + return &(QDeclarativeData::get(object)->v8object); + } + + // no parent, and has CPP ownership - doesn't have an implicit parent. + *shouldBeStrong = true; + return 0; + } + + // if it is owned by CPP, it's root parent may still be owned by JS. + // in that case, the owner of the persistent handle is the root parent's v8object. + while (parent->parent()) + parent = parent->parent(); + + if (QDeclarativeEngine::objectOwnership(parent) == QDeclarativeEngine::JavaScriptOwnership) { + // root parent is owned by JS. It's v8object owns the persistent value in question. + *shouldBeStrong = false; + return &(QDeclarativeData::get(parent)->v8object); + } else { + // root parent has CPP ownership. The persistent value should not be made weak. + *shouldBeStrong = true; + return 0; + } +} + QDateTime QV8Engine::qtDateTimeFromJsDate(double jsDate) { // from QScriptEngine::MsToDateTime() @@ -769,6 +803,32 @@ QDateTime QV8Engine::qtDateTimeFromJsDate(double jsDate) return convertedUTC.toLocalTime(); } +void QV8Engine::addRelationshipForGC(QObject *object, v8::Persistent handle) +{ + if (handle.IsEmpty()) + return; + + bool handleShouldBeStrong = false; + v8::Persistent *implicitOwner = findOwnerAndStrength(object, &handleShouldBeStrong); + if (handleShouldBeStrong) { + v8::V8::AddImplicitReferences(m_strongReferencer, &handle, 1); + } else if (!implicitOwner->IsEmpty()) { + v8::V8::AddImplicitReferences(*implicitOwner, &handle, 1); + } +} + +void QV8Engine::addRelationshipForGC(QObject *object, QObject *other) +{ + bool handleShouldBeStrong = false; + v8::Persistent *implicitOwner = findOwnerAndStrength(object, &handleShouldBeStrong); + v8::Persistent handle = QDeclarativeData::get(other, true)->v8object; + if (handleShouldBeStrong) { + v8::V8::AddImplicitReferences(m_strongReferencer, &handle, 1); + } else if (!implicitOwner->IsEmpty()) { + v8::V8::AddImplicitReferences(*implicitOwner, &handle, 1); + } +} + static QThreadStorage perThreadEngineData; bool QV8Engine::hasThreadData() @@ -1456,8 +1516,6 @@ void QV8GCCallback::registerGcPrologueCallback() QV8Engine::ThreadData *td = QV8Engine::threadData(); if (!td->gcPrologueCallbackRegistered) { td->gcPrologueCallbackRegistered = true; - if (!td->referencer) - td->referencer = new QV8GCCallback::Referencer; v8::V8::AddGCPrologueCallback(QV8GCCallback::garbageCollectorPrologueCallback, v8::kGCTypeMarkSweepCompact); } } @@ -1472,89 +1530,6 @@ QV8GCCallback::Node::~Node() node.remove(); } -QV8GCCallback::Referencer::~Referencer() -{ - dispose(); -} - -QV8GCCallback::Referencer::Referencer() -{ - v8::HandleScope handleScope; - context = v8::Context::New(); - qPersistentRegister(context); - v8::Context::Scope contextScope(context); - strongReferencer = qPersistentNew(v8::Object::New()); -} - -void QV8GCCallback::Referencer::addRelationship(QObject *object, QObject *other) -{ - bool handleShouldBeStrong = false; - v8::Persistent *implicitOwner = findOwnerAndStrength(object, &handleShouldBeStrong); - v8::Persistent handle = QDeclarativeData::get(other, true)->v8object; - if (handleShouldBeStrong) { - v8::V8::AddImplicitReferences(strongReferencer, &handle, 1); - } else if (!implicitOwner->IsEmpty()) { - v8::V8::AddImplicitReferences(*implicitOwner, &handle, 1); - } -} - -void QV8GCCallback::Referencer::dispose() -{ - if (!strongReferencer.IsEmpty()) { - Q_ASSERT_X(v8::Isolate::GetCurrent(), "QV8GCCallback::Referencer::~Referencer()", "called after v8::Isolate has exited"); - // automatically release the strongReferencer if it hasn't - // been explicitly released already. - qPersistentDispose(strongReferencer); - } - if (!context.IsEmpty()) - qPersistentDispose(context); -} - -void QV8GCCallback::Referencer::addRelationship(QObject *object, v8::Persistent handle) -{ - if (handle.IsEmpty()) - return; - - bool handleShouldBeStrong = false; - v8::Persistent *implicitOwner = findOwnerAndStrength(object, &handleShouldBeStrong); - if (handleShouldBeStrong) { - v8::V8::AddImplicitReferences(strongReferencer, &handle, 1); - } else if (!implicitOwner->IsEmpty()) { - v8::V8::AddImplicitReferences(*implicitOwner, &handle, 1); - } -} - -v8::Persistent *QV8GCCallback::Referencer::findOwnerAndStrength(QObject *object, bool *shouldBeStrong) -{ - QObject *parent = object->parent(); - if (!parent) { - // if the object has JS ownership, the object's v8object owns the lifetime of the persistent value. - if (QDeclarativeEngine::objectOwnership(object) == QDeclarativeEngine::JavaScriptOwnership) { - *shouldBeStrong = false; - return &(QDeclarativeData::get(object)->v8object); - } - - // no parent, and has CPP ownership - doesn't have an implicit parent. - *shouldBeStrong = true; - return 0; - } - - // if it is owned by CPP, it's root parent may still be owned by JS. - // in that case, the owner of the persistent handle is the root parent's v8object. - while (parent->parent()) - parent = parent->parent(); - - if (QDeclarativeEngine::objectOwnership(parent) == QDeclarativeEngine::JavaScriptOwnership) { - // root parent is owned by JS. It's v8object owns the persistent value in question. - *shouldBeStrong = false; - return &(QDeclarativeData::get(parent)->v8object); - } else { - // root parent has CPP ownership. The persistent value should not be made weak. - *shouldBeStrong = true; - return 0; - } -} - /* Ensure that each persistent handle is strong if it has CPP ownership and has no implicitly JS owned object owner in its parent chain, and @@ -1574,14 +1549,13 @@ void QV8GCCallback::garbageCollectorPrologueCallback(v8::GCType, v8::GCCallbackF return; QV8Engine::ThreadData *td = QV8Engine::threadData(); - Q_ASSERT(td->referencer); QV8GCCallback::Node *currNode = td->gcCallbackNodes.first(); while (currNode) { // The client which adds itself to the list is responsible // for maintaining the correct implicit references in the // specified callback. - currNode->prologueCallback(td->referencer, currNode); + currNode->prologueCallback(currNode); currNode = td->gcCallbackNodes.next(currNode); } } @@ -1593,8 +1567,7 @@ void QV8GCCallback::addGcCallbackNode(QV8GCCallback::Node *node) } QV8Engine::ThreadData::ThreadData() - : referencer(0) - , gcPrologueCallbackRegistered(false) + : gcPrologueCallbackRegistered(false) { if (!v8::Isolate::GetCurrent()) { isolate = v8::Isolate::New(); @@ -1606,8 +1579,6 @@ QV8Engine::ThreadData::ThreadData() QV8Engine::ThreadData::~ThreadData() { - delete referencer; - referencer = 0; if (isolate) { isolate->Exit(); isolate->Dispose(); diff --git a/src/declarative/qml/v8/qv8engine_p.h b/src/declarative/qml/v8/qv8engine_p.h index 4e6d3de..7564a69 100644 --- a/src/declarative/qml/v8/qv8engine_p.h +++ b/src/declarative/qml/v8/qv8engine_p.h @@ -231,23 +231,9 @@ public: static void garbageCollectorPrologueCallback(v8::GCType, v8::GCCallbackFlags); static void registerGcPrologueCallback(); - class Q_AUTOTEST_EXPORT Referencer { - public: - ~Referencer(); - void addRelationship(QObject *object, v8::Persistent handle); - void addRelationship(QObject *object, QObject *other); - void dispose(); - private: - Referencer(); - static v8::Persistent *findOwnerAndStrength(QObject *qobjectOwner, bool *shouldBeStrong); - v8::Persistent strongReferencer; - v8::Persistent context; - friend class QV8GCCallback; - }; - class Q_AUTOTEST_EXPORT Node { public: - typedef void (*PrologueCallback)(Referencer *r, Node *node); + typedef void (*PrologueCallback)(Node *node); Node(PrologueCallback callback); ~Node(); @@ -451,11 +437,13 @@ public: static QDateTime qtDateTimeFromJsDate(double jsDate); + void addRelationshipForGC(QObject *object, v8::Persistent handle); + void addRelationshipForGC(QObject *object, QObject *other); + struct ThreadData { ThreadData(); ~ThreadData(); v8::Isolate* isolate; - QV8GCCallback::Referencer* referencer; bool gcPrologueCallbackRegistered; QIntrusiveList gcCallbackNodes; }; @@ -464,6 +452,8 @@ public: static ThreadData* threadData(); static void ensurePerThreadIsolate(); + v8::Persistent m_strongReferencer; + protected: QJSEngine* q; QDeclarativeEngine *m_engine; @@ -503,6 +493,8 @@ protected: double qtDateTimeToJsDate(const QDateTime &dt); private: + static v8::Persistent *findOwnerAndStrength(QObject *object, bool *shouldBeStrong); + typedef QScriptIntrusiveList ValueList; ValueList m_values; typedef QScriptIntrusiveList ValueIteratorList; diff --git a/tests/auto/declarative/qdeclarativeecmascript/testtypes.h b/tests/auto/declarative/qdeclarativeecmascript/testtypes.h index 2f1d3f9..0a5fc22 100644 --- a/tests/auto/declarative/qdeclarativeecmascript/testtypes.h +++ b/tests/auto/declarative/qdeclarativeecmascript/testtypes.h @@ -1031,6 +1031,7 @@ public: { CircularReferenceObject *retn = new CircularReferenceObject(parent); retn->m_dtorCount = m_dtorCount; + retn->m_engine = m_engine; return retn; } @@ -1039,17 +1040,23 @@ public: m_referenced = other; } - static void callback(QV8GCCallback::Referencer *r, QV8GCCallback::Node *n) + static void callback(QV8GCCallback::Node *n) { CircularReferenceObject *cro = static_cast(n); if (cro->m_referenced) { - r->addRelationship(cro, cro->m_referenced); + cro->m_engine->addRelationshipForGC(cro, cro->m_referenced); } } + void setEngine(QDeclarativeEngine* declarativeEngine) + { + m_engine = QDeclarativeEnginePrivate::get(declarativeEngine)->v8engine(); + } + private: QObject *m_referenced; int *m_dtorCount; + QV8Engine* m_engine; }; Q_DECLARE_METATYPE(CircularReferenceObject*) @@ -1060,7 +1067,7 @@ class CircularReferenceHandle : public QObject, public: CircularReferenceHandle(QObject *parent = 0) - : QObject(parent), QV8GCCallback::Node(gccallback), m_dtorCount(0) + : QObject(parent), QV8GCCallback::Node(gccallback), m_dtorCount(0), m_engine(0) { QV8GCCallback::addGcCallbackNode(this); } @@ -1079,6 +1086,7 @@ public: { CircularReferenceHandle *retn = new CircularReferenceHandle(parent); retn->m_dtorCount = m_dtorCount; + retn->m_engine = m_engine; return retn; } @@ -1095,15 +1103,21 @@ public: crh->m_referenced.Clear(); } - static void gccallback(QV8GCCallback::Referencer *r, QV8GCCallback::Node *n) + static void gccallback(QV8GCCallback::Node *n) { CircularReferenceHandle *crh = static_cast(n); - r->addRelationship(crh, crh->m_referenced); + crh->m_engine->addRelationshipForGC(crh, crh->m_referenced); + } + + void setEngine(QDeclarativeEngine* declarativeEngine) + { + m_engine = QDeclarativeEnginePrivate::get(declarativeEngine)->v8engine(); } private: v8::Persistent m_referenced; int *m_dtorCount; + QV8Engine* m_engine; }; Q_DECLARE_METATYPE(CircularReferenceHandle*) diff --git a/tests/auto/declarative/qdeclarativeecmascript/tst_qdeclarativeecmascript.cpp b/tests/auto/declarative/qdeclarativeecmascript/tst_qdeclarativeecmascript.cpp index f3d810e..9bb763c 100644 --- a/tests/auto/declarative/qdeclarativeecmascript/tst_qdeclarativeecmascript.cpp +++ b/tests/auto/declarative/qdeclarativeecmascript/tst_qdeclarativeecmascript.cpp @@ -4129,6 +4129,7 @@ void tst_qdeclarativeecmascript::handleReferenceManagement() QObject *object = component.create(); QVERIFY(object != 0); CircularReferenceObject *cro = object->findChild("cro"); + cro->setEngine(&hrmEngine); cro->setDtorCount(&dtorCount); QMetaObject::invokeMethod(object, "createReference"); gc(engine); @@ -4147,6 +4148,7 @@ void tst_qdeclarativeecmascript::handleReferenceManagement() QObject *object = component.create(); QVERIFY(object != 0); CircularReferenceObject *cro = object->findChild("cro"); + cro->setEngine(&hrmEngine); cro->setDtorCount(&dtorCount); QMetaObject::invokeMethod(object, "circularReference"); gc(engine); @@ -4166,6 +4168,7 @@ void tst_qdeclarativeecmascript::handleReferenceManagement() QVERIFY(object != 0); CircularReferenceHandle *crh = object->findChild("crh"); QVERIFY(crh != 0); + crh->setEngine(&hrmEngine); crh->setDtorCount(&dtorCount); QMetaObject::invokeMethod(object, "createReference"); CircularReferenceHandle *first = object->property("first").value(); @@ -4193,6 +4196,7 @@ void tst_qdeclarativeecmascript::handleReferenceManagement() QVERIFY(object != 0); CircularReferenceHandle *crh = object->findChild("crh"); QVERIFY(crh != 0); + crh->setEngine(&hrmEngine); crh->setDtorCount(&dtorCount); QMetaObject::invokeMethod(object, "circularReference"); CircularReferenceHandle *first = object->property("first").value(); @@ -4229,6 +4233,8 @@ void tst_qdeclarativeecmascript::handleReferenceManagement() CircularReferenceHandle *crh2 = object2->findChild("crh"); QVERIFY(crh1 != 0); QVERIFY(crh2 != 0); + crh1->setEngine(&hrmEngine1); + crh2->setEngine(&hrmEngine2); crh1->setDtorCount(&dtorCount); crh2->setDtorCount(&dtorCount); QMetaObject::invokeMethod(object1, "createReference"); @@ -4271,6 +4277,8 @@ void tst_qdeclarativeecmascript::handleReferenceManagement() CircularReferenceHandle *crh2 = object2->findChild("crh"); QVERIFY(crh1 != 0); QVERIFY(crh2 != 0); + crh1->setEngine(&hrmEngine1); + crh2->setEngine(&hrmEngine2); crh1->setDtorCount(&dtorCount); crh2->setDtorCount(&dtorCount); QMetaObject::invokeMethod(object1, "createReference"); @@ -4322,6 +4330,8 @@ void tst_qdeclarativeecmascript::handleReferenceManagement() CircularReferenceHandle *crh2 = object2->findChild("crh"); QVERIFY(crh1 != 0); QVERIFY(crh2 != 0); + crh1->setEngine(hrmEngine1); + crh2->setEngine(hrmEngine2); crh1->setDtorCount(&dtorCount); crh2->setDtorCount(&dtorCount); QMetaObject::invokeMethod(object1, "createReference");