From 160b5860e25afe291a1697865f3759474f331bdc Mon Sep 17 00:00:00 2001 From: falkTX Date: Thu, 15 Sep 2022 00:46:14 +0100 Subject: [PATCH] Fix several race conditions Signed-off-by: falkTX --- source/backend/engine/CarlaEngine.cpp | 19 +++-- source/backend/engine/CarlaEngineGraph.cpp | 82 +++++++++++++------ source/backend/engine/CarlaEngineGraph.hpp | 2 +- source/backend/engine/CarlaEngineInternal.cpp | 26 ++++-- source/backend/engine/CarlaEngineInternal.hpp | 4 +- 5 files changed, 91 insertions(+), 42 deletions(-) diff --git a/source/backend/engine/CarlaEngine.cpp b/source/backend/engine/CarlaEngine.cpp index 662b321cc..d9470ccd4 100644 --- a/source/backend/engine/CarlaEngine.cpp +++ b/source/backend/engine/CarlaEngine.cpp @@ -860,7 +860,10 @@ bool CarlaEngine::addPlugin(const BinaryType btype, const float oldVolume = oldPlugin->getInternalParameterValue(PARAMETER_VOLUME); oldPlugin->prepareForDeletion(); - pData->pluginsToDelete.push_back(oldPlugin); + { + const CarlaMutexLocker cml(pData->pluginsToDeleteMutex); + pData->pluginsToDelete.push_back(oldPlugin); + } if (plugin->getHints() & PLUGIN_CAN_DRYWET) plugin->setDryWet(oldDryWet, true, true); @@ -939,12 +942,15 @@ bool CarlaEngine::removePlugin(const uint id) */ #else pData->curPluginCount = 0; - pData->plugins[0].plugin = nullptr; + pData->plugins[0].plugin.reset(); carla_zeroStruct(pData->plugins[0].peaks); #endif plugin->prepareForDeletion(); - pData->pluginsToDelete.push_back(plugin); + { + const CarlaMutexLocker cml(pData->pluginsToDeleteMutex); + pData->pluginsToDelete.push_back(plugin); + } callback(true, true, ENGINE_CALLBACK_PLUGIN_REMOVED, id, 0, 0, 0, 0.0f, nullptr); return true; @@ -969,7 +975,7 @@ bool CarlaEngine::removeAllPlugins() #ifndef BUILD_BRIDGE_ALTERNATIVE_ARCH if (pData->options.processMode == ENGINE_PROCESS_MODE_PATCHBAY) - pData->graph.removeAllPlugins(); + pData->graph.removeAllPlugins(pData->aboutToClose); #endif const ScopedActionLock sal(this, kEnginePostActionZeroCount, 0, 0); @@ -982,7 +988,10 @@ bool CarlaEngine::removeAllPlugins() EnginePluginData& pluginData(pData->plugins[id]); pluginData.plugin->prepareForDeletion(); - pData->pluginsToDelete.push_back(pluginData.plugin); + { + const CarlaMutexLocker cml(pData->pluginsToDeleteMutex); + pData->pluginsToDelete.push_back(pluginData.plugin); + } pluginData.plugin.reset(); carla_zeroStruct(pluginData.peaks); diff --git a/source/backend/engine/CarlaEngineGraph.cpp b/source/backend/engine/CarlaEngineGraph.cpp index 6808dd962..1ac229230 100644 --- a/source/backend/engine/CarlaEngineGraph.cpp +++ b/source/backend/engine/CarlaEngineGraph.cpp @@ -1476,9 +1476,10 @@ public: void reconfigure() override { - CARLA_SAFE_ASSERT_RETURN(fPlugin.get() != nullptr,); + const CarlaPluginPtr plugin = fPlugin; + CARLA_SAFE_ASSERT_RETURN(plugin.get() != nullptr,); - CarlaEngineClient* const client = fPlugin->getEngineClient(); + CarlaEngineClient* const client = plugin->getEngineClient(); CARLA_SAFE_ASSERT_RETURN(client != nullptr,); carla_stdout("reconfigure called"); @@ -1501,7 +1502,10 @@ public: const String getName() const override { - return fPlugin->getName(); + const CarlaPluginPtr plugin = fPlugin; + CARLA_SAFE_ASSERT_RETURN(plugin.get() != nullptr, String()); + + return plugin->getName(); } void processBlockWithCV(AudioSampleBuffer& audio, @@ -1509,7 +1513,9 @@ public: AudioSampleBuffer& cvOut, MidiBuffer& midi) override { - if (fPlugin.get() == nullptr || ! fPlugin->isEnabled() || ! fPlugin->tryLock(kEngine->isOffline())) + const CarlaPluginPtr plugin = fPlugin; + + if (plugin.get() == nullptr || !plugin->isEnabled() || !plugin->tryLock(kEngine->isOffline())) { audio.clear(); cvOut.clear(); @@ -1517,7 +1523,7 @@ public: return; } - if (CarlaEngineEventPort* const port = fPlugin->getDefaultEventInPort()) + if (CarlaEngineEventPort* const port = plugin->getDefaultEventInPort()) { EngineEvent* const engineEvents(port->fBuffer); CARLA_SAFE_ASSERT_RETURN(engineEvents != nullptr,); @@ -1528,7 +1534,7 @@ public: midi.clear(); - fPlugin->initBuffers(); + plugin->initBuffers(); const uint32_t numSamples = audio.getNumSamples(); const uint32_t numAudioChan = audio.getNumChannels(); @@ -1538,14 +1544,14 @@ public: if (numAudioChan+numCVInChan+numCVOutChan == 0) { // nothing to process - fPlugin->process(nullptr, nullptr, nullptr, nullptr, numSamples); + plugin->process(nullptr, nullptr, nullptr, nullptr, numSamples); } else if (numAudioChan != 0) { // processing audio, include code for peaks const uint32_t numChan2 = jmin(numAudioChan, 2U); - if (fPlugin->getAudioInCount() == 0) + if (plugin->getAudioInCount() == 0) audio.clear(); float* audioBuffers[numAudioChan]; @@ -1562,17 +1568,17 @@ public: float inPeaks[2] = { 0.0f }; float outPeaks[2] = { 0.0f }; - for (uint32_t i=0, count=jmin(fPlugin->getAudioInCount(), numChan2); igetAudioInCount(), numChan2); iprocess(const_cast(audioBuffers), audioBuffers, - cvInBuffers, cvOutBuffers, - numSamples); + plugin->process(const_cast(audioBuffers), audioBuffers, + cvInBuffers, cvOutBuffers, + numSamples); - for (uint32_t i=0, count=jmin(fPlugin->getAudioOutCount(), numChan2); igetAudioOutCount(), numChan2); isetPluginPeaksRT(fPlugin->getId(), inPeaks, outPeaks); + kEngine->setPluginPeaksRT(plugin->getId(), inPeaks, outPeaks); } else { @@ -1585,14 +1591,14 @@ public: for (uint32_t i=0; iprocess(nullptr, nullptr, - cvInBuffers, cvOutBuffers, - numSamples); + plugin->process(nullptr, nullptr, + cvInBuffers, cvOutBuffers, + numSamples); } midi.clear(); - if (CarlaEngineEventPort* const port = fPlugin->getDefaultEventOutPort()) + if (CarlaEngineEventPort* const port = plugin->getDefaultEventOutPort()) { /*const*/ EngineEvent* const engineEvents(port->fBuffer); CARLA_SAFE_ASSERT_RETURN(engineEvents != nullptr,); @@ -1601,12 +1607,15 @@ public: carla_zeroStructs(engineEvents, kMaxEngineEventInternalCount); } - fPlugin->unlock(); + plugin->unlock(); } const String getInputChannelName(ChannelType t, uint i) const override { - CarlaEngineClient* const client = fPlugin->getEngineClient(); + const CarlaPluginPtr plugin = fPlugin; + CARLA_SAFE_ASSERT_RETURN(plugin.get() != nullptr, String()); + + CarlaEngineClient* const client = plugin->getEngineClient(); switch (t) { @@ -1623,7 +1632,10 @@ public: const String getOutputChannelName(ChannelType t, uint i) const override { - CarlaEngineClient* const client(fPlugin->getEngineClient()); + const CarlaPluginPtr plugin = fPlugin; + CARLA_SAFE_ASSERT_RETURN(plugin.get() != nullptr, String()); + + CarlaEngineClient* const client(plugin->getEngineClient()); switch (t) { @@ -1641,8 +1653,21 @@ public: void prepareToPlay(double, int) override {} void releaseResources() override {} - bool acceptsMidi() const override { return fPlugin->getDefaultEventInPort() != nullptr; } - bool producesMidi() const override { return fPlugin->getDefaultEventOutPort() != nullptr; } + bool acceptsMidi() const override + { + const CarlaPluginPtr plugin = fPlugin; + CARLA_SAFE_ASSERT_RETURN(plugin.get() != nullptr, false); + + return plugin->getDefaultEventInPort() != nullptr; + } + + bool producesMidi() const override + { + const CarlaPluginPtr plugin = fPlugin; + CARLA_SAFE_ASSERT_RETURN(plugin.get() != nullptr, false); + + return plugin->getDefaultEventOutPort() != nullptr; + } private: CarlaEngine* const kEngine; @@ -2040,10 +2065,12 @@ void PatchbayGraph::removePlugin(const CarlaPluginPtr plugin) CARLA_SAFE_ASSERT_RETURN(graph.removeNode(node->nodeId),); } -void PatchbayGraph::removeAllPlugins() +void PatchbayGraph::removeAllPlugins(const bool aboutToClose) { carla_debug("PatchbayGraph::removeAllPlugins()"); + stopRunner(); + const bool sendHost = !usingExternalHost; const bool sendOSC = !usingExternalOSC; @@ -2062,6 +2089,9 @@ void PatchbayGraph::removeAllPlugins() graph.removeNode(node->nodeId); } + + if (!aboutToClose) + startRunner(100); } bool PatchbayGraph::connect(const bool external, @@ -2764,10 +2794,10 @@ void EngineInternalGraph::removePlugin(const CarlaPluginPtr plugin) fPatchbay->removePlugin(plugin); } -void EngineInternalGraph::removeAllPlugins() +void EngineInternalGraph::removeAllPlugins(const bool aboutToClose) { CARLA_SAFE_ASSERT_RETURN(fPatchbay != nullptr,); - fPatchbay->removeAllPlugins(); + fPatchbay->removeAllPlugins(aboutToClose); } bool EngineInternalGraph::isUsingExternalHost() const noexcept diff --git a/source/backend/engine/CarlaEngineGraph.hpp b/source/backend/engine/CarlaEngineGraph.hpp index 6a8a6cde5..ceb4ef35a 100644 --- a/source/backend/engine/CarlaEngineGraph.hpp +++ b/source/backend/engine/CarlaEngineGraph.hpp @@ -197,7 +197,7 @@ public: void reconfigureForCV(CarlaPluginPtr plugin, const uint portIndex, bool added); void reconfigurePlugin(CarlaPluginPtr plugin, bool portsAdded); void removePlugin(CarlaPluginPtr plugin); - void removeAllPlugins(); + void removeAllPlugins(bool aboutToClose); bool connect(bool external, uint groupA, uint portA, uint groupB, uint portB); bool disconnect(bool external, uint connectionId); diff --git a/source/backend/engine/CarlaEngineInternal.cpp b/source/backend/engine/CarlaEngineInternal.cpp index 65a251dff..7af29927a 100644 --- a/source/backend/engine/CarlaEngineInternal.cpp +++ b/source/backend/engine/CarlaEngineInternal.cpp @@ -409,6 +409,7 @@ CarlaEngine::ProtectedData::ProtectedData(CarlaEngine* const engine) xruns(0), dspLoad(0.0f), #endif + pluginsToDeleteMutex(), pluginsToDelete(), events(), #ifndef BUILD_BRIDGE_ALTERNATIVE_ARCH @@ -433,6 +434,8 @@ CarlaEngine::ProtectedData::~ProtectedData() CARLA_SAFE_ASSERT(plugins == nullptr); #endif + const CarlaMutexLocker cml(pluginsToDeleteMutex); + if (pluginsToDelete.size() != 0) { for (std::vector::iterator it = pluginsToDelete.begin(); it != pluginsToDelete.end(); ++it) @@ -571,22 +574,27 @@ void CarlaEngine::ProtectedData::initTime(const char* const features) void CarlaEngine::ProtectedData::deletePluginsAsNeeded() { - for (bool stop;;) + std::vector safePluginListToDelete; + + if (const size_t size = pluginsToDelete.size()) + safePluginListToDelete.reserve(size); + { - stop = true; + const CarlaMutexLocker cml(pluginsToDeleteMutex); - for (std::vector::iterator it = pluginsToDelete.begin(); it != pluginsToDelete.end(); ++it) + for (std::vector::iterator it = pluginsToDelete.begin(); it != pluginsToDelete.end();) { if (it->use_count() == 1) { - stop = false; + const CarlaPluginPtr plugin = *it; + safePluginListToDelete.push_back(plugin); pluginsToDelete.erase(it); - break; + } + else + { + ++it; } } - - if (stop) - break; } } @@ -614,7 +622,7 @@ void CarlaEngine::ProtectedData::doPluginRemove(const uint pluginId) noexcept const uint id = curPluginCount; // reset last plugin (now removed) - plugins[id].plugin.reset(); + plugins[id].plugin = nullptr; carla_zeroFloats(plugins[id].peaks, 4); } diff --git a/source/backend/engine/CarlaEngineInternal.hpp b/source/backend/engine/CarlaEngineInternal.hpp index 9e9d5dace..40d723d1d 100644 --- a/source/backend/engine/CarlaEngineInternal.hpp +++ b/source/backend/engine/CarlaEngineInternal.hpp @@ -117,7 +117,7 @@ public: void renamePlugin(CarlaPluginPtr plugin, const char* newName); void switchPlugins(CarlaPluginPtr pluginA, CarlaPluginPtr pluginB); void removePlugin(CarlaPluginPtr plugin); - void removeAllPlugins(); + void removeAllPlugins(bool aboutToClose); bool isUsingExternalHost() const noexcept; bool isUsingExternalOSC() const noexcept; @@ -288,6 +288,8 @@ struct CarlaEngine::ProtectedData { float dspLoad; #endif float peaks[4]; + + CarlaMutex pluginsToDeleteMutex; std::vector pluginsToDelete; EngineInternalEvents events;