Threading nightmares

borkman
Posts: 51
Joined: Tue May 09, 2023 7:26 pm

Re: Threading nightmares

Post by borkman »

Cherryaudio Danny wrote: Wed Jul 26, 2023 6:21 pm It is possible for notifications to come from the GUI thread, timer threads, and even the sound engine thread if adjustments are made to knobs within the ProcessSample() function. However, it is not recommended to modify knobs within the ProcessSample() function as this is a time-sensitive thread and GUI functions shouldn't occur within it.
Thanks for chiming in, Danny! The explanation you gave seems to concern performance. I have a more general question about correctness. Do module developers need to be concerned about thread synchronization? For example, is it safe to modify an instance of one of the standard yet not thread safe collection classes, say ArrayList, in Notify() and iterate it in ProcessSample()? If these are called from separate threads that can interleave, then the answer is no. Even simple assignment of a double value isn't thread safe. Do we need to be synchronizing all variable access? I hope not. :cry:
poetix
Posts: 54
Joined: Mon Nov 28, 2022 3:26 pm

Re: Threading nightmares

Post by poetix »

Yes, if you are accessing an unsynchronised collection both in ProcessSample() and in Notify(), you may well run into thread-safety issues. It's not necessarily a good idea to deal with these by synchronising the collection, since that involves placing locks around read/write access that might stall ProcessSample() while Notify() is doing work, and as a rule we want ProcessSample() to get its work done as expeditiously as possible.

So, suppose we have a module with three components: an input jack, a button, and an output jack. The input jack receives a trigger signal, and the button can be used to trigger the module manually. When triggered, the module adds a "voice" to a collection of voices which each play for a fixed length of time; the output jack receives the sum of the currently active voices.

Inside ProcessSample() we want to scan over a list of active voices, and get each to contribute a sample to the sum which we will write to the output jack. We also want to check the input jack to see if the input signal has crossed over from negative or zero to a positive voltage, which is the event we will treat as a trigger. When we detect that event, we add a new voice to the list of active voices, let's say by appending a Voice object to an ArrayList. Fine so far - no conflicts with anything else. But what about the button?

We get notified about the button press in Notify(), but if we append a new Voice object to the ArrayList there we run the risk of modifying the collection while ProcessSample() is iterating over it, which will cause a ConcurrentModificationException to be thrown inside ProcessSample(). So instead we need to pass information about the event that's occurred - the button press - over to ProcessSample() so that it can update the ArrayList prior to iteration. We can do that just by setting a boolean value somewhere - say a field called buttonPushed - and getting ProcessSample() to treat either a zero-crossing in the input signal or a true value for buttonPushed as a trigger event. When ProcessSample() detects a true value of buttonPushed, it adds a Voice to the ArrayList and sets buttonPushed to false. The field becomes our channel of communication across the two threads.

Do we need to worry about synchronisation over the field? Is it worth marking it as volatile, or wrapping it as an AtomicBoolean? Probably not, to be honest. It doesn't especially matter if it takes a few cycles for ProcessSample()'s thread to see the true value written by Notify(), and Notify() never checks the value of the field, it just sets it when the button is pressed. Writing a primitive field value is safe enough for our purposes.

Things get a bit hairier if the work we want to do when triggered might take a bit of time and could potentially hold up ProcessSample() - in that case we may need to start dispatching requests into a work queue, and handling them in another thread. If it gets really complicated we might want to use a lock-free structure like a Ring Buffer to dispatch things back and forth between ProcessSample() and our worker thread. (I've never yet needed to do anything like that, but I can see how the need might arise). The main thing to keep in mind is that ProcessSample() needs a stable "frame" of values to work on, and ideally needs not to be held up by anything that isn't sample-by-sample DSP processing or very lightweight bookkeeping - you wouldn't stick a fast fourier transform on a block of recorded samples in there, you'd hand that off to another thread to do while ProcessSample() keeps on trucking.
London, UK
Developer, Vulpus Labs
Musician, w/trem
ColinP
Posts: 953
Joined: Mon Aug 03, 2020 7:46 pm

Re: Threading nightmares

Post by ColinP »

Unless my diagnostics are incorrect then there is not synchronization and any non-atomic operations on shared data in VM are not thread-safe.

But even if you don't trust my diagnostics one can arrive at the same conclusion analytically because it's easy to block a Notify() thread by calling something like menu or dialog code. Yet even though that thread blocks the audio keeps working. Therefore ProcessSample() MUST asynchronously interrupt Notify() if they share the same process (which they do).

Fortunately in practice simple modules don't crash because they use the linear flow model I mentioned earlier where only primitive data is used and only UI threads write and ProcessSample() only reads shared data.

But as borkman correctly states operations on longs and doubles (even assignment) are not guaranteed to be atomic in the Java Virtual Machine and Get/GetValue use doubles so there is the postential for a thread switch to occur between the two 32 bit chunks. However in CA's code with a bit of luck these variables are marked with the volatile keyword so writes to them will be atomic.

However even if not it's statistically very unlikely ithat you'll ever split the 64 bits in two and if/when it does happen it's just going to create "noise" in the double's value. Noise that is likey to be filtered out by SmoothValue or any other filtering/slew limiting so it will result in a slight glitch or maybe some freakish behaviour like a button press being ignored once in a hundred presses for no apparent reason.

Now there is debate about whether double assignments actually are atomic or not in practice given that virtually everybody is now using 64 bit architecture. My understanding is that a JVM can handle doubles atomically but it's not guaranteed to. You need to use the voltatile keyword to explicitly mark doubles to be sure...

https://docs.oracle.com/javase/specs/jl ... l#jls-17.7

One possibility is that early in a Votlage Module session the bytecode is being interpreted and double assignment is non-atomic but once HotSpot does its JIT compilation they do become atomic.

To pass a fragile data structure such as an ArraList from a Notify() thread to ProcessSample() one must use some synchronization technique. Now as reference assignment is guaranteed to be atomic one simple way to do this is to use atomic pointers as I mentioned previously. Here one has two separate objects. One that can be safely modified by a Notify() thread and another that is in effect immutable that's read by ProcessSample(). When your Notify() thread is done and the object is stable you can then do an atomic refernce assignment that changes the object seen by ProcessSample() in a safe manner without having to pay the performance penalty of a lock.

So to summarize. It's only when you want to do non-trivial stuff that this becomes a serious issue. Then you need to take care of thread-safety yourself. Ideas like getting the GUI Update thread to handle things safely without any explicit synchronization only appear to work they don't actually.
Last edited by ColinP on Thu Jul 27, 2023 11:50 am, edited 1 time in total.
ColinP
Posts: 953
Joined: Mon Aug 03, 2020 7:46 pm

Re: Threading nightmares

Post by ColinP »

Ah, I see poetix and my posts crossed and he's already covered some of the same ground. :)

By the way I wouldn't recommend doing a trigger test with zero as the threshold. I think most people use 2.5 V as the threshold as this means if someone uses something like a leaky integrator to smooth then it's more reliable. For instance an input that's 0.0000000001 V would be seen as high even though most people would expect that to be treated as low.

In hardware one would use a Schmitt trigger to add hysteresis too but that's not needed in software.
User avatar
utdgrant
Posts: 541
Joined: Wed Apr 07, 2021 8:58 am
Location: Scotland
Contact:

Re: Threading nightmares

Post by utdgrant »

ColinP wrote: Thu Jul 27, 2023 11:28 am By the way I wouldn't recommend doing a trigger test with zero as the threshold. I think most people use 2.5 V as the threshold as this means if someone uses something like a leaky integrator to smooth then it's more reliable.
I originally made the Gate In threshold of Big Rat to be 0V. I had designed it such that it could be used as a zero-crossing detector for guitar-synth type applications, so anything even slightly above 0.0V was considered to be 'Gate On'. I later realised that it could cause problems for users who assumed that it was using the VM 'standard' of 2.5V, so I introduced a switch. (In my defence, it was the very first module I designed.)
BigRatGateSwitch.PNG
BigRatGateSwitch.PNG (72.82 KiB) Viewed 7320 times
The VM921 Oscillator has even more non-standard ("even less-standard" ???) specs on its Clamp Trig Jack inputs:
Clamping Point and Clamp Trig. jack- This is commonly known as oscillator sync. Sending a trigger or gate voltage to the Clamp Trig. jack restarts the wave from a point defined by the Clamping Point knob, from 0% to 100% of its cycle. (Incidentally, my family used to vacation at Clamping Point Knob in rural Maine in the 70s, but that's not important right now).

The V input is sensitive to rising edges passing through a +1V threshold. The S input is sensitive to falling edges passing through +1V. This reproduces the logic of S-trigger action without the headache.
BTW, I totally love the humo(u)r of the CA documentation!
______________________
Dome Music Technologies
ColinP
Posts: 953
Joined: Mon Aug 03, 2020 7:46 pm

Re: Threading nightmares

Post by ColinP »

Cherryaudio Danny wrote: Wed Jul 26, 2023 6:21 pm However, it is not recommended to modify knobs within the ProcessSample() function as this is a time-sensitive thread and GUI functions shouldn't occur within it.
If VoltageKnob actually is calling AWT rendering methiods inside SetValue() then it shouldn't be. Instead it should be using AWT repaint() methods either directly or via JUCE.

The repaint() methods only mark an AWT component as damaged they don't do any rendering. AWT will then (while potentially collapsing multiple repaint calls and building a clipping union) schedule its own event dispatcher to update and eventualy repair the display by calling AWT paint() methods on damaged components. AWT paint is presumably used by JUCE or VM to repaint VM's knobs etc and also patched through to VM's Notify() Canvas_Painting for custom painting jobs. The AWT event dispatcher has it's own thread so it will not directly impact performance iin ProcessSample().

Also presumably VoltageCanvas Invalidate() and InvalidateRect() are just wrappers for java.awt.Component repaint() methods.

If the VM API exposed its underlying AWT Component instances it would make my life a lot easier but I can see why one might decide not to do that as it would give third party devs more "rope" than might seem sensible to CA.

For more info see https://www.oracle.com/java/technologies/painting.html
borkman
Posts: 51
Joined: Tue May 09, 2023 7:26 pm

Re: Threading nightmares

Post by borkman »

ColinP wrote: Thu Jul 27, 2023 11:23 am So to summarize. It's only when you want to do non-trivial stuff that this becomes a serious issue. Then you need to take care of thread-safety yourself. Ideas like getting the GUI Update thread to handle things safely without any explicit synchronization only appear to work they don't actually.
This is a great summary. However, I would say that the bar for "non-trivial" is quite low and has been crossed many times without people realizing it. Setting two variables at once? Incrementing a variable? Getting multiple variables for use in a calculation? These are all potential issues. Will it matter to the functioning of the module? It depends on the module. It could hang. It could produce weird results. It could self correct. It depends. There is no mention of handling threading issues in the documentation I found. This error really should rectified.

A module is essentially a (potentially) stateful callback mechanism. Each callback has to be thread safe? It seems fragile to rely on each module developer to do the right thing both in both performance and stability. Threading can be subtle. The last statement of "only appear to work" scares me. I wonder how many module modules this describes.
ColinP
Posts: 953
Joined: Mon Aug 03, 2020 7:46 pm

Re: Threading nightmares

Post by ColinP »

I totally agree borkman. What's "non-trivial" is very subjective, sorry it was just shorthand. But I hope my extensive commentary does address how "dangerous" programming VM can be and why strange stuff happens now and then for no apparent reason.

To be fair to CA thread-safety problems are not unusual in the world of software, especially when there are multiple opaque sub-systems. Most of the time everything is fine but now and then the proverbial hits the fan. During a recent gig using VM I confess I was a little nervous about it crashing but it was fine.

It would help if CA updated VM to remove these vulnerabilities but I can't see that happening so I'm currently working my way through the API replacing most things that can be replaced. But what would be relatively easy for CA to do is add some warnings in the SDK documentation so that devs, especially beginners, were made more aware of thread-safety risks.
borkman
Posts: 51
Joined: Tue May 09, 2023 7:26 pm

Re: Threading nightmares

Post by borkman »

ColinP wrote: Thu Jul 27, 2023 6:18 pm I totally agree borkman. What's "non-trivial" is very subjective, sorry it was just shorthand. But I hope my extensive commentary does address how "dangerous" programming VM can be and why strange stuff happens now and then for no apparent reason.

To be fair to CA thread-safety problems are not unusual in the world of software, especially when there are multiple opaque sub-systems. Most of the time everything is fine but now and then the proverbial hits the fan. During a recent gig using VM I confess I was a little nervous about it crashing but it was fine.

It would help if CA updated VM to remove these vulnerabilities but I can't see that happening so I'm currently working my way through the API replacing most things that can be replaced. But what would be relatively easy for CA to do is add some warnings in the SDK documentation so that devs, especially beginners, were made more aware of thread-safety risks.
I wasn't calling you out on the use of "non-trivial". I was just emphasizing how common the potential for error likely is. Simple things may be dangerous. Good word choice!

As far as threading issues, I am unfortunately well aware of them. I've fixed, and caused, many over the years. :roll: Usually though, I have a more complete contextual understanding. There is none here except what has been gleaned from people that have gone deep into VM like yourself and others who have contributed to this thread. Without even mentioning threading in the docs, I assumed it was handled at the VM level. I can naively envision a way for that to happen, but I know better than to say for sure. Most of VM is opaque, which isn't necessarily a bad thing, unless there are hidden gotchas, which there seem to be.

I was going to submit a new module today, but I think I need to carefully review it, and my other modules, for threading issues. I was really hoping this was handled for us, at least for all but the most complex cases. I thought about the subtleties of this sort of thing for 30+ years; I didn't want to think about it during my retirement. I just wanted to write cool audio software. Maybe there's no getting away from it. :cry:
ColinP
Posts: 953
Joined: Mon Aug 03, 2020 7:46 pm

Re: Threading nightmares

Post by ColinP »

Yeah, thread-safety has bitten me so many times and continues to!

I think I mentioned before on this forum that in my 1980's game ECO (not Ecco or echo) for the Atari ST and Amiga as I was handing it over to the publisher it crashed because the graphical double-buffering and two creatures mating coincided at exactly the wrong time and what I thought was an atomic pointer switch wasn't actually atomic.

https://youtu.be/og4_cAgzMlc

Not the best demo as the guy hasn't figured out that you can automate eating and mating so he struggles to evolve past being a spider and unfortunately the music is turned off and the music was actually pretty good. It even output MIDI so you could drive a synth with it.
Post Reply

Return to “Module Designer”