MessageManagerLock hangs

Since a few versions of Reaper, when loading a plug of mine that has an AudioProcessorGraph, during the rendering sequence it keeps hanging in the following piece of code of the MessageManagerLock::init routine:

while (! blockingMessage->lockedEvent.wait (20)) { if ((threadToCheck != nullptr && threadToCheck->threadShouldExit()) || (job != nullptr && job->shouldExit())) { blockingMessage->releaseEvent.signal(); blockingMessage = nullptr; MessageManager::instance->lockingLock.exit(); return; } }
threadToCheck and job are both nullptr, so the only way out is via the while condition in the beginning, which is never fulfilled. Unfortunately the debugger jumps to the wrong line numbers when following that call (damn visual studio…). Any idear what might be going wrong here?

Well, the class is highly susceptible to deadlock, which is why you should always provide a thread pointer for it to check - from the comments:

If you pass zero for the thread object, it will wait indefinitely for the lock - be careful when doing this, because it's very easy to deadlock if your message thread attempts to call stopThread() on a thread just as that thread attempts to get the message lock.

Well, it comes from here, in the AudioProcessorGraph code:

[code]void AudioProcessorGraph::buildRenderingSequence()
Array<void*> newRenderingOps;
int numRenderingBuffersNeeded = 2;
int numMidiBuffersNeeded = 1;

	MessageManagerLock mml;

	Array<void*> orderedNodes;
		const GraphRenderingOps::ConnectionLookupTable table (connections);

		for (int i = 0; i < nodes.size(); ++i)
			Node* const node = nodes.getUnchecked(i);

			node->prepare (getSampleRate(), getBlockSize(), this);

			int j = 0;
			for (; j < orderedNodes.size(); ++j)
				if (table.isAnInputTo (node->nodeId, ((Node*) orderedNodes.getUnchecked(j))->nodeId))

			orderedNodes.insert (j, node);

	GraphRenderingOps::RenderingOpSequenceCalculator calculator (*this, orderedNodes, newRenderingOps);

	numRenderingBuffersNeeded = calculator.getNumBuffersNeeded();
	numMidiBuffersNeeded = calculator.getNumMidiBuffersNeeded();

What do you suggest ?

Ah. Mea culpa. Unfortunately, because in that case it’s the audio thread, there’s no way to use a safe MM lock for it.

Presumably the deadlock is because your UI thread is trying to stop the audio engine, and is blocking the UI thread while it waits for it to stop?

The call stack is resume() -> prepareToPlay()-> buildRenderingSequence() , all on the main thread… My question is, can Reaper be blamed for this or not? :?

IMHO no, i think the use of MMLock in a Plugin is generally unsafe, because you never now how the host handles GUI and Audio-Thread…

So doesn’t the MML at this point do more harm than good then? What changes in that section that, if requested by the message thread, could give intermediate wrong results (at least that’s how I understand the MML at this point) ?

chkn’s right that in a plugin it’s just generally a bad idea to use an MM lock, because there are so many unknowns about what the host is doing.

I guess I wrote that class without really considering that it might be used in a plugin, so didn’t worry about using a lock there. To avoid the problem in your case, I think it’d need some careful refactoring to use a different locking scheme. If that was an easy change, I’d do so, but it’d probably be quite complicated and I’m in the middle of some other stuff at the moment…

I see… could you maybe explain just briefly what exactly you want to prevent, maybe I can try my luck.

AudioProcessorGraph should use its own lock to prevent its data integrity, and all accessors (GUI) should use methods to get its info (no pointers/only copies)

Or each thread should have its own read-only copy of the graph. When changes are made to the graph (in a lock-free fashion of course), each thread is notified of the modification via a call on it’s thread-specific queue. The call, which executes on the related thread, modifies that thread’s copy of the graph. The thread queue should be lock free and mostly wait-free. I have all this implemented in my app - and you can too!

Wouldn’t it be better if AudioProcessorGraph had a copy of its own graph on which it can make the changes and then, when it’s done, point the real thing to the copy?