LV2's "threading rules" not followed?

[PieterPenninckx]

Edit: conclusion: some plugins crash in Qtractor when "Auto Deactivate" is turned on (due to concurrent calls of lilv_instance_deactivate and lilv_instance_run)). This feature is not intended to be used in most cases; turning "Auto Deactivate" off resolves the issue.

I experienced some crashes with the sfizz plugin (sfztools/sfizz#1127 , see my comment). While rendering audio, the plugin uses an internal field, which is set to a NULL pointer by its deactivation method which is called from another thread while audio is being rendered.

Thinking about it some more, I think this may be because Qtractor maybe doesn't follow LV2's threading rules. These threading rules are defined in LV2's documentation and specify in particular that LV2_Descriptor::run should not run concurrently with LV2_Descriptor::deactivate. Looking at my stack traces, it seems that QTractor calls lilv_instance_deactivate from the main thread and lilv_instance_run from qtractorAudioEngine_process, which runs in a separate (audio?) thread. Looking at lilv's source code, it appears to me that lilv_instance_deactivate is just a wrapper around LV2_Descriptor::deactivate and lilv_instance_run is just a wrapper around LV2_Descriptor::run. Since these appear to not apply any thread synchronisation, I believe that LV2's threading rules should also be followed when calling lilv's methods. lilv's documentation mentions threading classes, but doesn't specify the threading rules; it may have been omitted from the documentation by accident.

So, in short, I think that

• Qtractor calls lilv_instance_deactivate and lilv_instance_run from separate threads
• lilv_instance_deactivate and lilv_instance_run should not be called from separate threads because of LV2's threading rules (edit: this is not correct. In fact, lilv_instance_deactivate and lilv_instance_run can run from separate threads, but not concurrently.)

[rncbc]

you might have a point, but unfortunately qtractor LV2 implementation predates by far the wordings in those later "LV2 threading rules" and so for "the book" by an even farther margin...

I'll try to detach the activate/deactivate guards in a later review, but for all that matters, since the alleged threading rules violation is in fact there for more than a decade now, I'll let you craft a PoC or PR in the mean time ;)

thanks && cheers

EDIT: I think you're misunderstanding those threading rules ... to put it short, even jalv, the LV2/lilv reference host implementation, doesn't issue lilv_instance_(de)activate() on the same thread context as lilv_instance_run() and I doubt any other LV2 host would do such a thing...

[PieterPenninckx]

Thank you for your quick reply, Rui!

You are right: calling lilv_instance_deactivate and lilv_instance_run from separate threads is not an issue. In fact, calling lilv_instance_deactivate from the (realtime) audio thread is likely going to give you xruns and not a good idea, which is likely why other hosts don't do that either.

I should have written

• Qtractor calls lilv_instance_deactivate and lilv_instance_run concurrently
• lilv_instance_deactivate and lilv_instance_run should not be called concurrently because of LV2's threading rules

So solving this is not as easy as moving the call to lilv_instance_deactivate to another thread, but rather involves some thread synchronisation. What thread synchronisation mechanism do you usually use when one thread is a real-time thead?

[rncbc]

yes but... that's prolly an issue to be tamed by the plugin, not the host.

Qtractor calls lilv_instance_deactivate and lilv_instance_run concurrently

almost sure every other Lv2 host does the same

[rerdavies]

@rncbc: Pretty sure NO other lv2 host does the same. AFAIK, the only reason to call deactivate is in preparation for deleting the plugin, or maybe just maybe if the plugin is being bypassed on the realtime thread. What exactly do you think deactivate is going to do?

[rncbc]

@rerdavies : exactly what qtractor lv2 host does: deactivates the plugin to bypass the realtime thread and/or prior to removal from the plugin chain.

[rerdavies]

Surely you should be calling deactivate AFTER removal from the realtime plugin chain, so no race condition there.

As for the bypass case, I'm hard-pressed to think any value at all for calling deactivate (and activate as well?). Just bypass the plugin on the realtime thread, and don't bother with the activate/deactivate calls. All you're going to get is a small reduction in memory use, offset by a nightmare without a solution for plugins that have to execute run() and activate()/deactivate() concurrently.

If you really must call deactivate, it's easy enough for you (the host) to delay the deactivate call until you receive a message from the realtime thread indicating the the plugin actually is bypassed on the realtime thread.

[rncbc]

OK. there's this mitigation over the develop branch; can you test it?

[PieterPenninckx]

Hi @rncbc, thank you for the very quick mitigation!

I have a test file that I can use for testing since it can quickly generate crashes (probably caused by this bug). I've also compiled the version of Qtractor from the develop branch. Unfortunately, when I open the test file with it, all sfizz puligns (which are on the track, not on the bus) are deactivated (also when playing) and they do not generate sound. I can open the plugin's user-interface. Trying to activate them via the pop-up menu that comes up with the right-mouse-button does not help. When I create a new file and add a sfizz plugin, it is activated (or at least, that's what the UI shows, I did not try to generate sound). I reverted my compiled version to the one that corresponds with commit fe96cc57aa and then the plugins are activated and generate sound.

[rncbc]

@PieterPenninckx : please note that the green-led may show the plugin is "Activated" but under this so called "mitigation" this is now only telling whether the plugin is being bypassed (led is off/black) or not (led is lit green); under the said "mitigation", actual call to the plugin's activate() is done only once, on the very first/initial activation or insert; conversely deactivate() is only called once on plugin removal or session closing.

it is strange that this seems to be not working for you; can you share your test file?

also, have you the main menu Track > Auto Deactivate option turned on? if yes, then please turn it off now: it doesn't work at all under the current mitigation.

[PieterPenninckx]

@PieterPenninckx : please note that [...]

Thanks for clarifying. The plugins stay bypassed, also when playing.

it is strange that this seems to be not working for you; can you share your test file?

Yes, I've sent it via e-mail. Looking at the file itself, I see the <activated>0</activated> tag for every plugin. That may be the issue. I don't have time anymore to try editing the xml file; I will try later and let you know.

also, have you the main menu Track > Auto Deactivate option turned on? if yes, then please turn it off now: it doesn't work at all under the current mitigation.

Yes, I had that option turned on. Turning it off didn't fix my issue (of the plugins staying bypassed).

I tried with a "brand new file" and that works: not only is the led lit green, it also actually plays :-)

I expect that the <activated>0</activated> is the cause of my issue with my test file. I'll try editing and let you know later, but I assume this is the issue.

[rncbc]

please redo your assesments with the Track > Auto Deactivate off.

turn it off; quit and start again your tests... hth

[rncbc]

now hopefully fixed in qtractor >= 0.9.34.13git.49752c

nb. when Track > Auto Deactivate is on, the so called "mitigation" is overruled, so that plugins may get actually activate()'d and deactivate()'d in-flight (eg. changing current hilighted track or start/stopping playback, and all that concurrently to the realtime run() thread, obviously...)

so, again, turn Track > Auto Deactivate off and do your thing ;)

[PieterPenninckx]

Hi Rui, turning "Track > Auto Deactivate" alone didn't do the job (even when closing and re-opening Qtractor).

When I edited the XML file and replaced <activated>0</activated> by <activated>1</activated> (and turned Track > Auto Deactivate off), it worked: I got sound :smile:

I used my test file that I used to recreate crashes and it didn't crash anymore. Thank you so much!

Some thing I noted:

• when I use an instrument (piano in sfizz) with the sustain pedal on, the sound continues playing when I and stop the transport and select another track and mute the (piano) track. I assume that Qtractor sends an all-sound-off midi event to the midi plugins, so even if the plugin is not bypassed, the plugin should listen to the all-sound-off midi event and stop producing sound. I think it is worth mentioning since, based on your description, I would assume that the sfizz plugin is bypassed when transport is not playing (and the track is not selected), so it would not produce any sound anymore, even if the plugin (incorrectly) ignores the all-sound-off midi event.

[rncbc]

Track > Auto Deactivate is tricky and not really a recommendation here

it was proposed long ago and indulged to whom was on some underpowered RPi's as far to keep the cpu% load due on activated plugins as low as possible, when idle (playback is not rolling).

if you're not short on resources, please don't turn it on. ever.

otoh. are you testing for the latest git master (qtractor 0.9.34.16git.a3c021) ? if not please do.

[PieterPenninckx]

Hi @rncbc , thank you for providing context.

Having "Track > Auto Deactivate" turned on was indeed the cause of the crashes that I experienced. I have now also tried with an older version (0.9.27 or something) and then I didn't have any crashes anymore.

This makes me wonder what the so-called "mitigation" actually does (I thought that it created the possibility to turn off "Auto Deactivate", but that was a misunderstanding from my side).

I will close this issue since the problem that I originally reported (crashes due to concurrent calls of lilv_instance_deactivate and lilv_instance_run) is only when using the "Auto Deactivate" feature, which is not intended to be used in my case.

[drobilla]

The threading rules have more or less always been what they are (if perhaps clarified a bit at times, in any case it's pretty obvious that calling deactivate concurrently with run is not okay).

On could certainly argue that we need lighter activate() and deactivate() functions in the same threading class as run() so you can cheaply signal that the host isn't going to call run() for a bit without having to go through all of this business, but we don't, and actually doing bypass properly is hard. There are some conversations about that on the mailing list, and there is https://lv2plug.in/ns/ext/parameters#bypass which is somewhat related, but in general this is an area of things that's still just straight-up copied from LADSPA and we haven't really figured it out yet.