Fixed crashes that happened when clearing cache

interface/src/Application.cpp

@@ -7453,7 +7453,7 @@ void Application::addingEntityWithCertificate(const QString& certificateID, cons
     ledger->updateLocation(certificateID, placeName);
 }
 
-void Application::registerScriptEngineWithApplicationServices(const ScriptManagerPointer& scriptManager) {
+void Application::registerScriptEngineWithApplicationServices(ScriptManagerPointer& scriptManager) {
 
     auto scriptEngine = scriptManager->engine();
     scriptManager->setEmitScriptUpdatesFunction([this]() {
@@ -7587,6 +7587,12 @@ void Application::registerScriptEngineWithApplicationServices(const ScriptManage
     }
     auto scriptingInterface = DependencyManager::get<controller::ScriptingInterface>();
     scriptEngine->registerGlobalObject("Controller", scriptingInterface.data());
+    scriptManager->connect(scriptManager.get(), &ScriptManager::scriptEnding, [scriptManager]() {
+        // Request removal of controller routes with callbacks to a given script engine
+        auto userInputMapper = DependencyManager::get<UserInputMapper>();
+        userInputMapper->scheduleScriptEndpointCleanup(scriptManager->engine().get());
+        // V8TODO: Maybe we should wait until removal is finished if there are still crashes
+    });
     UserInputMapper::registerControllerTypes(scriptEngine.get());
 
     auto recordingInterface = DependencyManager::get<RecordingScriptingInterface>();

interface/src/Application.h

@@ -254,7 +254,7 @@ public:
     NodeToOctreeSceneStats* getOcteeSceneStats() { return &_octreeServerSceneStats; }
 
     virtual controller::ScriptingInterface* getControllerScriptingInterface() { return _controllerScriptingInterface; }
-    virtual void registerScriptEngineWithApplicationServices(const ScriptManagerPointer& scriptManager) override;
+    virtual void registerScriptEngineWithApplicationServices(ScriptManagerPointer& scriptManager) override;
 
     virtual void copyCurrentViewFrustum(ViewFrustum& viewOut) const override { copyDisplayViewFrustum(viewOut); }
     virtual QThread* getMainThread() override { return thread(); }

libraries/controllers/src/controllers/ScriptingInterface.cpp

@@ -45,6 +45,9 @@ STATIC_SCRIPT_TYPES_INITIALIZER((+[](ScriptManager* manager) {
     auto scriptEngine = manager->engine().get();
 
     scriptRegisterMetaType<controller::InputController*, inputControllerToScriptValue, inputControllerFromScriptValue>(scriptEngine);
+    manager->connect(manager, &ScriptManager::scriptEnding, [manager]() {
+        ;
+    });
 }));
 
 static QRegularExpression SANITIZE_NAME_EXPRESSION{ "[\\(\\)\\.\\s]" };

libraries/controllers/src/controllers/UserInputMapper.cpp

@@ -270,6 +270,9 @@ void UserInputMapper::update(float deltaTime) {
         channel = Pose();
     }
 
+    // Remove callbacks to script engines that are being destroyed
+    runScriptEndpointCleanup();
+
     // Run the mappings code
     runMappings();
 
@@ -855,6 +858,12 @@ void UserInputMapper::unloadMapping(const QString& jsonFile) {
     }
 }
 
+void UserInputMapper::scheduleScriptEndpointCleanup(ScriptEngine* engine) {
+    _lock.lock();
+    scriptEnginesRequestingCleanup.enqueue(engine);
+    _lock.unlock();
+}
+
 static const QString JSON_NAME = QStringLiteral("name");
 static const QString JSON_CHANNELS = QStringLiteral("channels");
 static const QString JSON_CHANNEL_FROM = QStringLiteral("from");
@@ -1249,6 +1258,50 @@ void UserInputMapper::disableMapping(const Mapping::Pointer& mapping) {
     }
 }
 
+void UserInputMapper::runScriptEndpointCleanup() {
+    _lock.lock();
+    QList<RoutePointer> routesToRemove;
+    while (!scriptEnginesRequestingCleanup.empty()){
+        auto engine = scriptEnginesRequestingCleanup.dequeue();
+        QList<RouteList*> routeLists = {&_deviceRoutes, &_standardRoutes};
+        auto iterator = _mappingsByName.begin();
+        while (iterator != _mappingsByName.end()) {
+            if (iterator->second) {
+                routeLists.append(&iterator->second->routes);
+            }
+            iterator++;
+        }
+        for (auto routeList: routeLists) {
+            for (auto route: *routeList) {
+                auto source = std::dynamic_pointer_cast<ScriptEndpoint>(route->source);
+                if (source && source->getEngine() == engine) {
+                    qDebug() << "UserInputMapper::runScriptEndpointCleanup source";
+                    routesToRemove.append(route);
+                }
+                auto destination = std::dynamic_pointer_cast<ScriptEndpoint>(route->destination);
+                if (destination && destination->getEngine() == engine) {
+                    qDebug() << "UserInputMapper::runScriptEndpointCleanup destination";
+                    routesToRemove.append(route);
+                }
+            }
+        }
+    }
+    while (!routesToRemove.empty()) {
+        qDebug() << "UserInputMapper::runScriptEndpointCleanup routesToRemove";
+        auto route = routesToRemove.first();
+        _deviceRoutes.remove(route);
+        _standardRoutes.remove(route);
+        auto iterator = _mappingsByName.begin();
+        while (iterator != _mappingsByName.end()) {
+            iterator->second->routes.remove(route);
+            iterator++;
+        }
+
+        routesToRemove.removeAll(route);
+    }
+    _lock.unlock();
+}
+
 void UserInputMapper::setActionState(Action action, float value, bool valid) {
     Locker locker(_lock);
     _actionStates[toInt(action)] = value;

libraries/controllers/src/controllers/UserInputMapper.h

@@ -19,6 +19,7 @@
 #include <memory>
 #include <mutex>
 
+#include <QQueue>
 #include <QtQml/QJSValue>
 
 #include <DependencyManager.h>
@@ -126,6 +127,17 @@ namespace controller {
         void unloadMappings(const QStringList& jsonFiles);
         void unloadMapping(const QString& jsonFile);
 
+        /**
+         * @brief Request cleaning up endpoints on script engine shutdown
+         *
+         * Script endpoints need to be removed before script engine they belong to gets deleted, because otherwise
+         * script callback will cause a crash. Script engine invokes this function during shutdown and then waits
+         * for confirmation before being shut down.
+         *
+         * @param engine Pointer to the script engine that will be shut down
+         */
+        void scheduleScriptEndpointCleanup(ScriptEngine* engine);
+
         AxisValue getValue(const Input& input) const;
         Pose getPose(const Input& input) const;
 
@@ -167,6 +179,16 @@ namespace controller {
         static bool applyRoute(const RoutePointer& route, bool force = false);
         void enableMapping(const MappingPointer& mapping);
         void disableMapping(const MappingPointer& mapping);
+
+        /**
+         * @brief Clean up endpoints on script engine shutdown
+         *
+         * Script endpoints need to be removed before script engine they belong to gets deleted, because otherwise
+         * script callback will cause a crash. This function is called from UserInputMapper::runMappings.
+         *
+         */
+        void runScriptEndpointCleanup();
+
         EndpointPointer endpointFor(const QJSValue& endpoint);
         EndpointPointer endpointFor(const ScriptValue& endpoint);
         EndpointPointer compositeEndpointFor(EndpointPointer first, EndpointPointer second);
@@ -200,6 +222,9 @@ namespace controller {
 
         InputCalibrationData inputCalibrationData;
 
+        // Contains pointers to script engines that are requesting callback cleanup during their shutdown process
+        QQueue<ScriptEngine*> scriptEnginesRequestingCleanup;
+
         mutable std::recursive_mutex _lock;
     };
 

libraries/controllers/src/controllers/impl/endpoints/ScriptEndpoint.h

@@ -33,6 +33,7 @@ public:
     virtual void apply(const Pose& newValue, const Pointer& source) override;
 
     virtual bool isPose() const override { return _returnPose; }
+    virtual const ScriptEngine* getEngine() const { return _callable.engine().get(); }
 
 protected:
     Q_INVOKABLE void updateValue();

libraries/script-engine/src/AbstractScriptingServicesInterface.h

@@ -26,7 +26,7 @@ using ScriptManagerPointer = std::shared_ptr<ScriptManager>;
 class AbstractScriptingServicesInterface {
 public:
     /// Registers application specific services with a script engine.
-    virtual void registerScriptEngineWithApplicationServices(const ScriptManagerPointer& scriptEngine) = 0;
+    virtual void registerScriptEngineWithApplicationServices(ScriptManagerPointer& scriptEngine) = 0;
 };
 
 

libraries/shared/src/shared/QtHelpers.cpp

@@ -83,6 +83,8 @@ bool blockingInvokeMethod(
     }
 
     PROFILE_RANGE(app, function);
+    // V8TODO: this causes a deadlock when main thread calls blocking invoke method on entity script thread,
+    // for example when clearing cache. Some sort of mutex is needed to prevent this.
     return QMetaObject::invokeMethod(obj, member,
             Qt::BlockingQueuedConnection, ret, val0, val1, val2, val3, val4, val5, val6, val7, val8, val9);
 }