JUCE with LUA (locking issues)

My program (a plugin, VST) is a standard plugin, it does no audio processing just MIDI. It has embedded lua for processing, the basic stuff it does is:

  1. when the user moves a knob on the GUI a LUA function is called (that function can update some other GUI elements on the plugin)
  2. the processBlock method in the plugin calls the corresponding processBlock function in LUA (implemented by the user, this also can change the GUI, same principal)

it’s all fine if i don’t change GUI elements in LUA in the processBlock, but when i start doing that, i get assetions about MessageManager locking. I tried locking the MessageManager like it says in the manual immediatly when the setValue method is called from lua, but that didn’t help (the setValue method of my class that catches LUA calls, modifies the GUI by calling the gui’s setValue() method). I put the MessageManager lock in the setValue() method of the GUI, that helped, cause i was able to open the editor and see the GUI changed from the processBlock (some text in the TextEditor called on each processBlock run). But when i touched a slider after a few moves i get a host+plugin freeze (a deadlock i imagine).

I’m stuck with how to do this locking, this plugin has 3 “layers”, the GUI layer, the VST layer and the LUA layer, all interact with each other somehow.

some code

int ModifierContainer::setValue (LuaVirtualMachine& vm)
{
        /* controllers are indexed with leading 1 not zero
           we could keep zero indexed in the global arrays but
           for consistency we just do a -1 for controller index here */

        lua_State *state = (lua_State *) vm;
        const int valueType = lua_type (state, -1);

        if (valueType == LUA_TNUMBER)
        {
                const uint32 newValue           = (uint32) lua_tonumber (state, -1);
                lua_remove (state, -1);
                const uint32 index                      = (uint32) lua_tonumber (state, -1);

                setEditorValue (index-1, newValue, true);

                return (true);
        }

        else if (valueType == LUA_TSTRING)
        {
                const String newValue (lua_tostring (state, -1));
                lua_remove (state, -1);
                const uint32 index                      = (uint32) lua_tonumber (state, -1);

                setEditorValue (index-1, newValue, true);

                return (true);
        }
        else
        {
                return (false);
        }
}

the setEditorValue method just checks if the editor is open (pointer is valid) and calls the editor method setControllerValueText

void DeviceEditor::setControllerValueText (const uint32 controllerId, const String controllerValue, const bool update)
{
	Component *c = getComponentById (controllerId);

	if (c)
	{
		TextEditor		*editor		= dynamic_cast <TextEditor*> (c);

		if (editor != 0)
		{
			const MessageManagerLock mmLock;

			editor->setText (controllerValue, update);
		}
	}
}

in the VST processBlock method i call

modifierContainer->luaProcessBlock(pos);

witch is defined as

void ModifierContainer::luaProcessBlock (const AudioPlayHead::CurrentPositionInfo lastPosInfo)
{
	lua_State *L = (lua_State *) m_vm;

	lua_pushstring (L, "position");
	lua_newtable (L);

	luaSetArrayElement (T("bpm"), (uint32)lastPosInfo.bpm);
	luaSetArrayElement (T("ppqPosition"), (uint32)lastPosInfo.ppqPosition);
	luaSetArrayElement (T("ppqPositionOfLastBarStart"), (uint32)lastPosInfo.ppqPositionOfLastBarStart);
	luaSetArrayElement (T("timeSigDenominator"), lastPosInfo.timeSigDenominator);
	luaSetArrayElement (T("timeSigNumerator"), lastPosInfo.timeSigNumerator);
	lua_settable (L, LUA_GLOBALSINDEX);


	if (scriptHasFunction ("processBlock"))
	{
		selectScriptFunction ("processBlock");
		go (0,0);
	}
}

when the gui is changed a lua method is called

/* internal setValue used just by this class */
void ModifierContainer::intSetValue (const uint32 index, const uint32 newValue)
{
	Modifier *m = modifierArray[index];

	if (m)
	{
		parent->getCallbackLock().enter();

		const String lua = m->getLuaCode();

		if (scriptHasFunction (lua.toUTF8()))
		{
			selectScriptFunction (lua.toUTF8());

			addParam (midiChannel);
			addParam (newValue);

			if (m->isController())
			{
				const uint32 n = m->getControllerNumber();
				addParam (n);
			}

			go (1, m);
			
			if (!luaDbg->error().isEmpty())
			{
				Logger::writeToLog (T("ModifierContainer::intSetValue Funcion call errors"));
				Logger::writeToLog (luaDbg->error());
			}
		}

		m->setValue (newValue);
		m = 0;

		parent->getCallbackLock().exit();
	}
}

in LUA the processBlock just does

    function this.processBlock (this)
      this:setValue (parameters["parameterName"], "processBlock");
    end

or the same this:setValue can be called on any part of the gui when the user touches it.

Could you perhaps give me some pointers on how to do locking the right way here. I spawn no additional Threads of my own (i don’t know about LUA internal stuff).

the code (working) is here http://code.google.com/p/ctrlr/source/browse/#svn/trunk/src

Unless I’m misreading you, isn’t the problem just that you’re trying to call UI methods from a thread other the message manager thread?

Perhaps you just need to use MessageManager::callFunctionOnMessageThread() ? Or alternatively just have your Lua function implementations store a state change, without trying to actually update the GUI. A ChangeMessage could then be used to cause the GUI to update itself from the stored state in its own time.

Sorry if I’m missing the point.

yeah, as valley said…
try to avoid messing with GUI stuff in the audio thread.

when you want to update the gui from the audio thread then it is a lot better to make lua post change messages in a already allocated fifo queue of messages (so no malloc’ing in the realtime thread), and let the GUI have a timer (better a thread waiting to be woke up from the audio one) that checks that queue and makes the changes in the GUI if it find something to do.

also, any kind of locking in the audio thread is be avoid like the plague…

i’ll use change listeners then (again :slight_smile: i already did but i had problems exchanging data on updates). i’ll try to avoid locking in the audio process, but like i said there is not audio processing done, just midi events and position changes.

The important thing is to make sure that there is no way your audio process block can be writing to your stored state variables whilst the GUI thread is trying to read from them. The obvious approach is a critical section, but as Kraken says, that’s not the way to go. Even though you aren’t doing much of anything in the process block, critical sections are expensive, especially if they have to wait for some UI redraws to happen.

If you really must use a critical section, use tryEnter() in the processBlock, and if you don’t get the lock, let the update happen in the next block, or the next after that if needs be.

Using a helper thread as Kraken suggests is the better approach in general, but there are a few pitfalls involved in doing so.

ok what if i use changeBroadcaster and changeListener (the gui), do i need to lock the gui when it receives onChangeUpdate?

also how do i pass data to the changeUpdate, my class that catches the changes from LUA and VST has a lot of components, i need to indicate to the gui withc one changed, each component is a class (class Modifier), i keep them in a OwnedArray, can i pass a pointer to that class when doing sendChangesMessage() (to that Modifier class) or do i have to pass the parent class (ModifierContainer class) as the pointer to that call and the gui has to look for the “dirty” component.

Here’s the situation you’re trying to avoid:

Some group of variables, or block of RAM called ‘B’ is used by thread ‘A’ to store some data that it thinks thread ‘C’ might be interested in.

As long as C is never trying to read the contents of B whilst A is writing to it, all is well in the garden. Typically one would use a critical section to stop A or C accessing B whilst it is in use by the other.

You know all this of course. The problem is that you’re looking for a solution that doesn’t require a critical section for A. You can lock C just fine, but locking A will upset the apple cart.

Your first question:

  • should be obvious if you step back and just think of things in terms of two threads trying to read from, or write to one block of memory at the same time. All the change listener is buying you here is that it’s moving the transfer of values over to the GUI thread. That’s a big deal in that most JUCE components don’t like being manipulated on threads other than the GUI thread. It doesn’t buy you thread safety though. The reason being that the change event is just waking up the GUI thread, but since it is a different thread to the audio process thread, you’re going to get race conditions on the shared memory ‘B’.

The first thing you need to decide is whether your UI can ‘drop’ events or not. Assuming that the last event will always get through, in many cases losing some events is perfectly fine. At the very worst the user might notice a little jerkiness as controls update on screen when the system is under load. That’s preferable to hearing stuttering though, so it’s a good trade off to make. However, if you need to catch all events, a log window for example can’t miss some MIDI notes, then you’ll need to do a little extra work.

By the sounds of it, you could potentially be moving a fair chunk of data from the audio thread to the GUI thread. Given this assumption, the first thing you’ll probably want to do is to group all of the data into a single structure so that it can be passed as a pointer. In doing this, you give yourself another handy tool. It’s easy to swap buffers if the buffers are located as a single pointer. Consider:

struct MyLargeStruct{};

MyLargeStruct buffer1;
MyLargeStruct buffer2;

MyLargeStruct* bufferInUseByUI = &buffer2;

process()
{
  MyLargeStruct* writeBuffer;
  if (bufferInUseByUI == &buffer2)
    writeBuffer = &buffer1;
  else 
    writeBuffer = &buffer2;

  // do your work, and write the output into writeBuffer
  
  bufferInUseByUI = writeBuffer;
  sendChangeMessage(this);
}

In this way, the UI thread is always reading from the alternate buffer to the one to which the audio thread is writing.

One obvious problem with this example is that if the UI is still processing the last change event by the time two audio blocks have passed, you’ve got a race condition. In most cases this is unlikely, but if your UI is working hard, then you’d need to handle that case far more robustly. In this case, some kind of growable buffer (consider a linked list of your struct type) might be a workable solution.

Essentially, the ideal solution is going to be very much a factor of how much data you’re moving around, how expensive your UI components are, and how essential it is for every event to make it to the UI.

it doesn’t matter, you are processing thing in a function called by the host in a realtime thread going out to the hardware device. even if you are doing only midi, if you lock here, you will lock the whole processing thread so other plugins may block and you will finish getting clicks and stutters if the situation get critical (you are blocking for too long).

You could try to work with the actual plugin parameters.

  • Every parameter is a Parameter class, which is a AsyncUpdater.
  • You register your Parameters in the plugin
  • Each GUI components will attach to one or more of this Parameters
  • From lua you trigger an async update (of the parameter) in the audio thread, and in the handleAsyncUpdate of the parameter you change your attached gui component.
  • If the user tweak the gui, you will need to call setParameterNotifyingHost from the plugin, in which you setValue on your desired Parameter.
  • This way you can expose a sort of interface to LUA for getting the value of the parameters and to set them (which also trigger the async to the gui)

this involves some mallocing in the audio thread but it definately works.

well i need to re-think my model a little, thanks for the help.

i’ll try to use changeListener stuff again. i can live with loosing events, this plugin does not log anything it’s just a controller for hardware, i don’t expect massive amounts of events and those that come just need to update the position of sliders on the screen.

i dropped processBlock in lua for now it causes too many issues, maybe later, i have to do some kind of IPC MIDI port sharing for windows for now that’s more important. I changed the gui updates to changeListener and giving the objectThatChanged pointer to a simple class that hold simple information abut the Modifier that changed (it’s value it’s name, index and so on) the GUI components all have a ID property corresponding to the ID of the Modifier so that’s how i find them on update. It looks like it’s working.

well i tired this again what i did

when the “nasty” user moves a knob on the gui, lua function is called and that function can call setValue witch then maps to my method that updates other components, this uses changeMessage and the pointer to the Parameter that changed

there is also a processBlock in lua that runs everytime a processBlock in the plugin runs, this uses setValueSafe to update Parameters in the plugin, it does that by adding a pointer to the changed parameter to a Array of “dirty” parameters, the GUI has a timerCallback and checks for those dirty parameters and updates it’s state.

no processBlock - everytinh works fine
if i enable processBlock i get heap/double free errors in FL (not in PluginHost), with this method i imagine i should do some locking when the GUI is accessing those dirty pointers, and possibly pass some kind of CriticalSection to the array that holds those dirty pointers, but i’m not sure how to do this.

just to update

when using lua to update gui in any way there is always a heap problem, but when i update from VST host (automation of parameters) i don’t get that. so the question is really is LUA VM a different thread ? how is it different from the VST host changing something in my plugin the methods that get called look the same, perhaps i should move the LUA methods somewhere else ?

i’ll try to show how this looks now


[Controller] - base class for the plugin
    |
    |----[ModifierContainer] a utility class that holds all parameters
    |            |
    |            |-----[Modifier] a single parameter, holds it's current value, max min 
    |            |        name and other properties (not gui)
    |            |
    |            |------[LUA VM] all lua calls are made here and all methods used 
    |            |          by lua are here (setValue)
    |            |
    |            |------[VST] all vst parameters handling are passed from the 
    |                       Controller class here, names indexing values
    |
    |----[ControllerEditor/DeviceEditor] - two GUI components, DeviceEditor is a subcomponent of ControllerEditor

what i have now working fine, is everything except the ability to change GUI from lua, any attempt (using pointers are changeMessages) to modify the GUI fails, i really don’t know how to make locking work here, how do i lock (and what) my components so that lua can safely modify a Modifier and so that a changeMessage can be broadcasted to DeviceEditor to indicate a change.

I tried putting CriticalSection in ModifierContainer and passing CriticalSection to the Array that holds Parameters (OwnedArray), but i’m doing this all blindly without the actual knowledge of what’s happening. I can’t debug this cause the heap errors happen after some time (sometrimes 5 minutes).

Lua’s VM runs on whichever thread calls lua_pcall(). If you call that function from the message handler thread, Lua will be running on the message handler thread. If you call it from the process thread, then it’ll instead be running on the process thread.

If you have one lua_State instance, and you’re calling into it from both the message manager (when the user adjusts a control) and the process thread, then potentially you’re going to have threading issues inside Lua itself.

Additionally, it should go without saying, calls from Lua into your other code will be on whichever thread is running lua_pcall().

In short, don’t think of the VM as being in anyway different to any other function call, because it isn’t. It’s just a long running function that happens to execute bytecode. Its behaviour in terms of how it acts in the context of threading is no different to a call to say StringArray::clear().

i don’t think you can share a lua_State object between multiple threads executing concurrently (at least from my limited knowledge of LUA itself) without any kind of locking.
this can be avoided if you spawn multiple states, so each one can’t corrupt the others (your processBlock state can’t corrupt your gui state), but you will then need some sort of messagin system for making them communicate (probably publishing a broadcaster in one and a listener in another state ? rant).

look here, can make things clear.

http://lua-users.org/wiki/ThreadsTutorial

from here you get:

...
Each thread in C which interacts with Lua will need its own Lua state. Each of these states has its own runtime stack. When a new C thread is started, you can create its Lua state in one of two ways. One way is to call lua_open. This creates a new state which is independent of the states in other threads. In this case, you'll need to initialize the Lua state (for example, loading libraries) as if it was the state of a new program. This approach eliminates the need for mutex locks (discussed below), but will keep the threads from sharing global data.

The other approach is to call lua_newthread. This creates a child state which has its own stack and which has access to global data.
...

no i have only one state and

i think i figured out

first i tried the AsyncUpdates and i got some results, but still somem crashes, then i started thinking what is beeing accessed by lua and gui at the same time (assuming that LUA is a different thread), and i think it’s the Modifier class that holds the current value name and other properties

so i added a CriticalSection to each Modifier and locked it’s get/set methods, now i don’t get any errors, i also moved to sendChangeMessage() and it’s working.

but what is weird for me is that LUA is not started in a different thread, but the same thread as the VST subsystem, so this shouldn’t cause any problems, unless lua itself works as a different thread.