locking attempt #2

libraries/entities/src/EntityScriptingInterface.cpp

@@ -1924,9 +1924,7 @@ bool EntityScriptingInterface::restartSound(const QUuid& entityID) {
         auto soundEntity = std::dynamic_pointer_cast<SoundEntityItem>(entity);
         bool isPlaying = soundEntity->getPlaying();
         if (isPlaying) {
-            soundEntity->withWriteLock([&] {
+            soundEntity->restartSound(true);
-                soundEntity->restartSound();
-            });
         }
         return isPlaying;
     }

libraries/entities/src/SoundEntityItem.cpp

@@ -138,59 +138,16 @@ bool SoundEntityItem::shouldCreateSound(const EntityTreePointer& tree) const {
 void SoundEntityItem::update(const quint64& now) {
     const auto tree = getTree();
     if (tree) {
+        std::lock_guard<std::recursive_mutex> lock(_soundLock);
+
         _updateNeeded = false;
 
-        withWriteLock([&] {
+        withReadLock([&] {
             if (shouldCreateSound(tree)) {
                 _sound = DependencyManager::get<SoundCache>()->getSound(_url);
             }
         });
 
-        withReadLock([&] {
-            if (_sound) {
-                if (_sound->isLoaded()) {
-                    updateSound(true);
-                } else {
-                    connect(_sound.data(), &Resource::finished, this, [&] {
-                        withReadLock([&] {
-                            updateSound(true);
-                        });
-                    });
-                }
-            }
-        });
-    }
-}
-
-void SoundEntityItem::locationChanged(bool tellPhysics, bool tellChildren) {
-    EntityItem::locationChanged(tellPhysics, tellChildren);
-    withReadLock([&] {
-        updateSound();
-    });
-}
-
-void SoundEntityItem::dimensionsChanged() {
-    EntityItem::dimensionsChanged();
-    withReadLock([&] {
-        updateSound();
-    });
-}
-
-void SoundEntityItem::setURL(const QString& value) {
-    withWriteLock([&] {
-        if (value != _url) {
-            _url = value;
-
-            const auto tree = getTree();
-            if (!tree) {
-                _updateNeeded = true;
-                return;
-            }
-
-            if (shouldCreateSound(tree)) {
-                _sound = DependencyManager::get<SoundCache>()->getSound(_url);
-            }
-
         if (_sound) {
             if (_sound->isLoaded()) {
                 updateSound(true);
@@ -199,7 +156,50 @@ void SoundEntityItem::setURL(const QString& value) {
             }
         }
     }
+}
+
+void SoundEntityItem::locationChanged(bool tellPhysics, bool tellChildren) {
+    EntityItem::locationChanged(tellPhysics, tellChildren);
+    updateSound();
+}
+
+void SoundEntityItem::dimensionsChanged() {
+    EntityItem::dimensionsChanged();
+    updateSound();
+}
+
+void SoundEntityItem::setURL(const QString& value) {
+    bool changed = false;
+    withWriteLock([&] {
+        if (value != _url) {
+            _url = value;
+            changed = true;
+        }
     });
+
+    if (changed) {
+        const auto tree = getTree();
+        if (!tree) {
+            _updateNeeded = true;
+            return;
+        }
+
+        std::lock_guard<std::recursive_mutex> lock(_soundLock);
+
+        withReadLock([&] {
+            if (shouldCreateSound(tree)) {
+                _sound = DependencyManager::get<SoundCache>()->getSound(_url);
+            }
+        });
+
+        if (_sound) {
+            if (_sound->isLoaded()) {
+                updateSound(true);
+            } else {
+                connect(_sound.data(), &Resource::finished, this, [&] { updateSound(true); });
+            }
+        }
+    }
 }
 
 QString SoundEntityItem::getURL() const {
@@ -209,12 +209,17 @@ QString SoundEntityItem::getURL() const {
 }
 
 void SoundEntityItem::setVolume(float value) {
+    bool changed = false;
     withWriteLock([&] {
         if (value != _volume) {
             _volume = value;
-            updateSound();
+            changed = true;
         }
     });
+
+    if (changed) {
+        updateSound();
+    }
 }
 
 float SoundEntityItem::getVolume() const {
@@ -224,12 +229,17 @@ float SoundEntityItem::getVolume() const {
 }
 
 void SoundEntityItem::setTimeOffset(float value) {
+    bool changed = false;
     withWriteLock([&] {
         if (value != _timeOffset) {
             _timeOffset = value;
-            updateSound(true);
+            changed = true;
         }
     });
+
+    if (changed) {
+        updateSound(true);
+    }
 }
 
 float SoundEntityItem::getTimeOffset() const {
@@ -239,12 +249,17 @@ float SoundEntityItem::getTimeOffset() const {
 }
 
 void SoundEntityItem::setPitch(float value) {
+    bool changed = false;
     withWriteLock([&] {
         if (value != _pitch) {
             _pitch = value;
-            updateSound(true);
+            changed = true;
         }
     });
+
+    if (changed) {
+        updateSound(true);
+    }
 }
 
 float SoundEntityItem::getPitch() const {
@@ -254,12 +269,17 @@ float SoundEntityItem::getPitch() const {
 }
 
 void SoundEntityItem::setPlaying(bool value) {
+    bool changed = false;
     withWriteLock([&] {
         if (value != _playing) {
             _playing = value;
-            updateSound();
+            changed = true;
         }
     });
+
+    if (changed) {
+        updateSound();
+    }
 }
 
 bool SoundEntityItem::getPlaying() const {
@@ -269,12 +289,17 @@ bool SoundEntityItem::getPlaying() const {
 }
 
 void SoundEntityItem::setLoop(bool value) {
+    bool changed = false;
     withWriteLock([&] {
         if (value != _loop) {
             _loop = value;
-            updateSound(true);
+            changed = true;
         }
     });
+
+    if (changed) {
+        updateSound(true);
+    }
 }
 
 bool SoundEntityItem::getLoop() const {
@@ -284,12 +309,17 @@ bool SoundEntityItem::getLoop() const {
 }
 
 void SoundEntityItem::setPositional(bool value) {
+    bool changed = false;
     withWriteLock([&] {
         if (value != _positional) {
             _positional = value;
-            updateSound();
+            changed = true;
         }
     });
+
+    if (changed) {
+        updateSound();
+    }
 }
 
 bool SoundEntityItem::getPositional() const {
@@ -299,19 +329,32 @@ bool SoundEntityItem::getPositional() const {
 }
 
 void SoundEntityItem::setLocalOnly(bool value) {
+    bool changed = false;
     withWriteLock([&] {
         if (value != _localOnly) {
             _localOnly = value;
+            changed = true;
+        }
+    });
 
+    if (changed) {
         const auto tree = getTree();
         if (!tree) {
             _updateNeeded = true;
             return;
         }
 
+        std::lock_guard<std::recursive_mutex> lock(_soundLock);
+
+        bool createdSound = false;
+        withReadLock([&] {
             if (shouldCreateSound(tree)) {
                 _sound = DependencyManager::get<SoundCache>()->getSound(_url);
-            } else {
+                createdSound = true;
+            }
+        });
+
+        if (!createdSound) {
             _sound = nullptr;
 
             if (_injector) {
@@ -328,7 +371,6 @@ void SoundEntityItem::setLocalOnly(bool value) {
             }
         }
     }
-    });
 }
 
 bool SoundEntityItem::getLocalOnly() const {
@@ -337,12 +379,20 @@ bool SoundEntityItem::getLocalOnly() const {
     });
 }
 
-bool SoundEntityItem::restartSound() {
+bool SoundEntityItem::restartSound(bool lock) {
+    if (lock) {
+        _soundLock.lock();
+    }
+
     if (!_sound) {
+        if (lock) {
+            _soundLock.unlock();
+        }
         return false;
     }
 
     AudioInjectorOptions options;
+    withReadLock([&] {
         const glm::quat orientation = getWorldOrientation();
         options.position = getWorldPosition() + orientation * (getScaledDimensions() * (ENTITY_ITEM_DEFAULT_REGISTRATION_POINT - getRegistrationPoint()));
         options.positionSet = _positional;
@@ -352,6 +402,7 @@ bool SoundEntityItem::restartSound() {
         options.localOnly = _localOnly || _sound->isAmbisonic(); // force localOnly when ambisonic
         options.secondOffset = _timeOffset;
         options.pitch = _pitch;
+    });
 
     // stereo option isn't set from script, this comes from sound metadata or filename
     options.stereo = _sound->isStereo();
@@ -363,10 +414,16 @@ bool SoundEntityItem::restartSound() {
         _injector = DependencyManager::get<AudioInjectorManager>()->playSound(_sound, options);
     }
 
+    if (lock) {
+        _soundLock.unlock();
+    }
+
     return true;
 }
 
 void SoundEntityItem::updateSound(bool restart) {
+    std::lock_guard<std::recursive_mutex> lock(_soundLock);
+
     if (!_sound) {
         return;
     }

libraries/entities/src/SoundEntityItem.h

@@ -78,7 +78,7 @@ public:
     void setLocalOnly(bool value);
     bool getLocalOnly() const;
 
-    bool restartSound();
+    bool restartSound(bool lock = false);
 
 protected:
     bool shouldCreateSound(const EntityTreePointer& tree) const;
@@ -93,6 +93,7 @@ protected:
     bool _positional { true };
     bool _localOnly { false };
 
+    std::recursive_mutex _soundLock;
     SharedSoundPointer _sound;
     AudioInjectorPointer _injector;
     bool _updateNeeded { false };