Delete JS-owned QML objects right away in the engine dtor.

This prevents memory leaks when the engine is destroyed after exec()
has already finished. In most cases this happens during application
shutdown, at which point the event loop is never entered again.

Task-number: QTBUG-20377
Change-Id: I65564ed3e56314d656d92fd66f11ae67d4eb932b
Reviewed-by: Jędrzej Nowacki <jedrzej.nowacki@nokia.com>
Reviewed-by: Lars Knoll <lars.knoll@nokia.com>

4 files changed, 41 insertions, 21 deletions

diff --git a/src/declarative/qml/qdeclarativeengine.cpp b/src/declarative/qml/qdeclarativeengine.cpp
index 92a739115a..91390cda76 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 c324f7ab65..ee951206d3 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 f45728673f..e3dba93620 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 8f8557fff3..f71a64d578 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;
 };