From 444d52070af733bb1c4ecce091407259df0ed357 Mon Sep 17 00:00:00 2001 From: HifiExperiments Date: Thu, 6 Jun 2024 11:31:50 -0700 Subject: [PATCH 1/2] fix locker issue --- .../recording/RecordingScriptingInterface.cpp | 36 +++++++++---------- .../recording/RecordingScriptingInterface.h | 2 +- 2 files changed, 19 insertions(+), 19 deletions(-) diff --git a/libraries/recording/src/recording/RecordingScriptingInterface.cpp b/libraries/recording/src/recording/RecordingScriptingInterface.cpp index c6c4d5cd24..43fe5c3688 100644 --- a/libraries/recording/src/recording/RecordingScriptingInterface.cpp +++ b/libraries/recording/src/recording/RecordingScriptingInterface.cpp @@ -38,33 +38,33 @@ using namespace recording; static const QString HFR_EXTENSION = "hfr"; RecordingScriptingInterface::RecordingScriptingInterface() { - Locker(_mutex); + Locker lock(_mutex); _player = DependencyManager::get(); _recorder = DependencyManager::get(); } 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); + Locker lock(_mutex); _player->queueClip(clipLoader->getClip()); if (callback.isFunction()) { @@ -75,7 +75,7 @@ void RecordingScriptingInterface::playClip(NetworkClipLoaderPointer clipLoader, } void RecordingScriptingInterface::loadRecording(const QString& url, const ScriptValue& callback) { - Locker(_mutex); + Locker lock(_mutex); auto clipLoader = DependencyManager::get()->getClipLoader(url); @@ -134,12 +134,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 +153,7 @@ void RecordingScriptingInterface::setPlayerTime(float time) { return; } - Locker(_mutex); + Locker lock(_mutex); _player->seek(time); } @@ -162,7 +162,7 @@ void RecordingScriptingInterface::setPlayFromCurrentLocation(bool playFromCurren } void RecordingScriptingInterface::setPlayerLoop(bool loop) { - Locker(_mutex); + Locker lock(_mutex); _player->loop(loop); } @@ -183,7 +183,7 @@ void RecordingScriptingInterface::setPlayerUseSkeletonModel(bool useSkeletonMode } void RecordingScriptingInterface::pausePlayer() { - Locker(_mutex); + Locker lock(_mutex); _player->pause(); } @@ -193,7 +193,7 @@ void RecordingScriptingInterface::stopPlaying() { return; } - Locker(_mutex); + Locker lock(_mutex); _player->stop(); } @@ -211,7 +211,7 @@ void RecordingScriptingInterface::startRecording() { return; } - Locker(_mutex); + Locker lock(_mutex); _recorder->start(); } @@ -221,7 +221,7 @@ void RecordingScriptingInterface::stopRecording() { return; } - Locker(_mutex); + Locker lock(_mutex); _recorder->stop(); _lastClip = _recorder->getClip(); _lastClip->seek(0); @@ -236,7 +236,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 +251,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 +293,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 index 4cc8a990cd..394c3e230d 100644 --- 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; using Flag = std::atomic; - Mutex _mutex; + mutable Mutex _mutex; QSharedPointer _player; QSharedPointer _recorder; From 5ba1fcecb568e804693aaa7c66115ba173f792a8 Mon Sep 17 00:00:00 2001 From: Karol Suprynowicz Date: Fri, 7 Jun 2024 01:24:34 +0200 Subject: [PATCH 2/2] Fix recording deadlocks --- .../src/recording/RecordingScriptingInterface.cpp | 10 ++++++---- .../script-engine/src/AssetScriptingInterface.cpp | 1 - 2 files changed, 6 insertions(+), 5 deletions(-) diff --git a/libraries/recording/src/recording/RecordingScriptingInterface.cpp b/libraries/recording/src/recording/RecordingScriptingInterface.cpp index 43fe5c3688..a05ee60604 100644 --- a/libraries/recording/src/recording/RecordingScriptingInterface.cpp +++ b/libraries/recording/src/recording/RecordingScriptingInterface.cpp @@ -64,8 +64,10 @@ float RecordingScriptingInterface::playerLength() const { } void RecordingScriptingInterface::playClip(NetworkClipLoaderPointer clipLoader, const QString& url, const ScriptValue& callback) { - Locker lock(_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 lock(_mutex); - auto clipLoader = DependencyManager::get()->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); diff --git a/libraries/script-engine/src/AssetScriptingInterface.cpp b/libraries/script-engine/src/AssetScriptingInterface.cpp index f412d9d2d6..fe11582abf 100644 --- 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;