Merge pull request #1006 from HifiExperiments/warnings_master

Fix Locker issue in RecordingScriptingInterface
overte-org · Jun 7, 2024 · c857228 · c857228
2 parents a79b714 + e351a92
commit c857228
Show file tree

Hide file tree

Showing 3 changed files with 23 additions and 22 deletions.
diff --git a/libraries/recording/src/recording/RecordingScriptingInterface.cpp b/libraries/recording/src/recording/RecordingScriptingInterface.cpp
@@ -38,34 +38,36 @@ using namespace recording;
 static const QString HFR_EXTENSION = "hfr";
 
 RecordingScriptingInterface::RecordingScriptingInterface() {
-    Locker(_mutex);
+    Locker lock(_mutex);
     _player = DependencyManager::get<Deck>();
     _recorder = DependencyManager::get<Recorder>();
 }
 
 bool RecordingScriptingInterface::isPlaying() const {
-    Locker(_mutex);
+    Locker lock(_mutex);
     return _player->isPlaying();
 }
 
 bool RecordingScriptingInterface::isPaused() const {
-    Locker(_mutex);
+    Locker lock(_mutex);
     return _player->isPaused();
 }
 
 float RecordingScriptingInterface::playerElapsed() const {
-    Locker(_mutex);
+    Locker lock(_mutex);
     return _player->position();
 }
 
 float RecordingScriptingInterface::playerLength() const {
-    Locker(_mutex);
+    Locker lock(_mutex);
     return _player->length();
 }
 
 void RecordingScriptingInterface::playClip(NetworkClipLoaderPointer clipLoader, const QString& url, const ScriptValue& callback) {
-    Locker(_mutex);
-    _player->queueClip(clipLoader->getClip());
+    {
+        Locker lock(_mutex);
+        _player->queueClip(clipLoader->getClip());
+    }
 
     if (callback.isFunction()) {
         auto engine = callback.engine();
@@ -75,8 +77,6 @@ void RecordingScriptingInterface::playClip(NetworkClipLoaderPointer clipLoader,
 }
 
 void RecordingScriptingInterface::loadRecording(const QString& url, const ScriptValue& callback) {
-    Locker(_mutex);
-
     auto clipLoader = DependencyManager::get<recording::ClipCache>()->getClipLoader(url);
 
     if (clipLoader->isLoaded()) {
@@ -85,6 +85,8 @@ void RecordingScriptingInterface::loadRecording(const QString& url, const Script
         return;
     }
 
+    Locker lock(_mutex);
+
     // hold a strong pointer to the loading clip so that it has a chance to load
     _clipLoaders.insert(clipLoader);
 
@@ -134,12 +136,12 @@ void RecordingScriptingInterface::startPlaying() {
         return;
     }
 
-    Locker(_mutex);
+    Locker lock(_mutex);
     _player->play();
 }
 
 void RecordingScriptingInterface::setPlayerVolume(float volume) {
-    Locker(_mutex);
+    Locker lock(_mutex);
     _player->setVolume(std::min(std::max(volume, 0.0f), 1.0f));
 }
 
@@ -153,7 +155,7 @@ void RecordingScriptingInterface::setPlayerTime(float time) {
         return;
     }
 
-    Locker(_mutex);
+    Locker lock(_mutex);
     _player->seek(time);
 }
 
@@ -162,7 +164,7 @@ void RecordingScriptingInterface::setPlayFromCurrentLocation(bool playFromCurren
 }
 
 void RecordingScriptingInterface::setPlayerLoop(bool loop) {
-    Locker(_mutex);
+    Locker lock(_mutex);
     _player->loop(loop);
 }
 
@@ -183,7 +185,7 @@ void RecordingScriptingInterface::setPlayerUseSkeletonModel(bool useSkeletonMode
 }
 
 void RecordingScriptingInterface::pausePlayer() {
-    Locker(_mutex);
+    Locker lock(_mutex);
     _player->pause();
 }
 
@@ -193,7 +195,7 @@ void RecordingScriptingInterface::stopPlaying() {
         return;
     }
 
-    Locker(_mutex);
+    Locker lock(_mutex);
     _player->stop();
 }
 
@@ -211,7 +213,7 @@ void RecordingScriptingInterface::startRecording() {
         return;
     }
 
-    Locker(_mutex);
+    Locker lock(_mutex);
     _recorder->start();
 }
 
@@ -221,7 +223,7 @@ void RecordingScriptingInterface::stopRecording() {
         return;
     }
 
-    Locker(_mutex);
+    Locker lock(_mutex);
     _recorder->stop();
     _lastClip = _recorder->getClip();
     _lastClip->seek(0);
@@ -236,7 +238,7 @@ QString RecordingScriptingInterface::getDefaultRecordingSaveDirectory() {
 }
 
 void RecordingScriptingInterface::saveRecording(const QString& filename) {
-    Locker(_mutex);
+    Locker lock(_mutex);
     if (!_lastClip) {
         qWarning() << "There is no recording to save";
         return;
@@ -251,7 +253,7 @@ bool RecordingScriptingInterface::saveRecordingToAsset(const ScriptValue& getCli
         return false;
     }
 
-    Locker(_mutex);
+    Locker lock(_mutex);
     if (!_lastClip) {
         qWarning() << "There is no recording to save";
         return false;
@@ -293,7 +295,7 @@ void RecordingScriptingInterface::loadLastRecording() {
         return;
     }
 
-    Locker(_mutex);
+    Locker lock(_mutex);
 
     if (!_lastClip) {
         qCDebug(scriptengine) << "There is no recording to load";

diff --git a/libraries/recording/src/recording/RecordingScriptingInterface.h b/libraries/recording/src/recording/RecordingScriptingInterface.h
@@ -357,7 +357,7 @@ public slots:
     using Locker = std::unique_lock<Mutex>;
     using Flag = std::atomic<bool>;
 
-    Mutex _mutex;
+    mutable Mutex _mutex;
 
     QSharedPointer<recording::Deck> _player;
     QSharedPointer<recording::Recorder> _recorder;

diff --git a/libraries/script-engine/src/AssetScriptingInterface.cpp b/libraries/script-engine/src/AssetScriptingInterface.cpp
@@ -244,7 +244,6 @@ Promise AssetScriptingInterface::jsPromiseReady(Promise promise, const ScriptVal
 void AssetScriptingInterface::jsCallback(const ScriptValue& handler,
                                          const ScriptValue& error, const ScriptValue& result) {
     Q_ASSERT(thread() == QThread::currentThread());
-    Q_ASSERT(engine());
     //V8TODO: which kind of script context guard needs to be used here?
     ScriptContextGuard scriptContextGuard(_scriptManager->engine()->currentContext());
     auto errorValue = !error.toBool() ? engine()->nullValue() : error;