From 49b66c2ba953134e01cf589e991ed9b564aaf8d3 Mon Sep 17 00:00:00 2001 From: Paul Ferrand Date: Fri, 23 Oct 2020 14:44:20 +0200 Subject: [PATCH 1/3] Clear the loading queues before changing the file --- src/sfizz/Synth.cpp | 3 +++ 1 file changed, 3 insertions(+) diff --git a/src/sfizz/Synth.cpp b/src/sfizz/Synth.cpp index 3365f791..4103f388 100644 --- a/src/sfizz/Synth.cpp +++ b/src/sfizz/Synth.cpp @@ -194,6 +194,9 @@ void sfz::Synth::clear() { const std::lock_guard disableCallback { callbackGuard }; + // Clear the background queues before removing everyone + resources.filePool.waitForBackgroundLoading(); + for (auto& voice : voices) voice->reset(); for (auto& list : noteActivationLists) From a372d40c48918a7b5cd3ef725c5e5dc52895dd31 Mon Sep 17 00:00:00 2001 From: Paul Ferrand Date: Fri, 23 Oct 2020 14:46:18 +0200 Subject: [PATCH 2/3] Use a single mutex for both the garbage and last used file data structures They're more or less jointly updated anyway by the garbage collection thread --- src/sfizz/FilePool.cpp | 12 ++++-------- src/sfizz/FilePool.h | 3 +-- 2 files changed, 5 insertions(+), 10 deletions(-) diff --git a/src/sfizz/FilePool.cpp b/src/sfizz/FilePool.cpp index db1a1dfb..3177018b 100644 --- a/src/sfizz/FilePool.cpp +++ b/src/sfizz/FilePool.cpp @@ -428,7 +428,7 @@ void sfz::FilePool::loadingJob(QueuedFileData data) noexcept data.data->status = FileData::Status::Done; - std::lock_guard guard { lastUsedMutex }; + std::lock_guard guard { garbageAndLastUsedMutex }; if (absl::c_find(lastUsedFiles, *id) == lastUsedFiles.end()) lastUsedFiles.push_back(*id); } @@ -514,10 +514,7 @@ void sfz::FilePool::dispatchingJob() noexcept void sfz::FilePool::garbageJob() noexcept { while (semGarbageBarrier.wait(), garbageFlag) { - std::lock_guard guard { garbageMutex }; - for (auto& g: garbageToCollect) - g.reset(); - + std::lock_guard guard { garbageAndLastUsedMutex }; garbageToCollect.clear(); } } @@ -587,9 +584,8 @@ void sfz::FilePool::setRamLoading(bool loadInRam) noexcept void sfz::FilePool::triggerGarbageCollection() noexcept { - const std::unique_lock lastUsedLock { lastUsedMutex, std::try_to_lock }; - const std::unique_lock garbageLock { garbageMutex, std::try_to_lock }; - if (!lastUsedLock.owns_lock() || !garbageLock.owns_lock()) + const std::unique_lock guard { garbageAndLastUsedMutex, std::try_to_lock }; + if (!guard.owns_lock()) return; const auto now = std::chrono::high_resolution_clock::now(); diff --git a/src/sfizz/FilePool.h b/src/sfizz/FilePool.h index 8c14f1ce..41068598 100644 --- a/src/sfizz/FilePool.h +++ b/src/sfizz/FilePool.h @@ -362,9 +362,8 @@ private: std::thread dispatchThread { &FilePool::dispatchingJob, this }; std::thread garbageThread { &FilePool::garbageJob, this }; - SpinMutex lastUsedMutex; + SpinMutex garbageAndLastUsedMutex; std::vector lastUsedFiles; - SpinMutex garbageMutex; std::vector garbageToCollect; std::shared_ptr threadPool; From 1d2c3a42443340f8f24ef81107550bd7c5d6528f Mon Sep 17 00:00:00 2001 From: Paul Ferrand Date: Fri, 23 Oct 2020 14:47:10 +0200 Subject: [PATCH 3/3] Upon clearing the file pool, clear the last used files and garbage queue I also added an early exit with an assertion to catch unintended behavior --- src/sfizz/FilePool.cpp | 13 ++++++++++++- 1 file changed, 12 insertions(+), 1 deletion(-) diff --git a/src/sfizz/FilePool.cpp b/src/sfizz/FilePool.cpp index 3177018b..ff7492d6 100644 --- a/src/sfizz/FilePool.cpp +++ b/src/sfizz/FilePool.cpp @@ -435,7 +435,10 @@ void sfz::FilePool::loadingJob(QueuedFileData data) noexcept void sfz::FilePool::clear() { + std::lock_guard guard { garbageAndLastUsedMutex }; emptyFileLoadingQueues(); + garbageToCollect.clear(); + lastUsedFiles.clear(); preloadedFiles.clear(); } @@ -593,7 +596,15 @@ void sfz::FilePool::triggerGarbageCollection() noexcept if (garbageToCollect.size() == garbageToCollect.capacity()) return false; - auto& data = preloadedFiles[id]; + auto it = preloadedFiles.find(id); + if (it == preloadedFiles.end()) { + // Getting here means that the preloadedFiles got changed (probably cleared) + // while the lastUsedFiles were untouched. + ASSERTFALSE; + return true; + } + + sfz::FileData& data = it->second; if (data.status == FileData::Status::Preloaded) return true;