diff --git a/assignment-client/src/avatars/ScriptableAvatar.cpp b/assignment-client/src/avatars/ScriptableAvatar.cpp index c919d2a2d7..1d0114d703 100644 --- a/assignment-client/src/avatars/ScriptableAvatar.cpp +++ b/assignment-client/src/avatars/ScriptableAvatar.cpp @@ -30,8 +30,26 @@ #include -ScriptableAvatar::ScriptableAvatar(): _scriptEngine(newScriptEngine()) { +ScriptableAvatar::ScriptableAvatar() { _clientTraitsHandler.reset(new ClientTraitsHandler(this)); + std::lock_guard lock(_scriptEngineLock); + _scriptEngine = newScriptEngine(); + if (!_scriptEngineThread) { + _scriptEngineThread.reset(new QThread()); + _scriptEngine->setThread(_scriptEngineThread.get()); + _scriptEngineThread->start(); + } +} + +ScriptableAvatar::~ScriptableAvatar() { + std::lock_guard lock(_scriptEngineLock); + if (_scriptEngine) { + if (_scriptEngineThread) { + _scriptEngineThread->quit(); + _scriptEngineThread->wait(); + } + _scriptEngine.reset(); + } } QByteArray ScriptableAvatar::toByteArrayStateful(AvatarDataDetail dataDetail, bool dropFaceTracking) { @@ -315,7 +333,10 @@ AvatarEntityMap ScriptableAvatar::getAvatarEntityDataInternal(bool allProperties EntityItemProperties properties = entity->getProperties(desiredProperties); QByteArray blob; - EntityItemProperties::propertiesToBlob(*_scriptEngine, sessionID, properties, blob, allProperties); + { + std::lock_guard lock(_scriptEngineLock); + EntityItemProperties::propertiesToBlob(*_scriptEngine, sessionID, properties, blob, allProperties); + } data[id] = blob; } }); @@ -339,8 +360,11 @@ void ScriptableAvatar::setAvatarEntityData(const AvatarEntityMap& avatarEntityDa while (dataItr != avatarEntityData.end()) { EntityItemProperties properties; const QByteArray& blob = dataItr.value(); - if (!blob.isNull() && EntityItemProperties::blobToProperties(*_scriptEngine, blob, properties)) { - newProperties[dataItr.key()] = properties; + { + std::lock_guard lock(_scriptEngineLock); + if (!blob.isNull() && EntityItemProperties::blobToProperties(*_scriptEngine, blob, properties)) { + newProperties[dataItr.key()] = properties; + } } ++dataItr; } @@ -419,9 +443,12 @@ void ScriptableAvatar::updateAvatarEntity(const QUuid& entityID, const QByteArra EntityItemPointer entity; EntityItemProperties properties; - if (!EntityItemProperties::blobToProperties(*_scriptEngine, entityData, properties)) { - // entityData is corrupt - return; + { + std::lock_guard lock(_scriptEngineLock); + if (!EntityItemProperties::blobToProperties(*_scriptEngine, entityData, properties)) { + // entityData is corrupt + return; + } } std::map::iterator itr = _entities.find(entityID); diff --git a/assignment-client/src/avatars/ScriptableAvatar.h b/assignment-client/src/avatars/ScriptableAvatar.h index 703a0a9f64..56f9d2169d 100644 --- a/assignment-client/src/avatars/ScriptableAvatar.h +++ b/assignment-client/src/avatars/ScriptableAvatar.h @@ -111,6 +111,7 @@ class ScriptableAvatar : public AvatarData, public Dependency { public: ScriptableAvatar(); + ~ScriptableAvatar(); /*@jsdoc * Starts playing an animation on the avatar. @@ -222,7 +223,11 @@ private: QHash _fstJointIndices; ///< 1-based, since zero is returned for missing keys QStringList _fstJointNames; ///< in order of depth-first traversal QUrl _skeletonFBXURL; - mutable ScriptEnginePointer _scriptEngine; + + mutable std::mutex _scriptEngineLock; + mutable ScriptEnginePointer _scriptEngine { nullptr }; + std::shared_ptr _scriptEngineThread { nullptr }; + std::map _entities; /// Loads the joint indices, names from the FST file (if any) diff --git a/interface/src/avatar/MyAvatar.cpp b/interface/src/avatar/MyAvatar.cpp index 62100f9cae..de2ba27a77 100644 --- a/interface/src/avatar/MyAvatar.cpp +++ b/interface/src/avatar/MyAvatar.cpp @@ -406,6 +406,14 @@ MyAvatar::~MyAvatar() { if (_addAvatarEntitiesToTreeTimer.isActive()) { _addAvatarEntitiesToTreeTimer.stop(); } + std::lock_guard lock(_scriptEngineLock); + if (_scriptEngine) { + if (_scriptEngineThread) { + _scriptEngineThread->quit(); + _scriptEngineThread->wait(); + } + _scriptEngine.reset(); + } } QString MyAvatar::getDominantHand() const { @@ -2092,8 +2100,16 @@ void MyAvatar::avatarEntityDataToJson(QJsonObject& root) const { } void MyAvatar::loadData() { - if (!_scriptEngine) { - _scriptEngine = newScriptEngine(); + { + std::lock_guard lock(_scriptEngineLock); + if (!_scriptEngine) { + _scriptEngine = newScriptEngine(); + if (!_scriptEngineThread) { + _scriptEngineThread.reset(new QThread()); + _scriptEngine->setThread(_scriptEngineThread.get()); + _scriptEngineThread->start(); + } + } } getHead()->setBasePitch(_headPitchSetting.get()); diff --git a/interface/src/avatar/MyAvatar.h b/interface/src/avatar/MyAvatar.h index 5e0627360c..f3a04284c3 100644 --- a/interface/src/avatar/MyAvatar.h +++ b/interface/src/avatar/MyAvatar.h @@ -3101,8 +3101,12 @@ private: mutable std::set _staleCachedAvatarEntityBlobs; // // keep a ScriptEngine around so we don't have to instantiate on the fly (these are very slow to create/delete) + // TODO: profile if it performs better when script engine is on avatar thread or on its own thread + // Own thread is safer from deadlocks mutable std::mutex _scriptEngineLock; ScriptEnginePointer _scriptEngine { nullptr }; + std::shared_ptr _scriptEngineThread { nullptr }; + bool _needToSaveAvatarEntitySettings { false }; bool _reactionTriggers[NUM_AVATAR_TRIGGER_REACTIONS] { false, false }; diff --git a/libraries/baking/src/MaterialBaker.cpp b/libraries/baking/src/MaterialBaker.cpp index 48718cd6c3..d28fc9aacf 100644 --- a/libraries/baking/src/MaterialBaker.cpp +++ b/libraries/baking/src/MaterialBaker.cpp @@ -36,9 +36,24 @@ MaterialBaker::MaterialBaker(const QString& materialData, bool isURL, const QStr _isURL(isURL), _destinationPath(destinationPath), _bakedOutputDir(bakedOutputDir), - _textureOutputDir(bakedOutputDir + "/materialTextures/" + QString::number(materialNum++)), - _scriptEngine(newScriptEngine()) + _textureOutputDir(bakedOutputDir + "/materialTextures/" + QString::number(materialNum++)) { + std::lock_guard lock(_scriptEngineLock); + _scriptEngine = newScriptEngine(); + _scriptEngineThread.reset(new QThread()); + _scriptEngine->setThread(_scriptEngineThread.get()); + _scriptEngineThread->start(); +} + +MaterialBaker::~MaterialBaker() { + std::lock_guard lock(_scriptEngineLock); + if (_scriptEngine) { + if (_scriptEngineThread) { + _scriptEngineThread->quit(); + _scriptEngineThread->wait(); + } + _scriptEngine.reset(); + } } void MaterialBaker::bake() { @@ -214,6 +229,7 @@ void MaterialBaker::outputMaterial() { if (_materialResource->parsedMaterials.networkMaterials.size() == 1) { auto networkMaterial = _materialResource->parsedMaterials.networkMaterials.begin(); auto scriptableMaterial = scriptable::ScriptableMaterial(networkMaterial->second); + std::lock_guard lock(_scriptEngineLock); QVariant materialVariant = scriptable::scriptableMaterialToScriptValue(_scriptEngine.get(), scriptableMaterial).toVariant(); json.insert("materials", QJsonDocument::fromVariant(materialVariant).object()); @@ -221,6 +237,7 @@ void MaterialBaker::outputMaterial() { QJsonArray materialArray; for (auto networkMaterial : _materialResource->parsedMaterials.networkMaterials) { auto scriptableMaterial = scriptable::ScriptableMaterial(networkMaterial.second); + std::lock_guard lock(_scriptEngineLock); QVariant materialVariant = scriptable::scriptableMaterialToScriptValue(_scriptEngine.get(), scriptableMaterial).toVariant(); materialArray.append(QJsonDocument::fromVariant(materialVariant).object()); diff --git a/libraries/baking/src/MaterialBaker.h b/libraries/baking/src/MaterialBaker.h index 129b36aa8f..986dead3ed 100644 --- a/libraries/baking/src/MaterialBaker.h +++ b/libraries/baking/src/MaterialBaker.h @@ -32,6 +32,7 @@ class MaterialBaker : public Baker { Q_OBJECT public: MaterialBaker(const QString& materialData, bool isURL, const QString& bakedOutputDir, QUrl destinationPath = QUrl()); + virtual ~MaterialBaker(); QString getMaterialData() const { return _materialData; } bool isURL() const { return _isURL; } @@ -72,7 +73,9 @@ private: QString _textureOutputDir; QString _bakedMaterialData; - ScriptEnginePointer _scriptEngine; + mutable std::mutex _scriptEngineLock; + ScriptEnginePointer _scriptEngine { nullptr }; + std::shared_ptr _scriptEngineThread { nullptr }; static std::function _getNextOvenWorkerThreadOperator; TextureFileNamer _textureFileNamer; diff --git a/libraries/entities/src/EntityTree.cpp b/libraries/entities/src/EntityTree.cpp index fb7fbe6499..61535b913e 100644 --- a/libraries/entities/src/EntityTree.cpp +++ b/libraries/entities/src/EntityTree.cpp @@ -50,6 +50,12 @@ static const QString DOMAIN_UNLIMITED = "domainUnlimited"; EntityTree::EntityTree(bool shouldReaverage) : Octree(shouldReaverage) { + std::lock_guard lock(_scriptEngineLock); + _scriptEngine = newScriptEngine(); + _scriptEngineThread.reset(new QThread()); + _scriptEngine->setThread(_scriptEngineThread.get()); + _scriptEngineThread->start(); + resetClientEditStats(); } @@ -63,6 +69,14 @@ EntityTree::~EntityTree() { // TODO: EntityTreeElement::_tree should be raw back pointer. // AND: EntityItem::_element should be a raw back pointer. //eraseAllOctreeElements(false); // KEEP THIS + std::lock_guard lock(_scriptEngineLock); + if (_scriptEngine) { + if (_scriptEngineThread) { + _scriptEngineThread->quit(); + _scriptEngineThread->wait(); + } + _scriptEngine.reset(); + } } void EntityTree::setEntityScriptSourceWhitelist(const QString& entityScriptSourceWhitelist) { @@ -2553,8 +2567,8 @@ bool EntityTree::writeToMap(QVariantMap& entityDescription, OctreeElementPointer } entityDescription["DataVersion"] = _persistDataVersion; entityDescription["Id"] = _persistID; - const std::lock_guard scriptLock(scriptEngineMutex); - RecurseOctreeToMapOperator theOperator(entityDescription, element, scriptEngine.get(), skipDefaultValues, + const std::lock_guard scriptLock(_scriptEngineLock); + RecurseOctreeToMapOperator theOperator(entityDescription, element, _scriptEngine.get(), skipDefaultValues, skipThoseWithBadParents, _myAvatar); withReadLock([&] { recurseTreeWithOperator(&theOperator); @@ -2729,8 +2743,8 @@ bool EntityTree::readFromMap(QVariantMap& map, const bool isImport) { EntityItemProperties properties; { - const std::lock_guard scriptLock(scriptEngineMutex); - ScriptValue entityScriptValue = variantMapToScriptValue(entityMap, *scriptEngine); + const std::lock_guard scriptLock(_scriptEngineLock); + ScriptValue entityScriptValue = variantMapToScriptValue(entityMap, *_scriptEngine); EntityItemPropertiesFromScriptValueIgnoreReadOnly(entityScriptValue, properties); } @@ -2881,8 +2895,8 @@ bool EntityTree::readFromMap(QVariantMap& map, const bool isImport) { } bool EntityTree::writeToJSON(QString& jsonString, const OctreeElementPointer& element) { - const std::lock_guard scriptLock(scriptEngineMutex); - RecurseOctreeToJSONOperator theOperator(element, scriptEngine.get(), jsonString); + const std::lock_guard scriptLock(_scriptEngineLock); + RecurseOctreeToJSONOperator theOperator(element, _scriptEngine.get(), jsonString); withReadLock([&] { recurseTreeWithOperator(&theOperator); }); diff --git a/libraries/entities/src/EntityTree.h b/libraries/entities/src/EntityTree.h index d5c3a74eb7..9d348e87a7 100644 --- a/libraries/entities/src/EntityTree.h +++ b/libraries/entities/src/EntityTree.h @@ -387,8 +387,9 @@ private: MovingEntitiesOperator& moveOperator, bool force, bool tellServer); // Script engine for writing entity tree data to and from JSON - std::mutex scriptEngineMutex; - ScriptEnginePointer scriptEngine{ newScriptEngine() }; + std::mutex _scriptEngineLock; + ScriptEnginePointer _scriptEngine{ nullptr }; + std::shared_ptr _scriptEngineThread { nullptr }; }; void convertGrabUserDataToProperties(EntityItemProperties& properties);