diff options
11 files changed, 119 insertions, 24 deletions
diff --git a/src/declarative/qml/qdeclarativeengine.cpp b/src/declarative/qml/qdeclarativeengine.cpp index 92a7391..91390cd 100644 --- a/src/declarative/qml/qdeclarativeengine.cpp +++ b/src/declarative/qml/qdeclarativeengine.cpp @@ -350,9 +350,9 @@ QDeclarativeEnginePrivate::QDeclarativeEnginePrivate(QDeclarativeEngine *e) : captureProperties(false), rootContext(0), isDebugging(false), outputWarningsToStdErr(true), contextClass(0), sharedContext(0), sharedScope(0), objectClass(0), valueTypeClass(0), globalClass(0), cleanup(0), erroredBindings(0), - inProgressCreations(0), scriptEngine(this), workerScriptEngine(0), componentAttached(0), - inBeginCreate(false), networkAccessManager(0), networkAccessManagerFactory(0), - typeLoader(e), importDatabase(e), uniqueId(1) + inProgressCreations(0), scriptEngine(new QDeclarativeScriptEngine(this)), + workerScriptEngine(0), componentAttached(0), inBeginCreate(false), networkAccessManager(0), + networkAccessManagerFactory(0), typeLoader(e), importDatabase(e), uniqueId(1) { if (!qt_QmlQtModule_registered) { qt_QmlQtModule_registered = true; @@ -365,7 +365,7 @@ QDeclarativeEnginePrivate::QDeclarativeEnginePrivate(QDeclarativeEngine *e) } QDeclarativeUtilModule::defineModule(appType); } - globalClass = new QDeclarativeGlobalScriptClass(&scriptEngine); + globalClass = new QDeclarativeGlobalScriptClass(scriptEngine); } /*! @@ -500,6 +500,12 @@ QDeclarativeEnginePrivate::~QDeclarativeEnginePrivate() c->clear(); } + // Destroy the script engine now, before object class is destroyed. + // This is needed here since destroying the script engine causes the + // JSC heap to be cleaned up, and that needs the object class intact + // to work. + delete scriptEngine; + delete rootContext; rootContext = 0; delete contextClass; @@ -589,7 +595,7 @@ void QDeclarativeEnginePrivate::init() rootContext = new QDeclarativeContext(q,true); QScriptValue applicationObject = objectClass->newQObject(new QDeclarativeApplication(q)); - scriptEngine.globalObject().property(QLatin1String("Qt")).setProperty(QLatin1String("application"), applicationObject); + scriptEngine->globalObject().property(QLatin1String("Qt")).setProperty(QLatin1String("application"), applicationObject); if (QCoreApplication::instance()->thread() == q->thread() && QDeclarativeEngineDebugService::isDebuggingEnabled()) { @@ -2111,11 +2117,11 @@ QScriptValue QDeclarativeEnginePrivate::scriptValueFromVariant(const QVariant &v if (p->object) { return listClass->newList(p->property, p->propertyType); } else { - return scriptEngine.nullValue(); + return scriptEngine->nullValue(); } } else if (val.userType() == qMetaTypeId<QList<QObject *> >()) { const QList<QObject *> &list = *(QList<QObject *>*)val.constData(); - QScriptValue rv = scriptEngine.newArray(list.count()); + QScriptValue rv = scriptEngine->newArray(list.count()); for (int ii = 0; ii < list.count(); ++ii) { QObject *object = list.at(ii); rv.setProperty(ii, objectClass->newQObject(object)); @@ -2130,7 +2136,7 @@ QScriptValue QDeclarativeEnginePrivate::scriptValueFromVariant(const QVariant &v if (objOk) { return objectClass->newQObject(obj); } else { - return scriptEngine.toScriptValue(val); + return scriptEngine->toScriptValue(val); } } @@ -2297,13 +2303,13 @@ bool QDeclarativeEngine::importPlugin(const QString &filePath, const QString &ur void QDeclarativeEngine::setOfflineStoragePath(const QString& dir) { Q_D(QDeclarativeEngine); - d->scriptEngine.offlineStoragePath = dir; + d->scriptEngine->offlineStoragePath = dir; } QString QDeclarativeEngine::offlineStoragePath() const { Q_D(const QDeclarativeEngine); - return d->scriptEngine.offlineStoragePath; + return d->scriptEngine->offlineStoragePath; } static void voidptr_destructor(void *v) diff --git a/src/declarative/qml/qdeclarativeengine_p.h b/src/declarative/qml/qdeclarativeengine_p.h index c324f7a..ee95120 100644 --- a/src/declarative/qml/qdeclarativeengine_p.h +++ b/src/declarative/qml/qdeclarativeengine_p.h @@ -181,7 +181,7 @@ public: QDeclarativeDelayedError *erroredBindings; int inProgressCreations; - QDeclarativeScriptEngine scriptEngine; + QDeclarativeScriptEngine *scriptEngine; QDeclarativeWorkerScriptEngine *getWorkerScriptEngine(); QDeclarativeWorkerScriptEngine *workerScriptEngine; @@ -311,7 +311,7 @@ public: static QScriptValue formatTime(QScriptContext*, QScriptEngine*); static QScriptValue formatDateTime(QScriptContext*, QScriptEngine*); #endif - static QScriptEngine *getScriptEngine(QDeclarativeEngine *e) { return &e->d_func()->scriptEngine; } + static QScriptEngine *getScriptEngine(QDeclarativeEngine *e) { return e->d_func()->scriptEngine; } static QDeclarativeEngine *getEngine(QScriptEngine *e) { return static_cast<QDeclarativeScriptEngine*>(e)->p->q_func(); } static QDeclarativeEnginePrivate *get(QDeclarativeEngine *e) { return e->d_func(); } static QDeclarativeEnginePrivate *get(QDeclarativeContext *c) { return (c && c->engine()) ? QDeclarativeEnginePrivate::get(c->engine()) : 0; } diff --git a/src/declarative/qml/qdeclarativeexpression.cpp b/src/declarative/qml/qdeclarativeexpression.cpp index f457286..e3dba93 100644 --- a/src/declarative/qml/qdeclarativeexpression.cpp +++ b/src/declarative/qml/qdeclarativeexpression.cpp @@ -176,7 +176,7 @@ QScriptValue QDeclarativeExpressionPrivate::evalInObjectScope(QDeclarativeContex int lineNumber, QScriptValue *contextObject) { QDeclarativeEnginePrivate *ep = QDeclarativeEnginePrivate::get(context->engine); - QScriptContext *scriptContext = QScriptDeclarativeClass::pushCleanContext(&ep->scriptEngine); + QScriptContext *scriptContext = QScriptDeclarativeClass::pushCleanContext(ep->scriptEngine); if (contextObject) { *contextObject = ep->contextClass->newContext(context, object); scriptContext->pushScope(*contextObject); @@ -184,8 +184,8 @@ QScriptValue QDeclarativeExpressionPrivate::evalInObjectScope(QDeclarativeContex scriptContext->pushScope(ep->contextClass->newContext(context, object)); } scriptContext->pushScope(ep->globalClass->staticGlobalObject()); - QScriptValue rv = ep->scriptEngine.evaluate(program, fileName, lineNumber); - ep->scriptEngine.popContext(); + QScriptValue rv = ep->scriptEngine->evaluate(program, fileName, lineNumber); + ep->scriptEngine->popContext(); return rv; } @@ -194,7 +194,7 @@ QScriptValue QDeclarativeExpressionPrivate::evalInObjectScope(QDeclarativeContex QScriptValue *contextObject) { QDeclarativeEnginePrivate *ep = QDeclarativeEnginePrivate::get(context->engine); - QScriptContext *scriptContext = QScriptDeclarativeClass::pushCleanContext(&ep->scriptEngine); + QScriptContext *scriptContext = QScriptDeclarativeClass::pushCleanContext(ep->scriptEngine); if (contextObject) { *contextObject = ep->contextClass->newContext(context, object); scriptContext->pushScope(*contextObject); @@ -202,8 +202,8 @@ QScriptValue QDeclarativeExpressionPrivate::evalInObjectScope(QDeclarativeContex scriptContext->pushScope(ep->contextClass->newContext(context, object)); } scriptContext->pushScope(ep->globalClass->staticGlobalObject()); - QScriptValue rv = ep->scriptEngine.evaluate(program); - ep->scriptEngine.popContext(); + QScriptValue rv = ep->scriptEngine->evaluate(program); + ep->scriptEngine->popContext(); return rv; } diff --git a/src/declarative/qml/qdeclarativeobjectscriptclass.cpp b/src/declarative/qml/qdeclarativeobjectscriptclass.cpp index 8f8557f..f71a64d 100644 --- a/src/declarative/qml/qdeclarativeobjectscriptclass.cpp +++ b/src/declarative/qml/qdeclarativeobjectscriptclass.cpp @@ -74,14 +74,28 @@ struct ObjectData : public QScriptDeclarativeClass::Object { } } - virtual ~ObjectData() { + enum Disposal { Immediate, Deferred }; + + inline void disposeObject(Disposal disposal) { if (object && !object->parent()) { QDeclarativeData *ddata = QDeclarativeData::get(object, false); - if (ddata && !ddata->indestructible && 0 == --ddata->objectDataRefCount) - object->deleteLater(); + if (ddata && !ddata->indestructible && 0 == --ddata->objectDataRefCount) { + if (disposal == Immediate) + delete object; + else + object->deleteLater(); + } } } + virtual void disposeNow() { + disposeObject(Immediate); + } + + virtual ~ObjectData() { + disposeObject(Deferred); + } + QDeclarativeGuard<QObject> object; int type; }; diff --git a/src/script/api/qscriptengine.cpp b/src/script/api/qscriptengine.cpp index d821084..cebbe0e 100644 --- a/src/script/api/qscriptengine.cpp +++ b/src/script/api/qscriptengine.cpp @@ -962,7 +962,7 @@ QScriptEnginePrivate::QScriptEnginePrivate() qobjectPrototype(0), qmetaobjectPrototype(0), variantPrototype(0), activeAgent(0), agentLineNumber(-1), registeredScriptValues(0), freeScriptValues(0), freeScriptValuesCount(0), - registeredScriptStrings(0), processEventsInterval(-1), inEval(false) + registeredScriptStrings(0), processEventsInterval(-1), inEval(false), inDestructor(false) { qMetaTypeId<QScriptValue>(); qMetaTypeId<QList<int> >(); @@ -1015,6 +1015,7 @@ QScriptEnginePrivate::QScriptEnginePrivate() QScriptEnginePrivate::~QScriptEnginePrivate() { + inDestructor = true; QScript::APIShim shim(this); //disconnect all loadedScripts and generate all jsc::debugger::scriptUnload events diff --git a/src/script/api/qscriptengine_p.h b/src/script/api/qscriptengine_p.h index 6a023d7..8a7037b 100644 --- a/src/script/api/qscriptengine_p.h +++ b/src/script/api/qscriptengine_p.h @@ -382,6 +382,7 @@ public: int processEventsInterval; QScriptValue abortResult; bool inEval; + bool inDestructor; JSC::UString cachedTranslationUrl; JSC::UString cachedTranslationContext; diff --git a/src/script/bridge/qscriptdeclarativeclass_p.h b/src/script/bridge/qscriptdeclarativeclass_p.h index c8c35bf..c67d23d 100644 --- a/src/script/bridge/qscriptdeclarativeclass_p.h +++ b/src/script/bridge/qscriptdeclarativeclass_p.h @@ -77,7 +77,14 @@ public: typedef void* Identifier; - struct Object { virtual ~Object() {} }; + struct Object { + // Like the destructor, frees resources associated with this object. The difference is that + // the destructor might delay resource freeing with deleteLater(), while disposeNow() always + // deletes the resource straigt away. + virtual void disposeNow() {} + + virtual ~Object() {} + }; static QScriptValue newObject(QScriptEngine *, QScriptDeclarativeClass *, Object *); static Value newObjectValue(QScriptEngine *, QScriptDeclarativeClass *, Object *); diff --git a/src/script/bridge/qscriptdeclarativeobject.cpp b/src/script/bridge/qscriptdeclarativeobject.cpp index 927309a..a9cd87d 100644 --- a/src/script/bridge/qscriptdeclarativeobject.cpp +++ b/src/script/bridge/qscriptdeclarativeobject.cpp @@ -53,6 +53,11 @@ DeclarativeObjectDelegate::DeclarativeObjectDelegate(QScriptDeclarativeClass *c, DeclarativeObjectDelegate::~DeclarativeObjectDelegate() { + // When the engine is being destructed, delete the object now, instead of using deleteLater(), + // to not have memory leaks on exit. + if (m_class->engine() && QScriptEnginePrivate::get(m_class->engine())->inDestructor) + m_object->disposeNow(); + delete m_object; } diff --git a/tests/auto/declarative/qdeclarativeecmascript/data/jsOwnedObjectsDeletedOnEngineDestroy.qml b/tests/auto/declarative/qdeclarativeecmascript/data/jsOwnedObjectsDeletedOnEngineDestroy.qml new file mode 100644 index 0000000..da1b682 --- /dev/null +++ b/tests/auto/declarative/qdeclarativeecmascript/data/jsOwnedObjectsDeletedOnEngineDestroy.qml @@ -0,0 +1,6 @@ +import QtQuick 1.0 + +Item { + property variant jsOwnedObject1: deleteObject.object1() + property variant jsOwnedObject2: deleteObject.object2 +} diff --git a/tests/auto/declarative/qdeclarativeecmascript/testtypes.h b/tests/auto/declarative/qdeclarativeecmascript/testtypes.h index 19505e1..3d0d37f 100644 --- a/tests/auto/declarative/qdeclarativeecmascript/testtypes.h +++ b/tests/auto/declarative/qdeclarativeecmascript/testtypes.h @@ -903,6 +903,27 @@ protected: qreal m_p4; }; +class MyDeleteObject : public QObject +{ + Q_OBJECT + Q_PROPERTY(QObject *object2 READ object2 NOTIFY object2Changed) + +public: + MyDeleteObject() : m_object1(0), m_object2(0) {} + + Q_INVOKABLE QObject *object1() const { return m_object1; } + Q_INVOKABLE QObject *object2() const { return m_object2; } + void setObject1(QObject *object) { m_object1 = object; } + void setObject2(QObject *object) { m_object2 = object; emit object2Changed(); } + +signals: + void object2Changed(); + +private: + QObject *m_object1; + QObject *m_object2; +}; + QML_DECLARE_TYPE(MyRevisionedBaseClassRegistered) QML_DECLARE_TYPE(MyRevisionedBaseClassUnregistered) QML_DECLARE_TYPE(MyRevisionedClass) diff --git a/tests/auto/declarative/qdeclarativeecmascript/tst_qdeclarativeecmascript.cpp b/tests/auto/declarative/qdeclarativeecmascript/tst_qdeclarativeecmascript.cpp index 1c54494..188cf1b 100644 --- a/tests/auto/declarative/qdeclarativeecmascript/tst_qdeclarativeecmascript.cpp +++ b/tests/auto/declarative/qdeclarativeecmascript/tst_qdeclarativeecmascript.cpp @@ -48,6 +48,7 @@ #include <QtDeclarative/private/qdeclarativeguard_p.h> #include <QtCore/qdir.h> #include <QtCore/qnumeric.h> +#include <QtTest/qsignalspy.h> #include <private/qdeclarativeengine_p.h> #include <private/qdeclarativeglobalscriptclass_p.h> #include <private/qscriptdeclarativeclass_p.h> @@ -178,6 +179,7 @@ private slots: void pushCleanContext(); void realToInt(); void qtbug_20648(); + void jsOwnedObjectsDeletedOnEngineDestroy(); void include(); @@ -1388,7 +1390,7 @@ void tst_qdeclarativeecmascript::callQtInvokables() QDeclarativeEngine qmlengine; QDeclarativeEnginePrivate *ep = QDeclarativeEnginePrivate::get(&qmlengine); - QScriptEngine *engine = &ep->scriptEngine; + QScriptEngine *engine = ep->scriptEngine; QStringList names; QList<QScriptValue> values; names << QLatin1String("object"); values << ep->objectClass->newQObject(&o); @@ -3100,6 +3102,38 @@ void tst_qdeclarativeecmascript::qtbug_20648() delete o; } +void tst_qdeclarativeecmascript::jsOwnedObjectsDeletedOnEngineDestroy() +{ + QDeclarativeEngine *myEngine = new QDeclarativeEngine; + + MyDeleteObject deleteObject; + deleteObject.setObjectName("deleteObject"); + QObject * const object1 = new QObject; + QObject * const object2 = new QObject; + object1->setObjectName("object1"); + object2->setObjectName("object2"); + deleteObject.setObject1(object1); + deleteObject.setObject2(object2); + + // Objects returned by function calls get marked as destructible, but objects returned by + // property getters do not - therefore we explicitly set the object as destructible. + QDeclarativeEngine::setObjectOwnership(object2, QDeclarativeEngine::JavaScriptOwnership); + + myEngine->rootContext()->setContextProperty("deleteObject", &deleteObject); + QDeclarativeComponent component(myEngine, TEST_FILE("jsOwnedObjectsDeletedOnEngineDestroy.qml")); + QObject *object = component.create(); + QVERIFY(object); + + // Destroying the engine should delete all JS owned QObjects + QSignalSpy spy1(object1, SIGNAL(destroyed())); + QSignalSpy spy2(object2, SIGNAL(destroyed())); + delete myEngine; + QCOMPARE(spy1.count(), 1); + QCOMPARE(spy2.count(), 1); + + delete object; +} + QTEST_MAIN(tst_qdeclarativeecmascript) #include "tst_qdeclarativeecmascript.moc" |