diff --git a/assignment-client/src/Agent.cpp b/assignment-client/src/Agent.cpp index e50c7180d1..4efc3343d1 100644 --- a/assignment-client/src/Agent.cpp +++ b/assignment-client/src/Agent.cpp @@ -354,15 +354,16 @@ void Agent::scriptRequestFinished() { void Agent::executeScript() { - _scriptEngine = std::unique_ptr(new ScriptEngine(ScriptEngine::AGENT_SCRIPT, _scriptContents, _payload)); + _scriptEngine = scriptEngineFactory(ScriptEngine::AGENT_SCRIPT, _scriptContents, _payload); _scriptEngine->setParent(this); // be the parent of the script engine so it gets moved when we do - DependencyManager::get()->setScriptEngine(_scriptEngine.get()); + DependencyManager::get()->setScriptEngine(_scriptEngine); // setup an Avatar for the script to use auto scriptedAvatar = DependencyManager::get(); - connect(_scriptEngine.get(), SIGNAL(update(float)), scriptedAvatar.data(), SLOT(update(float)), Qt::ConnectionType::QueuedConnection); + connect(_scriptEngine.data(), SIGNAL(update(float)), + scriptedAvatar.data(), SLOT(update(float)), Qt::ConnectionType::QueuedConnection); scriptedAvatar->setForceFaceTrackerConnected(true); // call model URL setters with empty URLs so our avatar, if user, will have the default models diff --git a/assignment-client/src/Agent.h b/assignment-client/src/Agent.h index b10f72db9a..cf18c0ed18 100644 --- a/assignment-client/src/Agent.h +++ b/assignment-client/src/Agent.h @@ -88,7 +88,7 @@ private: void encodeFrameOfZeros(QByteArray& encodedZeros); void computeLoudness(const QByteArray* decodedBuffer, QSharedPointer); - std::unique_ptr _scriptEngine; + QSharedPointer _scriptEngine; EntityEditPacketSender _entityEditSender; EntityTreeHeadlessViewer _entityViewer; diff --git a/assignment-client/src/scripts/EntityScriptServer.cpp b/assignment-client/src/scripts/EntityScriptServer.cpp index e7433e7c05..fd53ab2391 100644 --- a/assignment-client/src/scripts/EntityScriptServer.cpp +++ b/assignment-client/src/scripts/EntityScriptServer.cpp @@ -415,8 +415,7 @@ void EntityScriptServer::selectAudioFormat(const QString& selectedCodecName) { void EntityScriptServer::resetEntitiesScriptEngine() { auto engineName = QString("about:Entities %1").arg(++_entitiesScriptEngineCount); - auto newEngine = QSharedPointer(new ScriptEngine(ScriptEngine::ENTITY_SERVER_SCRIPT, NO_SCRIPT, engineName), - &ScriptEngine::deleteLater); + auto newEngine = scriptEngineFactory(ScriptEngine::ENTITY_SERVER_SCRIPT, NO_SCRIPT, engineName); auto webSocketServerConstructorValue = newEngine->newFunction(WebSocketServerClass::constructor); newEngine->globalObject().setProperty("WebSocketServer", webSocketServerConstructorValue); @@ -437,11 +436,14 @@ void EntityScriptServer::resetEntitiesScriptEngine() { newEngine->runInThread(); - DependencyManager::get()->setEntitiesScriptEngine(newEngine.data()); + auto newEngineSP = qSharedPointerCast(newEngine); + DependencyManager::get()->setEntitiesScriptEngine(newEngineSP); - disconnect(_entitiesScriptEngine.data(), &ScriptEngine::entityScriptDetailsUpdated, this, &EntityScriptServer::updateEntityPPS); + disconnect(_entitiesScriptEngine.data(), &ScriptEngine::entityScriptDetailsUpdated, + this, &EntityScriptServer::updateEntityPPS); _entitiesScriptEngine.swap(newEngine); - connect(_entitiesScriptEngine.data(), &ScriptEngine::entityScriptDetailsUpdated, this, &EntityScriptServer::updateEntityPPS); + connect(_entitiesScriptEngine.data(), &ScriptEngine::entityScriptDetailsUpdated, + this, &EntityScriptServer::updateEntityPPS); } diff --git a/interface/src/Application.cpp b/interface/src/Application.cpp index 3650c495f2..86ed0a6666 100644 --- a/interface/src/Application.cpp +++ b/interface/src/Application.cpp @@ -963,7 +963,7 @@ Application::Application(int& argc, char** argv, QElapsedTimer& startupTimer, bo DependencyManager::get().data(), &AddressManager::storeCurrentAddress); auto scriptEngines = DependencyManager::get().data(); - scriptEngines->registerScriptInitializer([this](ScriptEngine* engine){ + scriptEngines->registerScriptInitializer([this](QSharedPointer engine){ registerScriptEngineWithApplicationServices(engine); }); @@ -5930,7 +5930,7 @@ int Application::processOctreeStats(ReceivedMessage& message, SharedNodePointer void Application::packetSent(quint64 length) { } -void Application::registerScriptEngineWithApplicationServices(ScriptEngine* scriptEngine) { +void Application::registerScriptEngineWithApplicationServices(QSharedPointer scriptEngine) { scriptEngine->setEmitScriptUpdatesFunction([this]() { SharedNodePointer entityServerNode = DependencyManager::get()->soloNodeOfType(NodeType::EntityServer); @@ -5944,7 +5944,7 @@ void Application::registerScriptEngineWithApplicationServices(ScriptEngine* scri entityScriptingInterface->setEntityTree(getEntities()->getTree()); // give the script engine to the RecordingScriptingInterface for its callbacks - DependencyManager::get()->setScriptEngine(scriptEngine); + DependencyManager::get()->setScriptEngine(QSharedPointer(scriptEngine)); if (property(hifi::properties::TEST).isValid()) { scriptEngine->registerGlobalObject("Test", TestScriptingInterface::getInstance()); @@ -5965,29 +5965,31 @@ void Application::registerScriptEngineWithApplicationServices(ScriptEngine* scri ClipboardScriptingInterface* clipboardScriptable = new ClipboardScriptingInterface(); scriptEngine->registerGlobalObject("Clipboard", clipboardScriptable); - connect(scriptEngine, &ScriptEngine::finished, clipboardScriptable, &ClipboardScriptingInterface::deleteLater); + connect(scriptEngine.data(), &ScriptEngine::finished, clipboardScriptable, &ClipboardScriptingInterface::deleteLater); scriptEngine->registerGlobalObject("Overlays", &_overlays); - qScriptRegisterMetaType(scriptEngine, OverlayPropertyResultToScriptValue, OverlayPropertyResultFromScriptValue); - qScriptRegisterMetaType(scriptEngine, RayToOverlayIntersectionResultToScriptValue, + qScriptRegisterMetaType(scriptEngine.data(), OverlayPropertyResultToScriptValue, OverlayPropertyResultFromScriptValue); + qScriptRegisterMetaType(scriptEngine.data(), RayToOverlayIntersectionResultToScriptValue, RayToOverlayIntersectionResultFromScriptValue); scriptEngine->registerGlobalObject("OffscreenFlags", DependencyManager::get()->getFlags()); scriptEngine->registerGlobalObject("Desktop", DependencyManager::get().data()); - qScriptRegisterMetaType(scriptEngine, wrapperToScriptValue, wrapperFromScriptValue); - qScriptRegisterMetaType(scriptEngine, wrapperToScriptValue, wrapperFromScriptValue); + qScriptRegisterMetaType(scriptEngine.data(), wrapperToScriptValue, wrapperFromScriptValue); + qScriptRegisterMetaType(scriptEngine.data(), + wrapperToScriptValue, wrapperFromScriptValue); scriptEngine->registerGlobalObject("Toolbars", DependencyManager::get().data()); - qScriptRegisterMetaType(scriptEngine, wrapperToScriptValue, wrapperFromScriptValue); - qScriptRegisterMetaType(scriptEngine, wrapperToScriptValue, wrapperFromScriptValue); + qScriptRegisterMetaType(scriptEngine.data(), wrapperToScriptValue, wrapperFromScriptValue); + qScriptRegisterMetaType(scriptEngine.data(), + wrapperToScriptValue, wrapperFromScriptValue); scriptEngine->registerGlobalObject("Tablet", DependencyManager::get().data()); - - DependencyManager::get().data()->setToolbarScriptingInterface(DependencyManager::get().data()); + auto toolbarScriptingInterface = DependencyManager::get().data(); + DependencyManager::get().data()->setToolbarScriptingInterface(toolbarScriptingInterface); scriptEngine->registerGlobalObject("Window", DependencyManager::get().data()); - qScriptRegisterMetaType(scriptEngine, CustomPromptResultToScriptValue, CustomPromptResultFromScriptValue); + qScriptRegisterMetaType(scriptEngine.data(), CustomPromptResultToScriptValue, CustomPromptResultFromScriptValue); scriptEngine->registerGetterSetter("location", LocationScriptingInterface::locationGetter, LocationScriptingInterface::locationSetter, "Window"); // register `location` on the global object. @@ -6019,7 +6021,7 @@ void Application::registerScriptEngineWithApplicationServices(ScriptEngine* scri scriptEngine->registerGlobalObject("DialogsManager", _dialogsManagerScriptingInterface); scriptEngine->registerGlobalObject("GlobalServices", GlobalServicesScriptingInterface::getInstance()); - qScriptRegisterMetaType(scriptEngine, DownloadInfoResultToScriptValue, DownloadInfoResultFromScriptValue); + qScriptRegisterMetaType(scriptEngine.data(), DownloadInfoResultToScriptValue, DownloadInfoResultFromScriptValue); scriptEngine->registerGlobalObject("FaceTracker", DependencyManager::get().data()); @@ -6047,11 +6049,11 @@ void Application::registerScriptEngineWithApplicationServices(ScriptEngine* scri scriptEngine->registerGlobalObject("LimitlessSpeechRecognition", DependencyManager::get().data()); if (auto steamClient = PluginManager::getInstance()->getSteamClientPlugin()) { - scriptEngine->registerGlobalObject("Steam", new SteamScriptingInterface(scriptEngine, steamClient.get())); + scriptEngine->registerGlobalObject("Steam", new SteamScriptingInterface(scriptEngine.data(), steamClient.get())); } auto scriptingInterface = DependencyManager::get(); scriptEngine->registerGlobalObject("Controller", scriptingInterface.data()); - UserInputMapper::registerControllerTypes(scriptEngine); + UserInputMapper::registerControllerTypes(scriptEngine.data()); auto recordingInterface = DependencyManager::get(); scriptEngine->registerGlobalObject("Recording", recordingInterface.data()); @@ -6062,14 +6064,19 @@ void Application::registerScriptEngineWithApplicationServices(ScriptEngine* scri scriptEngine->registerGlobalObject("Selection", DependencyManager::get().data()); scriptEngine->registerGlobalObject("ContextOverlay", DependencyManager::get().data()); - qScriptRegisterMetaType(scriptEngine, OverlayIDtoScriptValue, OverlayIDfromScriptValue); + qScriptRegisterMetaType(scriptEngine.data(), OverlayIDtoScriptValue, OverlayIDfromScriptValue); // connect this script engines printedMessage signal to the global ScriptEngines these various messages - connect(scriptEngine, &ScriptEngine::printedMessage, DependencyManager::get().data(), &ScriptEngines::onPrintedMessage); - connect(scriptEngine, &ScriptEngine::errorMessage, DependencyManager::get().data(), &ScriptEngines::onErrorMessage); - connect(scriptEngine, &ScriptEngine::warningMessage, DependencyManager::get().data(), &ScriptEngines::onWarningMessage); - connect(scriptEngine, &ScriptEngine::infoMessage, DependencyManager::get().data(), &ScriptEngines::onInfoMessage); - connect(scriptEngine, &ScriptEngine::clearDebugWindow, DependencyManager::get().data(), &ScriptEngines::onClearDebugWindow); + connect(scriptEngine.data(), &ScriptEngine::printedMessage, + DependencyManager::get().data(), &ScriptEngines::onPrintedMessage); + connect(scriptEngine.data(), &ScriptEngine::errorMessage, + DependencyManager::get().data(), &ScriptEngines::onErrorMessage); + connect(scriptEngine.data(), &ScriptEngine::warningMessage, + DependencyManager::get().data(), &ScriptEngines::onWarningMessage); + connect(scriptEngine.data(), &ScriptEngine::infoMessage, + DependencyManager::get().data(), &ScriptEngines::onInfoMessage); + connect(scriptEngine.data(), &ScriptEngine::clearDebugWindow, + DependencyManager::get().data(), &ScriptEngines::onClearDebugWindow); } diff --git a/interface/src/Application.h b/interface/src/Application.h index c7f83ad28f..a8783e8e2b 100644 --- a/interface/src/Application.h +++ b/interface/src/Application.h @@ -218,7 +218,7 @@ public: NodeToOctreeSceneStats* getOcteeSceneStats() { return &_octreeServerSceneStats; } virtual controller::ScriptingInterface* getControllerScriptingInterface() { return _controllerScriptingInterface; } - virtual void registerScriptEngineWithApplicationServices(ScriptEngine* scriptEngine) override; + virtual void registerScriptEngineWithApplicationServices(QSharedPointer scriptEngine) override; virtual void copyCurrentViewFrustum(ViewFrustum& viewOut) const override { copyDisplayViewFrustum(viewOut); } virtual QThread* getMainThread() override { return thread(); } diff --git a/interface/src/avatar/MyAvatar.cpp b/interface/src/avatar/MyAvatar.cpp index 8592ff3d74..5b34ab6ea2 100755 --- a/interface/src/avatar/MyAvatar.cpp +++ b/interface/src/avatar/MyAvatar.cpp @@ -262,7 +262,7 @@ void MyAvatar::setDominantHand(const QString& hand) { } } -void MyAvatar::registerMetaTypes(QScriptEngine* engine) { +void MyAvatar::registerMetaTypes(QSharedPointer engine) { QScriptValue value = engine->newQObject(this, QScriptEngine::QtOwnership, QScriptEngine::ExcludeDeleteLater | QScriptEngine::ExcludeChildObjects); engine->globalObject().setProperty("MyAvatar", value); @@ -273,8 +273,8 @@ void MyAvatar::registerMetaTypes(QScriptEngine* engine) { } engine->globalObject().setProperty("DriveKeys", driveKeys); - qScriptRegisterMetaType(engine, audioListenModeToScriptValue, audioListenModeFromScriptValue); - qScriptRegisterMetaType(engine, driveKeysToScriptValue, driveKeysFromScriptValue); + qScriptRegisterMetaType(engine.data(), audioListenModeToScriptValue, audioListenModeFromScriptValue); + qScriptRegisterMetaType(engine.data(), driveKeysToScriptValue, driveKeysFromScriptValue); } void MyAvatar::setOrientationVar(const QVariant& newOrientationVar) { diff --git a/interface/src/avatar/MyAvatar.h b/interface/src/avatar/MyAvatar.h index 65dcc12e7d..c3d74a9609 100644 --- a/interface/src/avatar/MyAvatar.h +++ b/interface/src/avatar/MyAvatar.h @@ -19,6 +19,7 @@ #include #include #include +#include #include #include @@ -169,7 +170,7 @@ public: ~MyAvatar(); void instantiableAvatar() override {}; - void registerMetaTypes(QScriptEngine* engine); + void registerMetaTypes(QSharedPointer engine); virtual void simulateAttachments(float deltaTime) override; diff --git a/interface/src/ui/JSConsole.cpp b/interface/src/ui/JSConsole.cpp index 8c1ff87d13..6b079cbc93 100644 --- a/interface/src/ui/JSConsole.cpp +++ b/interface/src/ui/JSConsole.cpp @@ -111,7 +111,7 @@ void JSConsole::setScriptEngine(const QSharedPointer& scriptEngin // if scriptEngine is NULL then create one and keep track of it using _ownScriptEngine if (scriptEngine.isNull()) { - _scriptEngine = QSharedPointer(DependencyManager::get()->loadScript(_consoleFileName, false), &QObject::deleteLater); + _scriptEngine = DependencyManager::get()->loadScript(_consoleFileName, false); } else { _scriptEngine = scriptEngine; } diff --git a/interface/src/ui/TestingDialog.cpp b/interface/src/ui/TestingDialog.cpp index bba14cd5d7..6f499f2a8d 100644 --- a/interface/src/ui/TestingDialog.cpp +++ b/interface/src/ui/TestingDialog.cpp @@ -24,7 +24,7 @@ TestingDialog::TestingDialog(QWidget* parent) : _console->setFixedHeight(TESTING_CONSOLE_HEIGHT); auto _engines = DependencyManager::get(); - _engine.reset(_engines->loadScript(qApp->applicationDirPath() + testRunnerRelativePath)); + _engine = _engines->loadScript(qApp->applicationDirPath() + testRunnerRelativePath); _console->setScriptEngine(_engine); connect(_engine.data(), &ScriptEngine::finished, this, &TestingDialog::onTestingFinished); } diff --git a/libraries/entities-renderer/src/EntityTreeRenderer.cpp b/libraries/entities-renderer/src/EntityTreeRenderer.cpp index d754d59721..e8ad163964 100644 --- a/libraries/entities-renderer/src/EntityTreeRenderer.cpp +++ b/libraries/entities-renderer/src/EntityTreeRenderer.cpp @@ -70,45 +70,21 @@ EntityRendererPointer EntityTreeRenderer::renderableForEntityId(const EntityItem return itr->second; } -render::ItemID EntityTreeRenderer::renderableIdForEntityId(const EntityItemID& id) const { - auto renderable = renderableForEntityId(id); - return renderable ? renderable->getRenderItemID() : render::Item::INVALID_ITEM_ID; +render::ItemID EntityTreeRenderer::renderableIdForEntityId(const EntityItemID& id) const { + auto renderable = renderableForEntityId(id); + return renderable ? renderable->getRenderItemID() : render::Item::INVALID_ITEM_ID; } int EntityTreeRenderer::_entitiesScriptEngineCount = 0; -void entitiesScriptEngineDeleter(ScriptEngine* engine) { - class WaitRunnable : public QRunnable { - public: - WaitRunnable(ScriptEngine* engine) : _engine(engine) {} - virtual void run() override { - _engine->waitTillDoneRunning(); - _engine->deleteLater(); - } - - private: - ScriptEngine* _engine; - }; - - // Wait for the scripting thread from the thread pool to avoid hanging the main thread - auto threadPool = QThreadPool::globalInstance(); - if (threadPool) { - threadPool->start(new WaitRunnable(engine)); - } else { - delete engine; - } -} - void EntityTreeRenderer::resetEntitiesScriptEngine() { - // Keep a ref to oldEngine until newEngine is ready so EntityScriptingInterface has something to use auto oldEngine = _entitiesScriptEngine; - - auto newEngine = new ScriptEngine(ScriptEngine::ENTITY_CLIENT_SCRIPT, NO_SCRIPT, QString("about:Entities %1").arg(++_entitiesScriptEngineCount)); - _entitiesScriptEngine = QSharedPointer(newEngine, entitiesScriptEngineDeleter); - - _scriptingServices->registerScriptEngineWithApplicationServices(_entitiesScriptEngine.data()); + _entitiesScriptEngine = scriptEngineFactory(ScriptEngine::ENTITY_CLIENT_SCRIPT, NO_SCRIPT, + QString("about:Entities %1").arg(++_entitiesScriptEngineCount)); + _scriptingServices->registerScriptEngineWithApplicationServices(_entitiesScriptEngine); _entitiesScriptEngine->runInThread(); - DependencyManager::get()->setEntitiesScriptEngine(_entitiesScriptEngine.data()); + auto entitiesScriptEngineProvider = qSharedPointerCast(_entitiesScriptEngine); + DependencyManager::get()->setEntitiesScriptEngine(entitiesScriptEngineProvider); } void EntityTreeRenderer::clear() { diff --git a/libraries/entities/src/EntityScriptingInterface.cpp b/libraries/entities/src/EntityScriptingInterface.cpp index 127da47ae2..f91bc14fe4 100644 --- a/libraries/entities/src/EntityScriptingInterface.cpp +++ b/libraries/entities/src/EntityScriptingInterface.cpp @@ -539,7 +539,7 @@ void EntityScriptingInterface::deleteEntity(QUuid id) { } } -void EntityScriptingInterface::setEntitiesScriptEngine(EntitiesScriptEngineProvider* engine) { +void EntityScriptingInterface::setEntitiesScriptEngine(QSharedPointer engine) { std::lock_guard lock(_entitiesScriptEngineLock); _entitiesScriptEngine = engine; } @@ -749,7 +749,7 @@ bool EntityPropertyMetadataRequest::script(EntityItemID entityID, QScriptValue h request->deleteLater(); }); auto entityScriptingInterface = DependencyManager::get(); - entityScriptingInterface->withEntitiesScriptEngine([&](EntitiesScriptEngineProvider* entitiesScriptEngine) { + entityScriptingInterface->withEntitiesScriptEngine([&](QSharedPointer entitiesScriptEngine) { if (entitiesScriptEngine) { request->setFuture(entitiesScriptEngine->getLocalEntityScriptDetails(entityID)); } diff --git a/libraries/entities/src/EntityScriptingInterface.h b/libraries/entities/src/EntityScriptingInterface.h index 1eb754ef1c..9b2b6360f3 100644 --- a/libraries/entities/src/EntityScriptingInterface.h +++ b/libraries/entities/src/EntityScriptingInterface.h @@ -99,7 +99,7 @@ public: void setEntityTree(EntityTreePointer modelTree); EntityTreePointer getEntityTree() { return _entityTree; } - void setEntitiesScriptEngine(EntitiesScriptEngineProvider* engine); + void setEntitiesScriptEngine(QSharedPointer engine); float calculateCost(float mass, float oldVelocity, float newVelocity); void resetActivityTracking(); @@ -405,7 +405,7 @@ signals: void webEventReceived(const EntityItemID& entityItemID, const QVariant& message); protected: - void withEntitiesScriptEngine(std::function function) { + void withEntitiesScriptEngine(std::function)> function) { std::lock_guard lock(_entitiesScriptEngineLock); function(_entitiesScriptEngine); }; @@ -427,7 +427,7 @@ private: EntityTreePointer _entityTree; std::recursive_mutex _entitiesScriptEngineLock; - EntitiesScriptEngineProvider* _entitiesScriptEngine { nullptr }; + QSharedPointer _entitiesScriptEngine; bool _bidOnSimulationOwnership { false }; float _currentAvatarEnergy = { FLT_MAX }; diff --git a/libraries/script-engine/src/AbstractScriptingServicesInterface.h b/libraries/script-engine/src/AbstractScriptingServicesInterface.h index de18338fca..afc3bb1227 100644 --- a/libraries/script-engine/src/AbstractScriptingServicesInterface.h +++ b/libraries/script-engine/src/AbstractScriptingServicesInterface.h @@ -18,7 +18,7 @@ class ScriptEngine; class AbstractScriptingServicesInterface { public: /// Registers application specific services with a script engine. - virtual void registerScriptEngineWithApplicationServices(ScriptEngine* scriptEngine) = 0; + virtual void registerScriptEngineWithApplicationServices(QSharedPointer scriptEngine) = 0; }; diff --git a/libraries/script-engine/src/RecordingScriptingInterface.h b/libraries/script-engine/src/RecordingScriptingInterface.h index c4220958a2..bc0b019251 100644 --- a/libraries/script-engine/src/RecordingScriptingInterface.h +++ b/libraries/script-engine/src/RecordingScriptingInterface.h @@ -14,6 +14,7 @@ #include +#include #include #include #include @@ -28,7 +29,7 @@ class RecordingScriptingInterface : public QObject, public Dependency { public: RecordingScriptingInterface(); - void setScriptEngine(QScriptEngine* scriptEngine) { _scriptEngine = scriptEngine; } + void setScriptEngine(QSharedPointer scriptEngine) { _scriptEngine = scriptEngine; } public slots: void loadRecording(const QString& url, QScriptValue callback = QScriptValue()); @@ -85,7 +86,7 @@ protected: Flag _useSkeletonModel { false }; recording::ClipPointer _lastClip; - QScriptEngine* _scriptEngine; + QSharedPointer _scriptEngine; QSet _clipLoaders; }; diff --git a/libraries/script-engine/src/ScriptEngine.cpp b/libraries/script-engine/src/ScriptEngine.cpp index 8b8eed02fd..ba75ed9856 100644 --- a/libraries/script-engine/src/ScriptEngine.cpp +++ b/libraries/script-engine/src/ScriptEngine.cpp @@ -153,6 +153,15 @@ QString ScriptEngine::logException(const QScriptValue& exception) { return message; } +QSharedPointer scriptEngineFactory(ScriptEngine::Context context, + const QString& scriptContents, + const QString& fileNameString) { + ScriptEngine* engine = new ScriptEngine(context, scriptContents, fileNameString); + QSharedPointer engineSP = QSharedPointer(engine); + DependencyManager::get()->addScriptEngine(qSharedPointerCast(engineSP)); + return engineSP; +} + int ScriptEngine::processLevelMaxRetries { ScriptRequest::MAX_RETRIES }; ScriptEngine::ScriptEngine(Context context, const QString& scriptContents, const QString& fileNameString) : BaseScriptEngine(), @@ -160,10 +169,10 @@ ScriptEngine::ScriptEngine(Context context, const QString& scriptContents, const _scriptContents(scriptContents), _timerFunctionMap(), _fileNameString(fileNameString), - _arrayBufferClass(new ArrayBufferClass(this)) + _arrayBufferClass(new ArrayBufferClass(this)), + // don't delete `ScriptEngines` until all `ScriptEngine`s are gone + _scriptEngines(DependencyManager::get()) { - DependencyManager::get()->addScriptEngine(this); - connect(this, &QScriptEngine::signalHandlerException, this, [this](const QScriptValue& exception) { if (hasUncaughtException()) { // the engine's uncaughtException() seems to produce much better stack traces here @@ -208,22 +217,9 @@ QString ScriptEngine::getContext() const { } ScriptEngine::~ScriptEngine() { - // FIXME: are these scriptInfoMessage/scriptWarningMessage segfaulting anybody else at app shutdown? -#if !defined(Q_OS_LINUX) - scriptInfoMessage("Script Engine shutting down:" + getFilename()); -#else - qCDebug(scriptengine) << "~ScriptEngine()" << this; -#endif - auto scriptEngines = DependencyManager::get(); if (scriptEngines) { - scriptEngines->removeScriptEngine(this); - } else { -#if !defined(Q_OS_LINUX) - scriptWarningMessage("Script destroyed after ScriptEngines!"); -#else - qCWarning(scriptengine) << ("Script destroyed after ScriptEngines!"); -#endif + scriptEngines->removeScriptEngine(qSharedPointerCast(sharedFromThis())); } } @@ -294,7 +290,7 @@ void ScriptEngine::runDebuggable() { stopAllTimers(); // make sure all our timers are stopped if the script is ending emit scriptEnding(); - emit finished(_fileNameString, this); + emit finished(_fileNameString, qSharedPointerCast(sharedFromThis())); _isRunning = false; emit runningStateChanged(); @@ -503,7 +499,8 @@ static void animVarMapFromScriptValue(const QScriptValue& value, AnimVariantMap& parameters.animVariantMapFromScriptValue(value); } // ... while these two are not. But none of the four are ever used. -static QScriptValue resultHandlerToScriptValue(QScriptEngine* engine, const AnimVariantResultHandler& resultHandler) { +static QScriptValue resultHandlerToScriptValue(QScriptEngine* engine, + const AnimVariantResultHandler& resultHandler) { qCCritical(scriptengine) << "Attempt to marshall result handler to javascript"; assert(false); return QScriptValue(); @@ -516,7 +513,8 @@ static void resultHandlerFromScriptValue(const QScriptValue& value, AnimVariantR // Templated qScriptRegisterMetaType fails to compile with raw pointers using ScriptableResourceRawPtr = ScriptableResource*; -static QScriptValue scriptableResourceToScriptValue(QScriptEngine* engine, const ScriptableResourceRawPtr& resource) { +static QScriptValue scriptableResourceToScriptValue(QScriptEngine* engine, + const ScriptableResourceRawPtr& resource) { // The first script to encounter this resource will track its memory. // In this way, it will be more likely to GC. // This fails in the case that the resource is used across many scripts, but @@ -539,11 +537,11 @@ static void scriptableResourceFromScriptValue(const QScriptValue& value, Scripta resource = static_cast(value.toQObject()); } -static QScriptValue createScriptableResourcePrototype(QScriptEngine* engine) { +static QScriptValue createScriptableResourcePrototype(QSharedPointer engine) { auto prototype = engine->newObject(); // Expose enum State to JS/QML via properties - QObject* state = new QObject(engine); + QObject* state = new QObject(engine.data()); state->setObjectName("ResourceState"); auto metaEnum = QMetaEnum::fromType(); for (int i = 0; i < metaEnum.keyCount(); ++i) { @@ -693,7 +691,7 @@ void ScriptEngine::init() { qScriptRegisterMetaType(this, resultHandlerToScriptValue, resultHandlerFromScriptValue); // Scriptable cache access - auto resourcePrototype = createScriptableResourcePrototype(this); + auto resourcePrototype = createScriptableResourcePrototype(qSharedPointerCast(sharedFromThis())); globalObject().setProperty("Resource", resourcePrototype); setDefaultPrototype(qMetaTypeId(), resourcePrototype); qScriptRegisterMetaType(this, scriptableResourceToScriptValue, scriptableResourceFromScriptValue); @@ -1178,7 +1176,7 @@ void ScriptEngine::run() { } } - emit finished(_fileNameString, this); + emit finished(_fileNameString, qSharedPointerCast(sharedFromThis())); _isRunning = false; emit runningStateChanged(); diff --git a/libraries/script-engine/src/ScriptEngine.h b/libraries/script-engine/src/ScriptEngine.h index 7c473a305b..e743753da1 100644 --- a/libraries/script-engine/src/ScriptEngine.h +++ b/libraries/script-engine/src/ScriptEngine.h @@ -53,6 +53,8 @@ static const int SCRIPT_FPS = 60; static const int DEFAULT_MAX_ENTITY_PPS = 9000; static const int DEFAULT_ENTITY_PPS_PER_SCRIPT = 900; +class ScriptEngines; + class CallbackData { public: QScriptValue function; @@ -242,7 +244,7 @@ signals: void errorLoadingScript(const QString& scriptFilename); void update(float deltaTime); void scriptEnding(); - void finished(const QString& fileNameString, ScriptEngine* engine); + void finished(const QString& fileNameString, QSharedPointer); void cleanupMenuItem(const QString& menuItemString); void printedMessage(const QString& message, const QString& scriptName); void errorMessage(const QString& message, const QString& scriptName); @@ -328,6 +330,12 @@ protected: static const QString _SETTINGS_ENABLE_EXTENDED_EXCEPTIONS; Setting::Handle _enableExtendedJSExceptions { _SETTINGS_ENABLE_EXTENDED_EXCEPTIONS, true }; + + QSharedPointer _scriptEngines; }; +QSharedPointer scriptEngineFactory(ScriptEngine::Context context, + const QString& scriptContents, + const QString& fileNameString); + #endif // hifi_ScriptEngine_h diff --git a/libraries/script-engine/src/ScriptEngines.cpp b/libraries/script-engine/src/ScriptEngines.cpp index 57b04eeb82..686d8cae03 100644 --- a/libraries/script-engine/src/ScriptEngines.cpp +++ b/libraries/script-engine/src/ScriptEngines.cpp @@ -137,16 +137,14 @@ void ScriptEngines::registerScriptInitializer(ScriptInitializer initializer) { _scriptInitializers.push_back(initializer); } -void ScriptEngines::addScriptEngine(ScriptEngine* engine) { - if (_isStopped) { - engine->deleteLater(); - } else { +void ScriptEngines::addScriptEngine(QSharedPointer engine) { + if (!_isStopped) { QMutexLocker locker(&_allScriptsMutex); _allKnownScriptEngines.insert(engine); } } -void ScriptEngines::removeScriptEngine(ScriptEngine* engine) { +void ScriptEngines::removeScriptEngine(QSharedPointer 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 @@ -161,9 +159,9 @@ void ScriptEngines::shutdownScripting() { QMutexLocker locker(&_allScriptsMutex); qCDebug(scriptengine) << "Stopping all scripts.... currently known scripts:" << _allKnownScriptEngines.size(); - QMutableSetIterator i(_allKnownScriptEngines); + QMutableSetIterator> i(_allKnownScriptEngines); while (i.hasNext()) { - ScriptEngine* scriptEngine = i.next(); + QSharedPointer 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 @@ -187,12 +185,9 @@ void ScriptEngines::shutdownScripting() { 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...."; } @@ -369,9 +364,9 @@ void ScriptEngines::stopAllScripts(bool restart) { _isReloading = true; } - for (QHash::const_iterator it = _scriptEnginesHash.constBegin(); + for (QHash>::const_iterator it = _scriptEnginesHash.constBegin(); it != _scriptEnginesHash.constEnd(); it++) { - ScriptEngine *scriptEngine = it.value(); + QSharedPointer scriptEngine = it.value(); // skip already stopped scripts if (scriptEngine->isFinished() || scriptEngine->isStopping()) { continue; @@ -417,11 +412,12 @@ bool ScriptEngines::stopScript(const QString& rawScriptURL, bool restart) { QReadLocker lock(&_scriptEnginesHashLock); if (_scriptEnginesHash.contains(scriptURL)) { - ScriptEngine* scriptEngine = _scriptEnginesHash[scriptURL]; + QSharedPointer scriptEngine = _scriptEnginesHash[scriptURL]; if (restart) { auto scriptCache = DependencyManager::get(); scriptCache->deleteScript(scriptURL); - connect(scriptEngine, &ScriptEngine::finished, this, [this](QString scriptName, ScriptEngine* engine) { + connect(scriptEngine.data(), &ScriptEngine::finished, + this, [this](QString scriptName, QSharedPointer engine) { reloadScript(scriptName); }); } @@ -449,11 +445,11 @@ void ScriptEngines::reloadAllScripts() { stopAllScripts(true); } -ScriptEngine* ScriptEngines::loadScript(const QUrl& scriptFilename, bool isUserLoaded, bool loadScriptFromEditor, +QSharedPointer ScriptEngines::loadScript(const QUrl& scriptFilename, bool isUserLoaded, bool loadScriptFromEditor, bool activateMainWindow, bool reload) { if (thread() != QThread::currentThread()) { - ScriptEngine* result { nullptr }; - BLOCKING_INVOKE_METHOD(this, "loadScript", Q_RETURN_ARG(ScriptEngine*, result), + QSharedPointer result { nullptr }; + BLOCKING_INVOKE_METHOD(this, "loadScript", Q_RETURN_ARG(QSharedPointer, result), Q_ARG(QUrl, scriptFilename), Q_ARG(bool, isUserLoaded), Q_ARG(bool, loadScriptFromEditor), @@ -479,19 +475,16 @@ ScriptEngine* ScriptEngines::loadScript(const QUrl& scriptFilename, bool isUserL return scriptEngine; } - scriptEngine = new ScriptEngine(_context, NO_SCRIPT, "about:" + scriptFilename.fileName()); + scriptEngine = QSharedPointer(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, &ScriptEngine::errorLoadingScript, this, &ScriptEngines::onScriptEngineError); + connect(scriptEngine.data(), &ScriptEngine::scriptLoaded, this, &ScriptEngines::onScriptEngineLoaded); + 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); @@ -500,8 +493,8 @@ ScriptEngine* ScriptEngines::loadScript(const QUrl& scriptFilename, bool isUserL return scriptEngine; } -ScriptEngine* ScriptEngines::getScriptEngine(const QUrl& rawScriptURL) { - ScriptEngine* result = nullptr; +QSharedPointer ScriptEngines::getScriptEngine(const QUrl& rawScriptURL) { + QSharedPointer result; { QReadLocker lock(&_scriptEnginesHashLock); const QUrl scriptURL = normalizeScriptURL(rawScriptURL); @@ -516,7 +509,8 @@ ScriptEngine* ScriptEngines::getScriptEngine(const QUrl& rawScriptURL) { // FIXME - change to new version of ScriptCache loading notification void ScriptEngines::onScriptEngineLoaded(const QString& rawScriptURL) { UserActivityLogger::getInstance().loadedScript(rawScriptURL); - ScriptEngine* scriptEngine = qobject_cast(sender()); + QSharedPointer baseScriptEngine = qobject_cast(sender())->sharedFromThis(); + QSharedPointer scriptEngine = qSharedPointerCast(baseScriptEngine); launchScriptEngine(scriptEngine); @@ -532,12 +526,12 @@ void ScriptEngines::onScriptEngineLoaded(const QString& rawScriptURL) { emit scriptCountChanged(); } -void ScriptEngines::launchScriptEngine(ScriptEngine* scriptEngine) { - connect(scriptEngine, &ScriptEngine::finished, this, &ScriptEngines::onScriptFinished, Qt::DirectConnection); - connect(scriptEngine, &ScriptEngine::loadScript, [&](const QString& scriptName, bool userLoaded) { +void ScriptEngines::launchScriptEngine(QSharedPointer scriptEngine) { + connect(scriptEngine.data(), &ScriptEngine::finished, this, &ScriptEngines::onScriptFinished, Qt::DirectConnection); + 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); }); @@ -558,7 +552,7 @@ void ScriptEngines::launchScriptEngine(ScriptEngine* scriptEngine) { } } -void ScriptEngines::onScriptFinished(const QString& rawScriptURL, ScriptEngine* engine) { +void ScriptEngines::onScriptFinished(const QString& rawScriptURL, QSharedPointer engine) { bool removed = false; { QWriteLocker lock(&_scriptEnginesHashLock); diff --git a/libraries/script-engine/src/ScriptEngines.h b/libraries/script-engine/src/ScriptEngines.h index 70634345eb..396b5cc67f 100644 --- a/libraries/script-engine/src/ScriptEngines.h +++ b/libraries/script-engine/src/ScriptEngines.h @@ -33,7 +33,7 @@ class ScriptEngines : public QObject, public Dependency { Q_PROPERTY(ScriptsModelFilter* scriptsModelFilter READ scriptsModelFilter CONSTANT) public: - using ScriptInitializer = std::function; + using ScriptInitializer = std::function)>; ScriptEngines(ScriptEngine::Context context); void registerScriptInitializer(ScriptInitializer initializer); @@ -45,7 +45,7 @@ public: void loadDefaultScripts(); void setScriptsLocation(const QString& scriptsLocation); QStringList getRunningScripts(); - ScriptEngine* getScriptEngine(const QUrl& scriptHash); + QSharedPointer getScriptEngine(const QUrl& scriptHash); ScriptsModel* scriptsModel() { return &_scriptsModel; }; ScriptsModelFilter* scriptsModelFilter() { return &_scriptsModelFilter; }; @@ -53,7 +53,7 @@ public: QString getDefaultScriptsLocation() const; Q_INVOKABLE void loadOneScript(const QString& scriptFilename); - Q_INVOKABLE ScriptEngine* loadScript(const QUrl& scriptFilename = QString(), + Q_INVOKABLE QSharedPointer loadScript(const QUrl& scriptFilename = QString(), bool isUserLoaded = true, bool loadScriptFromEditor = false, bool activateMainWindow = false, bool reload = false); Q_INVOKABLE bool stopScript(const QString& scriptHash, bool restart = false); @@ -72,6 +72,8 @@ public: void shutdownScripting(); bool isStopped() const { return _isStopped; } + void addScriptEngine(QSharedPointer); + signals: void scriptCountChanged(); void scriptsReloading(); @@ -92,22 +94,21 @@ public slots: void onClearDebugWindow(); protected slots: - void onScriptFinished(const QString& fileNameString, ScriptEngine* engine); + void onScriptFinished(const QString& fileNameString, QSharedPointer engine); protected: friend class ScriptEngine; void reloadScript(const QString& scriptName) { loadScript(scriptName, true, false, false, true); } - void addScriptEngine(ScriptEngine* engine); - void removeScriptEngine(ScriptEngine* engine); + void removeScriptEngine(QSharedPointer); void onScriptEngineLoaded(const QString& scriptFilename); void onScriptEngineError(const QString& scriptFilename); - void launchScriptEngine(ScriptEngine* engine); + void launchScriptEngine(QSharedPointer); ScriptEngine::Context _context; QReadWriteLock _scriptEnginesHashLock; - QHash _scriptEnginesHash; - QSet _allKnownScriptEngines; + QHash> _scriptEnginesHash; + QSet> _allKnownScriptEngines; QMutex _allScriptsMutex; std::list _scriptInitializers; mutable Setting::Handle _scriptsLocationHandle;