This is a very specific question about general crashing safety.
This question is not about data consistency!
I have certain UI elements which are connected to values which live inside a ValueTree.
All my UI elements accessing/manipulating the values from the message thread.
Now I replace my root-ValueTree with another ValueTree from a background thread.
My UI-Elements than, of course, still referring to the old values/ValueTree, which I am aware of, this is 100% okay.
(The UI elements will tied to the new corresponding values/ValueTree from the message-Thread later)
So whats actually is happening, the reference count of the original ValueTree will be reduced and the ValueTree potentially destroyed.
Is it possible that this exchange of the root tree (aka changing the reference count/deleting the 0-referenced ValueTree Object) causes an access violation in any way?
As a rule of thumb, if the documentation for a class doesn’t mention thread safety, you should assume that a class is not thread safe, and use external synchronisation.
ValueTrees propagate notifications from leaf nodes up to parent nodes when the leaf nodes change, and these notifications will happen on the message thread. If the root ValueTree has listeners, and you call ValueTree::operator= from a background thread, there’s a chance you’ll end up trying to iterate the valueTreesWithListeners member on the main thread while modifying it on the background thread, which would be a data race. Even if there were no race, you’d need to ensure that all valueTreeRedirected callbacks could handle being called from threads other than the message thread.
Rather than modifying the ValueTree from a background thread, why not just prepare the ValueTree, and then use a mechanism like MessageManager::callAsync to do the actual installation on the message thread?
Can you confirm that? I see a straight path (setProperty > sendPropertyChangeMessage > callListenersForAllParents > callListeners > callExcluding > valueTreePropertyChanged) without any AsyncUpdater or callAsync involved.
An example where this is problematic is when the plugin state includes things that are not AudioProcessorParameters. If you’re using replaceState, you can get away with locking get/setStateInformation with the properties getters/setters. But replaceState is indiscriminate (it doesn’t check for missing or non-existent properties), and it clears undo history, which is terrible for preset selection. If you need to update manually, you’re not covered by APVTS’ lock, so you have to defer the extra properties to the message thread, which creates a window in which setState and getState don’t match.
Sorry, to be clear I was referring to the scenario posed in the original question. If the user is interacting with the UI controls and updating the ValueTree on the message thread, then there will be (synchronous) callbacks on the message thread. If the root node is swapped on a background thread while the user is updating the ValueTree on the message thread, then I believe there is a real danger of data races.
Thanks for clarification! Setting the ValueTree asynchronously would be a solution, but I also have the same problem with reading from the ValueTree, which can’t be done asynchronously. Also things not being synchronously can be a problem for itself. And using MessageManagerLock for that is risky, especially as plugin where you don’t have control about the Message-Thread.
In my project currently all access to the same ValueTree is handled under the scope of the same lock.
The only exception are values which are tied directly (via referTo) to GUI elements.
I guess i have to add another Value Access Wrapper which can use configured with a CriticalSection as an additional layer.
I ended up making my own version of ValueTreePropertyValueSource, which uses the lock for accessing the ValueTree.
Could you put your new value tree created in the background thread in a queue and swap it as soon as possible in the message thread? This way you make sure ui will never try to access the previous tree while it being swapped in the background thread
Yes i could, but things wouldn’t be synchronously, If the host does setStateInformation from a background thread, and then immediately renders from the audioThread, my plugin uses wrong non automable data (of course I take that there is no direct access from to the VT from processBlock, it uses a cache). Or if the host setStateInformation and the immediately reads from getStateInformation, data would be inconsistent.
Instead I use a wrapper which protects the ValueTree from simultan multithread access. (BTW CritcalSections act as memory fences?)
The only theoretical problem I had, was that values which control UI Elements like ComboBox and Sliders, didn’t access the ValueTree through the wrapper. I fixed it by creating a own version of ValueTreePropertyValueSource.
May I ask how you solved that? I have a lock between getStateInformation and the extra stuff setters, both from state changes and from the UI through a custom ValueSource, but I’m still deferring setState for these properties to the message thread because, if I understand correctly, I can’t protect APVTS flushes from them -for that I’d have to extend the valueTreeChanging lock to my own code, but we can’t access it. Or are you setting everything together with replaceState?
I don’t use APVTS, but from what regarding @reuk says, it has the same basic problem, when manipulating the ValueTree directly from messageThread. So I guess deferring setState for these properties to the message thread is a possible solution. But copyState() has the same problem when not called from the messageThread. But getStateInformation requires immediately a result, and no asynchrony mechanism can be used.
I choose a different approach where the access to the underlying ValueTree Structure is strictly tied to using a lock.