LV2: `program_changed` call deadlocks Carla

[kramlie]

I have only witnessed this while developing features for Yoshimi, so I'm not sure if this is reproducible in the wild yet. However, you don't really need to reproduce the issue to see what the problem is.

If the LV2 plugin calls program_changed with -1, it triggers a full reload of all programs. This happens in handleProgramChanged. Inside of reloadPrograms, setMidiProgram will usually be called. The problem is that setMidiProgram will attempt to use ScopedSingleProcessLocker, but handleProgramChanged already holds this lock, leading to a deadlock.

I would have submitted a pull request for this myself, but I'm not completely sure how to handle this situation, since the setMidiProgram is used from multiple places, so the lock can't just be removed. reloadPrograms is also used from multiple places, and only one place has the lock. So I thought I'd ask here first.

We could potentially pass in a bool to signal whether we are already holding the lock or not. Other ideas?

[falkTX]

if only deadlocks if you dont follow the threading rules around this API https://github.com/KXStudio/LV2-Extensions/blob/master/kx-programs.lv2/programs.h#L160

[kramlie]

@falkTX: Yes, I already hit that one 😄, and that mistake is on me.

But this is a different deadlock. I have fixed the issue with calling from RT context, and I'm certain I'm calling it from the right thread now (a low prio worker thread).

If we take a look at this backtrace (see inline comments in uppercase):

#0  __lll_clocklock_wait (futex=futex@entry=0x7fffdfa9d974, val=val@entry=0, clockid=clockid@entry=0, abstime=abstime@entry=0x0, private=private@entry=0) at lll_timedlock_wait.c:59
#1  0x00007ffff7d34ea1 in __pthread_mutex_lock_full (mutex=0x5fb71d8) at ../nptl/pthread_mutex_lock.c:432
#2  0x00007fffeeab3984 in CarlaMutex::lock() const (this=0x5fb71d8) at ../../utils/CarlaMutex.hpp:71
#3  0x00007fffeeb0e21f in CarlaBackend::CarlaPlugin::ScopedSingleProcessLocker::ScopedSingleProcessLocker(CarlaBackend::CarlaPlugin*, bool) (this=0x7fffdfa9da50, plugin=0x5efcbd0, block=true) at CarlaPlugin.cpp:2715
#4  0x00007fffeeb4a89f in CarlaBackend::CarlaPluginLV2::setMidiProgram(int, bool, bool, bool, bool) (this=0x5efcbd0, index=0, sendGui=true, sendOsc=true, sendCallback=true, doingInit=false) at CarlaPluginLV2.cpp:1734
    ^--- ATTEMPTS TO GRAB THE SAME LOCK HERE
#5  0x00007fffeeb52d7a in CarlaBackend::CarlaPluginLV2::reloadPrograms(bool) (this=0x5efcbd0, doInit=false) at CarlaPluginLV2.cpp:3544
#6  0x00007fffeeb5be2d in CarlaBackend::CarlaPluginLV2::handleProgramChanged(int) (this=0x5efcbd0, index=-1) at CarlaPluginLV2.cpp:5762
    ^--- LOCK IS GRABBED HERE
#7  0x00007fffeeb62544 in CarlaBackend::CarlaPluginLV2::carla_lv2_program_changed(void*, int) (handle=0x5efcbd0, index=-1) at CarlaPluginLV2.cpp:7688
#8  0x00007fffec0ba47c in YoshimiLV2Plugin::<lambda()>::operator()(void) const (__closure=0x64b6498) at ~/code/yoshimi/src/LV2_Plugin/YoshimiLV2Plugin.cpp:397
#9  ...

We can see this is all happening in one single thread. At least to my eyes it looks like it will always deadlock if it calls setMidiProgram, which happens whenever there is a change in the number of programs. Or am I missing something obvious here..?