Replace AtomicGuards by mutexes with try_lock

src/sfizz/AtomicGuard.h

@@ -1,114 +0,0 @@
-// SPDX-License-Identifier: BSD-2-Clause
-
-// This code is part of the sfizz library and is licensed under a BSD 2-clause
-// license. You should have receive a LICENSE.md file along with the code.
-// If not, contact the sfizz maintainers at https://github.com/sfztools/sfizz
-
-/**
- * @brief This file contains a pair of RAII helpers that handle some form
- * of lock-free mutex-type protection adapter to audio applications where you have 1 priority thread
- * that should never block and would rather return silence than wait, and another low-priority
- * thread that handles long computations.
- *
- * @code{.cpp}
- *
- * // Somewhere in a class...
- * std::atomic<bool> canEnterCallback;
- * std::atomic<bool> inCallback;
- *
- * void functionThatSuspendsCallback()
- * {
- *     AtomicDisabler callbackDisabler { canEnterCallback };
- *
- *     while (inCallback) {
- *         std::this_thread::sleep_for(1ms);
- *     }
- *
- * 	   // Do your thing.
- * }
- *
- * void callback(int samplesPerBlock) noexcept
- * {
- *     AtomicGuard callbackGuard { inCallback };
- *     if (!canEnterCallback)
- *         return;
- *
- * 	   // Do your thing.
- * }
- * @endcode
- * There are probably many ways to improve these and probably even debug them.
- * The spinlocking itself could be integrated in the constructor, although the
- * check for return in the callback could not.
- */
-#include <atomic>
-
-namespace sfz
-{
-/**
- * @brief Simple class to set an atomic to true and automatically set it back to false on
- * destruction.
- *
- * You call it like this assuming you need indicate that you are in e.g. a callback
- * @code{.cpp}
- * void functionToProtect()
- * {
- * 		AtomicGuard { guard };
- *
- * 		// Do stuff, the atomic will be set back to false as soon as you're back
- * }
- * @endcode
- * Note that this is not thread-safe at all, in the sense that it is only meant to be
- * used with 2 threads along with the AtomicDisabler. One thread uses AtomicGuards, the other
- * AtomicDisablers, and no other contending thread can share this pair of atomics.
- */
-class AtomicGuard
-{
-public:
-	AtomicGuard() = delete;
-	AtomicGuard(std::atomic<bool>& guard)
-	: guard(guard)
-	{
-		guard = true;
-	}
-	~AtomicGuard()
-	{
-		guard = false;
-	}
-private:
-	std::atomic<bool>& guard;
-};
-
-/**
- * @brief Simple class to set an atomic to false and automatically set it back to true on
- * destruction.
- *
- * You call it like this assuming you need to disable e.g. a callback
- * @code{.cpp}
- * void functionThatDisableAnotherFunction()
- * {
- * 		AtomicDisabler { disabler };
- *
- * 		// Do stuff, the atomic will be set back to true as soon as you're back
- * }
- * @endcode
- * Note that this is not thread-safe at all, in the sense that it is only meant to be
- * used with 2 threads along with the AtomicGuard. One thread uses AtomicGuards, the other
- * AtomicDisabler, and no other contending thread can share this pair of atomics.
- */
-class AtomicDisabler
-{
-public:
-	AtomicDisabler() = delete;
-	AtomicDisabler(std::atomic<bool>& allowed)
-	: allowed(allowed)
-	{
-		allowed = false;
-	}
-	~AtomicDisabler()
-	{
-		allowed = true;
-	}
-private:
-	std::atomic<bool>& allowed;
-};
-}

src/sfizz/Defer.h

@@ -0,0 +1,22 @@
+#pragma once
+
+// From https://stackoverflow.com/questions/48117908/is-the-a-practical-way-to-emulate-go-language-defer-in-c-or-c-destructors
+#include<type_traits>
+#include<utility>
+
+template<typename F>
+struct deferred
+{
+    std::decay_t<F> f;
+    template<typename G>
+    deferred(G&& g) : f{std::forward<G>(g)} {}
+    ~deferred() { f(); }
+};
+
+template<typename G>
+deferred(G&&) -> deferred<G>;
+
+#define CAT_(x, y) x##y
+#define CAT(x, y) CAT_(x, y)
+#define ANONYMOUS_VAR(x) CAT(x, __LINE__)
+#define DEFER deferred ANONYMOUS_VAR(defer_variable) = [&]

src/sfizz/EQPool.cpp

@@ -1,5 +1,5 @@
 #include "EQPool.h"
-#include "AtomicGuard.h"
+#include "Defer.h"
 #include <thread>
 #include "absl/algorithm/container.h"
 #include "SIMDHelpers.h"
@@ -108,9 +108,9 @@ sfz::EQPool::EQPool(const MidiState& state, int numEQs)
 
 sfz::EQHolderPtr sfz::EQPool::getEQ(const EQDescription& description, unsigned numChannels, float velocity)
 {
-    AtomicGuard guard { givingOutEQs };
+    if (!eqGuard.try_lock())
-    if (!canGiveOutEQs)
         return {};
+    DEFER { eqGuard.unlock(); };
 
     auto eq = absl::c_find_if(eqs, [](const EQHolderPtr& holder) {
         return holder.use_count() == 1;
@@ -132,10 +132,7 @@ size_t sfz::EQPool::getActiveEQs() const
 
 size_t sfz::EQPool::setnumEQs(size_t numEQs)
 {
-    AtomicDisabler disabler { canGiveOutEQs };
+    const std::lock_guard eqLock { eqGuard };
-
-    while(givingOutEQs)
-        std::this_thread::sleep_for(std::chrono::milliseconds(1));
 
     auto eqIterator = eqs.begin();
     auto eqSentinel = eqs.rbegin();

src/sfizz/EQPool.h

@@ -4,6 +4,7 @@
 #include "MidiState.h"
 #include <vector>
 #include <memory>
+#include <mutex>
 
 namespace sfz
 {
@@ -114,8 +115,7 @@ public:
      */
     void setSampleRate(float sampleRate);
 private:
-    std::atomic<bool> givingOutEQs { false };
+    std::mutex eqGuard;
-    std::atomic<bool> canGiveOutEQs { true };
     float sampleRate { config::defaultSampleRate };
     const MidiState& midiState;
     std::vector<EQHolderPtr> eqs;

src/sfizz/FilePool.cpp

@@ -29,7 +29,7 @@
 #include "Config.h"
 #include "Debug.h"
 #include "Oversampler.h"
-#include "AtomicGuard.h"
+#include "Defer.h"
 #include "absl/types/span.h"
 #include "absl/strings/match.h"
 #include "absl/memory/memory.h"
@@ -301,10 +301,7 @@ void sfz::FilePool::setPreloadSize(uint32_t preloadSize) noexcept
 
 void sfz::FilePool::tryToClearPromises()
 {
-    AtomicDisabler disabler { canAddPromisesToClear };
+    const std::lock_guard promiseLock { promiseGuard };
-
-    while (addingPromisesToClear)
-        std::this_thread::sleep_for(std::chrono::milliseconds(1));
 
     for (auto& promise: promisesToClear) {
         if (promise->dataStatus != FilePromise::DataStatus::Wait)
@@ -376,10 +373,9 @@ void sfz::FilePool::clear()
 
 void sfz::FilePool::cleanupPromises() noexcept
 {
-    AtomicGuard guard { addingPromisesToClear };
+    if (!promiseGuard.try_lock())
-
-    if (!canAddPromisesToClear)
         return;
+    DEFER { promiseGuard.unlock(); };
 
     // The garbage collection cleared the data from these so we can move them
     // back to the empty queue

src/sfizz/FilePool.h

@@ -38,6 +38,7 @@
 #include "Logger.h"
 #include <chrono>
 #include <thread>
+#include <mutex>
 
 namespace sfz {
 using AudioBufferPtr = std::shared_ptr<AudioBuffer<float>>;
@@ -264,8 +265,7 @@ private:
     std::vector<FilePromisePtr> emptyPromises;
     std::vector<FilePromisePtr> temporaryFilePromises;
     std::vector<FilePromisePtr> promisesToClear;
-    std::atomic<bool> addingPromisesToClear { false };
+    std::mutex promiseGuard;
-    std::atomic<bool> canAddPromisesToClear { true };
 
     // Preloaded data
     absl::flat_hash_map<absl::string_view, FileDataHandle> preloadedFiles;

src/sfizz/FilterPool.cpp

@@ -1,7 +1,7 @@
 #include "FilterPool.h"
 #include "SIMDHelpers.h"
 #include "absl/algorithm/container.h"
-#include "AtomicGuard.h"
+#include "Defer.h"
 #include <thread>
 #include <chrono>
 
@@ -109,9 +109,9 @@ sfz::FilterPool::FilterPool(const MidiState& state, int numFilters)
 
 sfz::FilterHolderPtr sfz::FilterPool::getFilter(const FilterDescription& description, unsigned numChannels, int noteNumber, float velocity)
 {
-    AtomicGuard guard { givingOutFilters };
+    if (!filterGuard.try_lock())
-    if (!canGiveOutFilters)
         return {};
+    DEFER { filterGuard.unlock(); };
 
     auto filter = absl::c_find_if(filters, [](const FilterHolderPtr& holder) {
         return holder.use_count() == 1;
@@ -133,10 +133,7 @@ size_t sfz::FilterPool::getActiveFilters() const
 
 size_t sfz::FilterPool::setNumFilters(size_t numFilters)
 {
-    AtomicDisabler disabler { canGiveOutFilters };
+    const std::lock_guard filterLock { filterGuard };
-
-    while(givingOutFilters)
-        std::this_thread::sleep_for(std::chrono::milliseconds(1));
 
     auto filterIterator = filters.begin();
     auto filterSentinel = filters.rbegin();

src/sfizz/FilterPool.h

@@ -4,6 +4,7 @@
 #include "MidiState.h"
 #include <vector>
 #include <memory>
+#include <mutex>
 
 namespace sfz
 {
@@ -118,8 +119,7 @@ public:
      */
     void setSampleRate(float sampleRate);
 private:
-    std::atomic<bool> givingOutFilters { false };
+    std::mutex filterGuard;
-    std::atomic<bool> canGiveOutFilters { true };
     float sampleRate { config::defaultSampleRate };
     const MidiState& midiState;
     std::vector<FilterHolderPtr> filters;

src/sfizz/Synth.cpp

@@ -5,7 +5,7 @@
 // If not, contact the sfizz maintainers at https://github.com/sfztools/sfizz
 
 #include "Synth.h"
-#include "AtomicGuard.h"
+#include "Defer.h"
 #include "Config.h"
 #include "Debug.h"
 #include "Macros.h"
@@ -28,21 +28,16 @@ sfz::Synth::Synth()
 
 sfz::Synth::Synth(int numVoices)
 {
+    const std::lock_guard disableCallback { callbackGuard };
     parser.setListener(this);
-
     effectFactory.registerStandardEffectTypes();
-
     effectBuses.reserve(5); // sufficient room for main and fx1-4
-
     resetVoices(numVoices);
 }
 
 sfz::Synth::~Synth()
 {
-    AtomicDisabler callbackDisabler { canEnterCallback };
+    const std::lock_guard disableCallback { callbackGuard };
-    while (inCallback) {
-        std::this_thread::sleep_for(std::chrono::milliseconds(1));
-    }
 
     for (auto& voice : voices)
         voice->reset();
@@ -128,10 +123,7 @@ void sfz::Synth::buildRegion(const std::vector<Opcode>& regionOpcodes)
 
 void sfz::Synth::clear()
 {
-    AtomicDisabler callbackDisabler { canEnterCallback };
+    const std::lock_guard disableCallback { callbackGuard };
-    while (inCallback) {
-        std::this_thread::sleep_for(std::chrono::milliseconds(1));
-    }
 
     for (auto& voice : voices)
         voice->reset();
@@ -327,13 +319,9 @@ void addEndpointsToVelocityCurve(sfz::Region& region)
 
 bool sfz::Synth::loadSfzFile(const fs::path& file)
 {
-    AtomicDisabler callbackDisabler { canEnterCallback };
-    while (inCallback) {
-        std::this_thread::sleep_for(std::chrono::milliseconds(1));
-    }
-
     clear();
 
+    const std::lock_guard disableCallback { callbackGuard };
     parser.parseFile(file);
     if (parser.getErrorCount() > 0)
         return false;
@@ -508,11 +496,7 @@ void sfz::Synth::setSamplesPerBlock(int samplesPerBlock) noexcept
 {
     ASSERT(samplesPerBlock < config::maxBlockSize);
 
-    AtomicDisabler callbackDisabler { canEnterCallback };
+    const std::lock_guard disableCallback { callbackGuard };
-
-    while (inCallback) {
-        std::this_thread::sleep_for(std::chrono::milliseconds(1));
-    }
 
     this->samplesPerBlock = samplesPerBlock;
     for (auto& voice : voices)
@@ -528,10 +512,7 @@ void sfz::Synth::setSamplesPerBlock(int samplesPerBlock) noexcept
 
 void sfz::Synth::setSampleRate(float sampleRate) noexcept
 {
-    AtomicDisabler callbackDisabler { canEnterCallback };
+    const std::lock_guard disableCallback { callbackGuard };
-    while (inCallback) {
-        std::this_thread::sleep_for(std::chrono::milliseconds(1));
-    }
 
     this->sampleRate = sampleRate;
     for (auto& voice : voices)
@@ -558,10 +539,9 @@ void sfz::Synth::renderBlock(AudioSpan<float> buffer) noexcept
     if (freeWheeling)
         resources.filePool.waitForBackgroundLoading();
 
-
+    if (!callbackGuard.try_lock())
-    AtomicGuard callbackGuard { inCallback };
-    if (!canEnterCallback)
         return;
+    DEFER { callbackGuard.unlock(); };
 
 
     size_t numFrames = buffer.getNumFrames();
@@ -655,9 +635,9 @@ void sfz::Synth::noteOn(int delay, int noteNumber, uint8_t velocity) noexcept
     ScopedTiming logger { dispatchDuration, ScopedTiming::Operation::addToDuration };
     resources.midiState.noteOnEvent(delay, noteNumber, normalizedVelocity);
 
-    AtomicGuard callbackGuard { inCallback };
+    if (!callbackGuard.try_lock())
-    if (!canEnterCallback)
         return;
+    DEFER { callbackGuard.unlock(); };
 
     noteOnDispatch(delay, noteNumber, normalizedVelocity);
 }
@@ -671,9 +651,9 @@ void sfz::Synth::noteOff(int delay, int noteNumber, uint8_t velocity) noexcept
     ScopedTiming logger { dispatchDuration, ScopedTiming::Operation::addToDuration };
     resources.midiState.noteOffEvent(delay, noteNumber, normalizedVelocity);
 
-    AtomicGuard callbackGuard { inCallback };
+    if (!callbackGuard.try_lock())
-    if (!canEnterCallback)
         return;
+    DEFER { callbackGuard.unlock(); };
 
     // FIXME: Some keyboards (e.g. Casio PX5S) can send a real note-off velocity. In this case, do we have a
     // way in sfz to specify that a release trigger should NOT use the note-on velocity?
@@ -767,9 +747,9 @@ void sfz::Synth::cc(int delay, int ccNumber, uint8_t ccValue) noexcept
     ScopedTiming logger { dispatchDuration, ScopedTiming::Operation::addToDuration };
     resources.midiState.ccEvent(delay, ccNumber, normalizedCC);
 
-    AtomicGuard callbackGuard { inCallback };
+    if (!callbackGuard.try_lock())
-    if (!canEnterCallback)
         return;
+    DEFER { callbackGuard.unlock(); };
 
     if (ccNumber == config::resetCC) {
         resetAllControllers(delay);
@@ -961,16 +941,12 @@ int sfz::Synth::getNumVoices() const noexcept
 void sfz::Synth::setNumVoices(int numVoices) noexcept
 {
     ASSERT(numVoices > 0);
+    const std::lock_guard disableCallback { callbackGuard };
     resetVoices(numVoices);
 }
 
 void sfz::Synth::resetVoices(int numVoices)
 {
-    AtomicDisabler callbackDisabler { canEnterCallback };
-    while (inCallback) {
-        std::this_thread::sleep_for(std::chrono::milliseconds(1));
-    }
-
     voices.clear();
     for (int i = 0; i < numVoices; ++i)
         voices.push_back(absl::make_unique<Voice>(resources));
@@ -986,10 +962,7 @@ void sfz::Synth::resetVoices(int numVoices)
 
 void sfz::Synth::setOversamplingFactor(sfz::Oversampling factor) noexcept
 {
-    AtomicDisabler callbackDisabler { canEnterCallback };
+    const std::lock_guard disableCallback { callbackGuard };
-    while (inCallback) {
-        std::this_thread::sleep_for(std::chrono::milliseconds(1));
-    }
 
     for (auto& voice : voices)
         voice->reset();
@@ -1006,10 +979,7 @@ sfz::Oversampling sfz::Synth::getOversamplingFactor() const noexcept
 
 void sfz::Synth::setPreloadSize(uint32_t preloadSize) noexcept
 {
-    AtomicDisabler callbackDisabler { canEnterCallback };
+    const std::lock_guard disableCallback { callbackGuard };
-    while (inCallback) {
-        std::this_thread::sleep_for(std::chrono::milliseconds(1));
-    }
 
     resources.filePool.setPreloadSize(preloadSize);
 }
@@ -1036,11 +1006,12 @@ void sfz::Synth::disableFreeWheeling() noexcept
 
 void sfz::Synth::resetAllControllers(int delay) noexcept
 {
-    AtomicGuard callbackGuard { inCallback };
-    if (!canEnterCallback)
-        return;
-
     resources.midiState.resetAllControllers(delay);
+
+    if (!callbackGuard.try_lock())
+        return;
+    DEFER { callbackGuard.unlock(); };
+
     for (auto& voice : voices) {
         voice->registerPitchWheel(delay, 0);
         for (int cc = 0; cc < config::numCCs; ++cc)
@@ -1086,10 +1057,7 @@ void sfz::Synth::disableLogging() noexcept
 
 void sfz::Synth::allSoundOff() noexcept
 {
-    AtomicDisabler callbackDisabler { canEnterCallback };
+    const std::lock_guard disableCallback { callbackGuard };
-    while (inCallback) {
-        std::this_thread::sleep_for(std::chrono::milliseconds(1));
-    }
 
     for (auto& voice : voices)
         voice->reset();

src/sfizz/Synth.h

@@ -18,6 +18,7 @@
 #include "absl/types/span.h"
 #include <absl/types/optional.h>
 #include <random>
+#include <mutex>
 #include <set>
 #include <string_view>
 #include <vector>
@@ -206,6 +207,7 @@ public:
      * @param noteNumber the midi note number
      * @param velocity the midi note velocity
      */
+
     void noteOff(int delay, int noteNumber, uint8_t velocity) noexcept;
     /**
      * @brief Send a CC event to the synth
@@ -436,13 +438,7 @@ private:
      *
      */
     void clear();
-    /**
+
-     * @brief Resets and possibly changes the number of voices (polyphony) in
-     * the synth.
-     *
-     * @param numVoices
-     */
-    void resetVoices(int numVoices);
     /**
      * @brief Helper function to dispatch <global> opcodes
      *
@@ -475,6 +471,13 @@ private:
      * @param regionOpcodes the opcodes that are specific to the region
      */
     void buildRegion(const std::vector<Opcode>& regionOpcodes);
+    /**
+     * @brief Resets and possibly changes the number of voices (polyphony) in
+     * the synth.
+     *
+     * @param numVoices
+     */
+    void resetVoices(int numVoices);
 
     fs::file_time_type checkModificationTime();
 
@@ -526,9 +529,7 @@ private:
     std::uniform_real_distribution<float> randNoteDistribution { 0, 1 };
     unsigned fileTicket { 1 };
 
-    // Atomic guards; must be used with AtomicGuard and AtomicDisabler
+    std::mutex callbackGuard;
-    std::atomic<bool> canEnterCallback { true };
-    std::atomic<bool> inCallback { false };
     bool freeWheeling { false };
 
     // Singletons passed as references to the voices