Browse Source

LV2 Host: Avoid potential deadlock in AudioProcessorGraph

Previously, preparing an AudioProcessorGraph containing hosted LV2
plugins would recreate the plugins and then set their state. For plugins
without threadsafe restore, setting the state would take the
callbackLock to avoid races with processBlock. This meant that

- During prepare, the graph would take the processorLock, then the
  processor would take its own callbackLock.
- During playback, the graph would take the processor's callbackLock,
  and then would take the node's processorLock.

This is probably benign (prepare shouldn't be called concurrently with
processBlock at all), but to be on the safe side we now avoid taking the
callbackLock when setting new plugin state during prepareToPlay.
pull/22/head
reuk 3 years ago
parent
commit
7ea93ce5d2
No known key found for this signature in database GPG Key ID: 9ADCD339CFC98A11
1 changed files with 25 additions and 15 deletions
  1. +25
    -15
      modules/juce_audio_processors/format_types/juce_LV2PluginFormat.cpp

+ 25
- 15
modules/juce_audio_processors/format_types/juce_LV2PluginFormat.cpp View File

@@ -4405,7 +4405,8 @@ public:
numSamples, numSamples,
sampleRate); sampleRate);
setStateInformation (mb.getData(), (int) mb.getSize());
// prepareToPlay is *guaranteed* not to be called concurrently with processBlock
setStateInformationImpl (mb.getData(), (int) mb.getSize(), ConcurrentWithAudioCallback::no);
jassert (numSamples == instance->features.getMaxBlockSize()); jassert (numSamples == instance->features.getMaxBlockSize());
@@ -4490,7 +4491,8 @@ public:
return; return;
lastAppliedPreset = newProgram; lastAppliedPreset = newProgram;
applyStateWithAppropriateLocking (loadStateWithUri (presetUris[(size_t) newProgram]));
applyStateWithAppropriateLocking (loadStateWithUri (presetUris[(size_t) newProgram]),
ConcurrentWithAudioCallback::yes);
} }
const String getProgramName (int program) override const String getProgramName (int program) override
@@ -4527,16 +4529,7 @@ public:
void setStateInformation (const void* data, int size) override void setStateInformation (const void* data, int size) override
{ {
JUCE_ASSERT_MESSAGE_THREAD;
if (data == nullptr || size == 0)
return;
auto begin = static_cast<const char*> (data);
std::vector<char> copy (begin, begin + size);
copy.push_back (0);
auto mapFeature = instance->symap->getMapFeature();
applyStateWithAppropriateLocking (PluginState { lilv_state_new_from_string (world->get(), &mapFeature, copy.data()) });
setStateInformationImpl (data, size, ConcurrentWithAudioCallback::yes);
} }
void setNonRealtime (bool newValue) noexcept override void setNonRealtime (bool newValue) noexcept override
@@ -4576,6 +4569,8 @@ public:
AudioProcessorParameter* getBypassParameter() const override { return bypassParam; } AudioProcessorParameter* getBypassParameter() const override { return bypassParam; }
private: private:
enum class ConcurrentWithAudioCallback { no, yes };
LV2AudioPluginInstance (std::shared_ptr<World> worldIn, LV2AudioPluginInstance (std::shared_ptr<World> worldIn,
const Plugin& pluginIn, const Plugin& pluginIn,
std::unique_ptr<InstanceWithSupports>&& in, std::unique_ptr<InstanceWithSupports>&& in,
@@ -4596,7 +4591,22 @@ private:
std::move (uiDescriptorIn), std::move (uiDescriptorIn),
[this] { postChangedParametersToUi(); }) [this] { postChangedParametersToUi(); })
{ {
applyStateWithAppropriateLocking (std::move (stateToApply));
applyStateWithAppropriateLocking (std::move (stateToApply), ConcurrentWithAudioCallback::no);
}
void setStateInformationImpl (const void* data, int size, ConcurrentWithAudioCallback concurrent)
{
JUCE_ASSERT_MESSAGE_THREAD;
if (data == nullptr || size == 0)
return;
auto begin = static_cast<const char*> (data);
std::vector<char> copy (begin, begin + size);
copy.push_back (0);
auto mapFeature = instance->symap->getMapFeature();
applyStateWithAppropriateLocking (PluginState { lilv_state_new_from_string (world->get(), &mapFeature, copy.data()) },
concurrent);
} }
// This does *not* destroy the editor component. // This does *not* destroy the editor component.
@@ -4694,13 +4704,13 @@ private:
return instance.get(); return instance.get();
} }
void applyStateWithAppropriateLocking (PluginState&& state)
void applyStateWithAppropriateLocking (PluginState&& state, ConcurrentWithAudioCallback concurrent)
{ {
PortMap portStateManager (instance->ports); PortMap portStateManager (instance->ports);
// If a plugin supports threadSafeRestore, its restore method is thread-safe // If a plugin supports threadSafeRestore, its restore method is thread-safe
// and may be called concurrently with audio class functions. // and may be called concurrently with audio class functions.
if (hasThreadSafeRestore)
if (hasThreadSafeRestore || concurrent == ConcurrentWithAudioCallback::no)
{ {
state.restore (*instance, portStateManager); state.restore (*instance, portStateManager);
} }


Loading…
Cancel
Save