From cd2c84c15768f31c98a306142e03765ef48827bb Mon Sep 17 00:00:00 2001 From: Matthias von Faber Date: Thu, 1 Oct 2020 22:34:58 +0200 Subject: [PATCH] Remove CriticalSection, replace with std::mutex Didn't seem to be used recursively. If it is after all, that's a bug that needs fixing. If it can't be fixed right away, there's always std::recursive_mutex to restore the old behavior. --- CMakeLists.txt | 1 - src/common/SurgePatch.cpp | 4 +- src/common/SurgeStorage.cpp | 19 ++++---- src/common/SurgeStorage.h | 4 +- src/common/SurgeSynthesizer.cpp | 26 ++++++----- src/common/WavSupport.cpp | 6 +-- src/common/gui/COscillatorDisplay.cpp | 8 ++-- src/common/gui/SurgeGUIEditor.cpp | 18 ++++---- src/common/gui/SurgeGUIEditor.h | 4 -- src/common/thread/CriticalSection.cpp | 40 ----------------- src/common/thread/CriticalSection.h | 62 --------------------------- src/vst2/Vst2PluginInstance.cpp | 2 + 12 files changed, 42 insertions(+), 152 deletions(-) delete mode 100644 src/common/thread/CriticalSection.cpp delete mode 100644 src/common/thread/CriticalSection.h diff --git a/CMakeLists.txt b/CMakeLists.txt index a48c0a093a6..82b82de290b 100644 --- a/CMakeLists.txt +++ b/CMakeLists.txt @@ -209,7 +209,6 @@ set(SURGE_COMMON_SOURCES src/common/dsp/Wavetable.cpp src/common/dsp/WavetableOscillator.cpp src/common/dsp/WindowOscillator.cpp - src/common/thread/CriticalSection.cpp src/common/util/FpuState.cpp src/common/vt_dsp/basic_dsp.cpp src/common/vt_dsp/halfratefilter.cpp diff --git a/src/common/SurgePatch.cpp b/src/common/SurgePatch.cpp index 340ea6a5687..2ec0da22a59 100644 --- a/src/common/SurgePatch.cpp +++ b/src/common/SurgePatch.cpp @@ -975,7 +975,7 @@ void SurgePatch::load_patch(const void* data, int datasize, bool preset) void* d = (void*)((char*)dr + sizeof(wt_header)); - storage->CS_WaveTableData.enter(); + storage->waveTableDataMutex.lock(); scene[sc].osc[osc].wt.BuildWT(d, *wth, false); if( scene[sc].osc[osc].wavetable_display_name[0] == '\0' ) { @@ -988,7 +988,7 @@ void SurgePatch::load_patch(const void* data, int datasize, bool preset) strncpy(scene[sc].osc[osc].wavetable_display_name, "(Patch Wavetable)", 256); } } - storage->CS_WaveTableData.leave(); + storage->waveTableDataMutex.unlock(); dr += ph->wtsize[sc][osc]; } diff --git a/src/common/SurgeStorage.cpp b/src/common/SurgeStorage.cpp index c23b1f6b09e..09e1e496a81 100644 --- a/src/common/SurgeStorage.cpp +++ b/src/common/SurgeStorage.cpp @@ -978,10 +978,10 @@ bool SurgeStorage::load_wt_wt(string filename, Wavetable* wt) data = malloc(ds); read = fread(data, 1, ds, f); // FIXME - error if read != ds - - CS_WaveTableData.enter(); + + waveTableDataMutex.lock(); bool wasBuilt = wt->BuildWT(data, wh, false); - CS_WaveTableData.leave(); + waveTableDataMutex.unlock(); free(data); if (!wasBuilt) @@ -1086,8 +1086,7 @@ void SurgeStorage::clipboard_copy(int type, int scene, int entry) return; } - // CS ENTER - CS_ModRouting.enter(); + modRoutingMutex.lock(); { clipboard_p.clear(); @@ -1138,8 +1137,7 @@ void SurgeStorage::clipboard_copy(int type, int scene, int entry) } } } - // CS LEAVE - CS_ModRouting.leave(); + modRoutingMutex.unlock(); } void SurgeStorage::clipboard_paste(int type, int scene, int entry) @@ -1191,8 +1189,7 @@ void SurgeStorage::clipboard_paste(int type, int scene, int entry) return; } - // CS ENTER - CS_ModRouting.enter(); + modRoutingMutex.lock(); { for (int i = start; i < n; i++) @@ -1274,8 +1271,8 @@ void SurgeStorage::clipboard_paste(int type, int scene, int entry) } } } - // CS LEAVE - CS_ModRouting.leave(); + + modRoutingMutex.unlock(); } TiXmlElement* SurgeStorage::getSnapshotSection(const char* name) diff --git a/src/common/SurgeStorage.h b/src/common/SurgeStorage.h index 5dd16561260..7c90a5cd3a1 100644 --- a/src/common/SurgeStorage.h +++ b/src/common/SurgeStorage.h @@ -19,8 +19,8 @@ #include "ModulationSource.h" #include "Wavetable.h" #include -#include #include +#include #include #include @@ -857,7 +857,7 @@ class alignas(16) SurgeStorage void storeMidiMappingToName( std::string name ); // float table_sin[512],table_sin_offset[512]; - Surge::CriticalSection CS_WaveTableData, CS_ModRouting; + std::mutex waveTableDataMutex, modRoutingMutex; Wavetable WindowWT; float note_to_pitch(float x); diff --git a/src/common/SurgeSynthesizer.cpp b/src/common/SurgeSynthesizer.cpp index a692c81e476..c834e43d0c7 100644 --- a/src/common/SurgeSynthesizer.cpp +++ b/src/common/SurgeSynthesizer.cpp @@ -2087,7 +2087,7 @@ float SurgeSynthesizer::getModulation(long ptag, modsources modsource) void SurgeSynthesizer::clear_osc_modulation(int scene, int entry) { - storage.CS_ModRouting.enter(); + storage.modRoutingMutex.lock(); vector::iterator iter; int pid = storage.getPatch().scene[scene].osc[entry].p[0].param_id_in_scene; @@ -2111,7 +2111,7 @@ void SurgeSynthesizer::clear_osc_modulation(int scene, int entry) else iter++; } - storage.CS_ModRouting.leave(); + storage.modRoutingMutex.unlock(); } void SurgeSynthesizer::clearModulation(long ptag, modsources modsource, bool clearEvenIfInvalid) @@ -2144,9 +2144,9 @@ void SurgeSynthesizer::clearModulation(long ptag, modsources modsource, bool cle { if ((modlist->at(i).destination_id == id) && (modlist->at(i).source_id == modsource)) { - storage.CS_ModRouting.enter(); + storage.modRoutingMutex.lock(); modlist->erase(modlist->begin() + i); - storage.CS_ModRouting.leave(); + storage.modRoutingMutex.unlock(); return; } } @@ -2173,7 +2173,7 @@ bool SurgeSynthesizer::setModulation(long ptag, modsources modsource, float val) modlist = &storage.getPatch().scene[scene - 1].modulation_voice; } - storage.CS_ModRouting.enter(); + storage.modRoutingMutex.lock(); int id = storage.getPatch().param_ptr[ptag]->param_id_in_scene; if (!scene) @@ -2211,7 +2211,7 @@ bool SurgeSynthesizer::setModulation(long ptag, modsources modsource, float val) modlist->at(found_id).depth = value; } } - storage.CS_ModRouting.leave(); + storage.modRoutingMutex.unlock(); return true; } @@ -2786,8 +2786,7 @@ void SurgeSynthesizer::process() clear_block_antidenormalnoise(fxsendout[1][1], BLOCK_SIZE_QUAD); } - // CS ENTER - storage.CS_ModRouting.enter(); + storage.modRoutingMutex.lock(); processControl(); amp.set_target_smoothed(db_to_linear(storage.getPatch().volume.val.f)); @@ -2854,7 +2853,7 @@ void SurgeSynthesizer::process() iter++; } - storage.CS_ModRouting.leave(); + storage.modRoutingMutex.unlock(); fbq_global g; g.FU1ptr = GetQFPtrFilterUnit(storage.getPatch().scene[s].filterunit[0].type.val.i, @@ -2895,11 +2894,10 @@ void SurgeSynthesizer::process() v->GetQFB(); // save filter state in voices after quad processing is done iter++; } - storage.CS_ModRouting.enter(); + storage.modRoutingMutex.lock(); } - // CS LEAVE - storage.CS_ModRouting.leave(); + storage.modRoutingMutex.unlock(); polydisplay = vcount; if (play_scene[0]) @@ -3136,7 +3134,7 @@ void SurgeSynthesizer::swapMetaControllers( int c1, int c2 ) strncpy( storage.getPatch().CustomControllerLabel[c1], storage.getPatch().CustomControllerLabel[c2], 16 ); strncpy( storage.getPatch().CustomControllerLabel[c2], nt, 16 ); - storage.CS_ModRouting.enter(); + storage.modRoutingMutex.lock(); auto tmp1 = storage.getPatch().scene[0].modsources[ms_ctrl1 + c1]; auto tmp2 = storage.getPatch().scene[0].modsources[ms_ctrl1 + c2]; @@ -3188,7 +3186,7 @@ void SurgeSynthesizer::swapMetaControllers( int c1, int c2 ) } } - storage.CS_ModRouting.leave(); + storage.modRoutingMutex.unlock(); refresh_editor = true; } diff --git a/src/common/WavSupport.cpp b/src/common/WavSupport.cpp index 3062f1359a2..428ea6f6854 100644 --- a/src/common/WavSupport.cpp +++ b/src/common/WavSupport.cpp @@ -463,11 +463,9 @@ void SurgeStorage::load_wt_wav_portable(std::string fn, Wavetable *wt) if( wavdata && wt ) { - CS_WaveTableData.enter(); - + waveTableDataMutex.lock(); wt->BuildWT(wavdata, wh, wh.flags & wtf_is_sample); - - CS_WaveTableData.leave(); + waveTableDataMutex.unlock(); free( wavdata ); } return; diff --git a/src/common/gui/COscillatorDisplay.cpp b/src/common/gui/COscillatorDisplay.cpp index 7164ce64a2d..74595641063 100644 --- a/src/common/gui/COscillatorDisplay.cpp +++ b/src/common/gui/COscillatorDisplay.cpp @@ -185,10 +185,10 @@ void COscillatorDisplay::draw(CDrawContext* dc) { if (uses_wavetabledata(oscdata->type.val.i)) { - storage->CS_WaveTableData.enter(); + storage->waveTableDataMutex.lock(); osc->process_block(disp_pitch_rs); block_pos = 0; - storage->CS_WaveTableData.leave(); + storage->waveTableDataMutex.unlock(); } else { @@ -304,7 +304,7 @@ void COscillatorDisplay::draw(CDrawContext* dc) rmenu.inset(14, 0); char wttxt[256]; - storage->CS_WaveTableData.enter(); + storage->waveTableDataMutex.lock(); int wtid = oscdata->wt.current_id; if( oscdata->wavetable_display_name[0] != '\0' ) @@ -324,7 +324,7 @@ void COscillatorDisplay::draw(CDrawContext* dc) strcpy(wttxt, "(Patch Wavetable)"); } - storage->CS_WaveTableData.leave(); + storage->waveTableDataMutex.unlock(); char* r = strrchr(wttxt, '.'); if (r) diff --git a/src/common/gui/SurgeGUIEditor.cpp b/src/common/gui/SurgeGUIEditor.cpp index 5bf71cd620c..3e160acda5e 100644 --- a/src/common/gui/SurgeGUIEditor.cpp +++ b/src/common/gui/SurgeGUIEditor.cpp @@ -51,6 +51,10 @@ #include #include +#if WINDOWS +#include +#endif + #if TARGET_VST3 #include "pluginterfaces/vst/ivstcontextmenu.h" #include "pluginterfaces/base/ustring.h" @@ -246,8 +250,6 @@ SurgeGUIEditor::SurgeGUIEditor(void* effect, SurgeSynthesizer* synth, void* user _userdata = userdata; this->synth = synth; - // ToolTipWnd = 0; - minimumZoom = 50; #if LINUX minimumZoom = 100; // See github issue #628 @@ -394,7 +396,7 @@ void SurgeGUIEditor::idle() int ptag = ((CControl*)v)->getTag() - start_paramtags; if (ptag >= 0) { - synth->storage.CS_ModRouting.enter(); + synth->storage.modRoutingMutex.lock(); for (int i = 1; i < n_modsources; i++) { @@ -403,19 +405,19 @@ void SurgeGUIEditor::idle() ->update_rt_vals(synth->isActiveModulation(ptag, (modsources)i), 0, synth->isModsourceUsed((modsources)i)); } - synth->storage.CS_ModRouting.leave(); + synth->storage.modRoutingMutex.unlock(); } } else { - synth->storage.CS_ModRouting.enter(); + synth->storage.modRoutingMutex.lock(); for (int i = 1; i < n_modsources; i++) { if( gui_modsrc[i] ) ((CModulationSourceButton*)gui_modsrc[i]) ->update_rt_vals(false, 0, synth->isModsourceUsed((modsources)i)); } - synth->storage.CS_ModRouting.leave(); + synth->storage.modRoutingMutex.unlock(); } #if MAC idleinc++; @@ -787,7 +789,7 @@ void SurgeGUIEditor::refresh_mod() if( cms->hasAlternate && cms->useAlternate ) thisms = (modsources)cms->alternateId; - synth->storage.CS_ModRouting.enter(); + synth->storage.modRoutingMutex.lock(); for (int i = 0; i < n_paramslots; i++) { if (param[i]) @@ -817,7 +819,7 @@ void SurgeGUIEditor::refresh_mod() } #endif - synth->storage.CS_ModRouting.leave(); + synth->storage.modRoutingMutex.unlock(); for (int i = 1; i < n_modsources; i++) { diff --git a/src/common/gui/SurgeGUIEditor.h b/src/common/gui/SurgeGUIEditor.h index 74805c671cd..8414687c386 100644 --- a/src/common/gui/SurgeGUIEditor.h +++ b/src/common/gui/SurgeGUIEditor.h @@ -368,10 +368,6 @@ class SurgeGUIEditor : public EditorType, public VSTGUI::IControlListener, publi VSTGUI::CControl* lfodisplay = nullptr; VSTGUI::CControl* filtersubtype[2] = {}; VSTGUI::CControl* fxmenu = nullptr; -#if MAC || LINUX -#else - HWND ToolTipWnd; -#endif int clear_infoview_countdown = 0; public: int clear_infoview_peridle = -1; diff --git a/src/common/thread/CriticalSection.cpp b/src/common/thread/CriticalSection.cpp deleted file mode 100644 index 03955707f6b..00000000000 --- a/src/common/thread/CriticalSection.cpp +++ /dev/null @@ -1,40 +0,0 @@ -#include "CriticalSection.h" - -#if WINDOWS - -#include "assert.h" - -namespace Surge { - -CriticalSection::CriticalSection() -{ - refcount = 0; - InitializeCriticalSection(&cs); -} - -CriticalSection::~CriticalSection() -{ - DeleteCriticalSection(&cs); -} - -void CriticalSection::enter() -{ - EnterCriticalSection(&cs); - refcount++; - assert(refcount > 0); - assert(!(refcount > 10)); // if its >10 there's some crazy *§%* going on ^^ -} - -void CriticalSection::leave() -{ - refcount--; - if (refcount < 0) - { - int breakpointme = 0; - } - assert(refcount >= 0); - LeaveCriticalSection(&cs); -} - -} -#endif diff --git a/src/common/thread/CriticalSection.h b/src/common/thread/CriticalSection.h deleted file mode 100644 index 256f85d416c..00000000000 --- a/src/common/thread/CriticalSection.h +++ /dev/null @@ -1,62 +0,0 @@ -#pragma once - -#if MAC | LINUX -#include -#include -#include - -#else -#include "windows.h" -#endif - -namespace Surge { - -#if MAC | LINUX - -class CriticalSection -{ -public: - CriticalSection(){ - pthread_mutexattr_t attr; - pthread_mutexattr_init(&attr); - pthread_mutexattr_settype(&attr, PTHREAD_MUTEX_RECURSIVE); - pthread_mutex_init(&mutex, &attr); - } - - ~CriticalSection(){ - } - - void enter(){ - pthread_mutex_lock(&mutex); - refcount++; - assert(refcount > 0); - assert(!(refcount > 10)); // if its >10 there's some crazy *§%* going on ^^ - } - void leave(){ - refcount--; - assert(refcount >= 0); - pthread_mutex_unlock(&mutex); - } - -protected: - pthread_mutex_t mutex; - int refcount = 0; -}; - -#else - -class CriticalSection -{ -public: - CriticalSection(); - ~CriticalSection(); - void enter(); - void leave(); - -protected: - CRITICAL_SECTION cs; - int refcount; -}; - -#endif -} diff --git a/src/vst2/Vst2PluginInstance.cpp b/src/vst2/Vst2PluginInstance.cpp index ddd5aa2fc0a..607365d3540 100644 --- a/src/vst2/Vst2PluginInstance.cpp +++ b/src/vst2/Vst2PluginInstance.cpp @@ -28,6 +28,8 @@ #include #include #pragma STDC FENV_ACCESS on +#elif WINDOWS +#include #endif using namespace std;