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 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;
 };