rework ScriptEngine lifetimes using 'smart' pointers

Seth Alves committed 9 years ago

Commit d89f2b71 parent 49ab8af18 files · +135 −146Browse files

Showing the changes to libraries/script-engine/src/ScriptEngines.cpp (1 of 18 files). Show all

Unified · Split

libraries/script-engine/src/ScriptEngines.cpp+28−34

     _scriptInitializers.push_back(initializer);
 }
-void ScriptEngines::addScriptEngine(ScriptEngine* engine) {
+void ScriptEngines::addScriptEngine(QSharedPointer<ScriptEngine> engine) {
-    if (_isStopped) {
+    if (!_isStopped) {
-        engine->deleteLater();
-    } else {
         QMutexLocker locker(&_allScriptsMutex);
         _allKnownScriptEngines.insert(engine);
     }
 }
-void ScriptEngines::removeScriptEngine(ScriptEngine* engine) {
+void ScriptEngines::removeScriptEngine(QSharedPointer<ScriptEngine> engine) {
     // If we're not already in the middle of stopping all scripts, then we should remove ourselves
     // from the list of running scripts. We don't do this if we're in the process of stopping all scripts
     // because that method removes scripts from its list as it iterates them
     QMutexLocker locker(&_allScriptsMutex);
     qCDebug(scriptengine) << "Stopping all scripts.... currently known scripts:" << _allKnownScriptEngines.size();
-    QMutableSetIterator<ScriptEngine*> i(_allKnownScriptEngines);
+    QMutableSetIterator<QSharedPointer<ScriptEngine>> i(_allKnownScriptEngines);
     while (i.hasNext()) {
-        ScriptEngine* scriptEngine = i.next();
+        QSharedPointer<ScriptEngine> scriptEngine = i.next();
         QString scriptName = scriptEngine->getFilename();
         // NOTE: typically all script engines are running. But there's at least one known exception to this, the
             qCDebug(scriptengine) << "waiting on script:" << scriptName;
             scriptEngine->waitTillDoneRunning();
             qCDebug(scriptengine) << "done waiting on script:" << scriptName;
-            scriptEngine->deleteLater();
-            // Once the script is stopped, we can remove it from our set
-            i.remove();
         }
+        // Once the script is stopped, we can remove it from our set
+        i.remove();
     }
     qCDebug(scriptengine) << "DONE Stopping all scripts....";
 }
         _isReloading = true;
     }
-    for (QHash<QUrl, ScriptEngine*>::const_iterator it = _scriptEnginesHash.constBegin();
+    for (QHash<QUrl, QSharedPointer<ScriptEngine>>::const_iterator it = _scriptEnginesHash.constBegin();
         it != _scriptEnginesHash.constEnd(); it++) {
-        ScriptEngine *scriptEngine = it.value();
+        QSharedPointer<ScriptEngine> scriptEngine = it.value();
         // skip already stopped scripts
         if (scriptEngine->isFinished() || scriptEngine->isStopping()) {
             continue;
         QReadLocker lock(&_scriptEnginesHashLock);
         if (_scriptEnginesHash.contains(scriptURL)) {
-            ScriptEngine* scriptEngine = _scriptEnginesHash[scriptURL];
+            QSharedPointer<ScriptEngine> scriptEngine = _scriptEnginesHash[scriptURL];
             if (restart) {
                 auto scriptCache = DependencyManager::get<ScriptCache>();
                 scriptCache->deleteScript(scriptURL);
-                connect(scriptEngine, &ScriptEngine::finished, this, [this](QString scriptName, ScriptEngine* engine) {
+                connect(scriptEngine.data(), &ScriptEngine::finished,
+                        this, [this](QString scriptName, QSharedPointer<ScriptEngine> engine) {
                     reloadScript(scriptName);
                 });
             }
     stopAllScripts(true);
 }
-ScriptEngine* ScriptEngines::loadScript(const QUrl& scriptFilename, bool isUserLoaded, bool loadScriptFromEditor,
+QSharedPointer<ScriptEngine> ScriptEngines::loadScript(const QUrl& scriptFilename, bool isUserLoaded, bool loadScriptFromEditor,
                                         bool activateMainWindow, bool reload) {
     if (thread() != QThread::currentThread()) {
-        ScriptEngine* result { nullptr };
+        QSharedPointer<ScriptEngine> result { nullptr };
-        BLOCKING_INVOKE_METHOD(this, "loadScript", Q_RETURN_ARG(ScriptEngine*, result),
+        BLOCKING_INVOKE_METHOD(this, "loadScript", Q_RETURN_ARG(QSharedPointer<ScriptEngine>, result),
             Q_ARG(QUrl, scriptFilename),
             Q_ARG(bool, isUserLoaded),
             Q_ARG(bool, loadScriptFromEditor),
         return scriptEngine;
     }
-    scriptEngine = new ScriptEngine(_context, NO_SCRIPT, "about:" + scriptFilename.fileName());
+    scriptEngine = QSharedPointer<ScriptEngine>(new ScriptEngine(_context, NO_SCRIPT, "about:" + scriptFilename.fileName()));
+    addScriptEngine(scriptEngine);
     scriptEngine->setUserLoaded(isUserLoaded);
-    connect(scriptEngine, &ScriptEngine::doneRunning, this, [scriptEngine] {
-        scriptEngine->deleteLater();
-    }, Qt::QueuedConnection);
     if (scriptFilename.isEmpty() || !scriptUrl.isValid()) {
         launchScriptEngine(scriptEngine);
     } else {
         // connect to the appropriate signals of this script engine
-        connect(scriptEngine, &ScriptEngine::scriptLoaded, this, &ScriptEngines::onScriptEngineLoaded);
+        connect(scriptEngine.data(), &ScriptEngine::scriptLoaded, this, &ScriptEngines::onScriptEngineLoaded);
-        connect(scriptEngine, &ScriptEngine::errorLoadingScript, this, &ScriptEngines::onScriptEngineError);
+        connect(scriptEngine.data(), &ScriptEngine::errorLoadingScript, this, &ScriptEngines::onScriptEngineError);
         // get the script engine object to load the script at the designated script URL
         scriptEngine->loadURL(scriptUrl, reload);
     return scriptEngine;
 }
-ScriptEngine* ScriptEngines::getScriptEngine(const QUrl& rawScriptURL) {
+QSharedPointer<ScriptEngine> ScriptEngines::getScriptEngine(const QUrl& rawScriptURL) {
-    ScriptEngine* result = nullptr;
+    QSharedPointer<ScriptEngine> result;
     {
         QReadLocker lock(&_scriptEnginesHashLock);
         const QUrl scriptURL = normalizeScriptURL(rawScriptURL);
 // FIXME - change to new version of ScriptCache loading notification
 void ScriptEngines::onScriptEngineLoaded(const QString& rawScriptURL) {
     UserActivityLogger::getInstance().loadedScript(rawScriptURL);
-    ScriptEngine* scriptEngine = qobject_cast<ScriptEngine*>(sender());
+    QSharedPointer<BaseScriptEngine> baseScriptEngine = qobject_cast<ScriptEngine*>(sender())->sharedFromThis();
+    QSharedPointer<ScriptEngine> scriptEngine = qSharedPointerCast<ScriptEngine>(baseScriptEngine);
     launchScriptEngine(scriptEngine);
     emit scriptCountChanged();
 }
-void ScriptEngines::launchScriptEngine(ScriptEngine* scriptEngine) {
+void ScriptEngines::launchScriptEngine(QSharedPointer<ScriptEngine> scriptEngine) {
-    connect(scriptEngine, &ScriptEngine::finished, this, &ScriptEngines::onScriptFinished, Qt::DirectConnection);
+    connect(scriptEngine.data(), &ScriptEngine::finished, this, &ScriptEngines::onScriptFinished, Qt::DirectConnection);
-    connect(scriptEngine, &ScriptEngine::loadScript, [&](const QString& scriptName, bool userLoaded) {
+    connect(scriptEngine.data(), &ScriptEngine::loadScript, [&](const QString& scriptName, bool userLoaded) {
         loadScript(scriptName, userLoaded);
     });
-    connect(scriptEngine, &ScriptEngine::reloadScript, [&](const QString& scriptName, bool userLoaded) {
+    connect(scriptEngine.data(), &ScriptEngine::reloadScript, [&](const QString& scriptName, bool userLoaded) {
         loadScript(scriptName, userLoaded, false, false, true);
     });
     }
 }
-void ScriptEngines::onScriptFinished(const QString& rawScriptURL, ScriptEngine* engine) {
+void ScriptEngines::onScriptFinished(const QString& rawScriptURL, QSharedPointer<ScriptEngine> engine) {
     bool removed = false;
     {
         QWriteLocker lock(&_scriptEnginesHashLock);