APP->history and multiple threads

For Oneiroi, I have a CV input to randomise the state. We can either do Rack style onRandomise, or do a built in specialised randomise that mirrors the behaviour of the module. If doing the latter I record the changes as a ComplexAction and apply APP->history->push(randomizeAction); so we can undo them. This works great with single threaded mode, but if multi-threaded this reliably causes a crash (double free of an action, in Rack/src/history.cpp at v2 · VCVRack/Rack · GitHub ).

Should I not call APP->history->push from audio thread (because that can lead to multiple threads competing to modify the undo actions variable)?

EDIT: stoermelder STRIP has some special logic to deal with this, I can’t easily follow but it tells me this is a non-standard thing to do vcvrack-packone/src/Strip.cpp at b8f505c8e44f198d04274c4bc1501b3872e07124 · stoermelder/vcvrack-packone · GitHub

2 Likes

Creating undo actions on CV triggers is highly questionable. Do you have a corresponding trigger to undo the action? Undo/redo are fundamentally UI gestures. Creating undo records likely causes allocations, which are to be avoided on audio threads.

2 Likes

Understood. Note it’s not actually “create undo on trigger” more “randomise on trigger” and there is a use case to “undo the randomise that just happened”.

It’s a convience, not an essential feature so can just remove. I was just curious if there was a thread safe way to push undo actions to the stack but it sounds difficult!

1 Like

I think we had similar decisions to make for ShapeMaster, and I seem to recall thinking the undo should be related to UI actions, as just mentioned, and not CV, so not supporting undo/redo in these cases sounds justified to me.

4 Likes

whether it is a good idea or not, why would you not accomplish this by having the CV randomize (1) set an atomic; (2) have a step method in the UI read the atomic, push the undo event, and randomize, and clear the atomic.

Since push and randomize are ui thread events, this seems just like any other version of ‘make the audio thread do something on the ui thread’ which is always either an atomic update or a lock free queue, right?

(I personally think undoable randomize on cv change is kinda a fun idea, but understand the critiques of fellow devs).

2 Likes